EricWF added inline comments.

================
Comment at: src/condition_variable.cpp:86
@@ +85,3 @@
+    if (tl_ptr.get() == nullptr) {
+        tl_ptr.set_pointer(new __thread_struct);
+    }
----------------
bcraig wrote:
> The standard synopsis of notify_all_at_thread_exit in the standard doesn't 
> have a "Throws:" clause.  I think that means that this is non-conformant.
> 
> There is some phrasing in 30.2.2 [thread.req.exception]:
>    Failure to allocate storage shall be reported as described in 17.6.5.12 
> [res.on.exception.handling].
> But I don't see wording in that section that gives permission to throw from 
> functions without Throws clauses.
> 
> From a user perspective, I generally expect my synchronization primitives to 
> be no-throw.  I don't know if it is really possible to provide that guarantee 
> with notify_all_at_thread_exit though, especially if we want it to work 
> outside of a std::thread.
[res.on.exception.handling]/p4
> 
> Destructor operations defined in the C ++ standard library shall not throw 
> exceptions. Every destructor
> in the C ++ standard library shall behave as if it had a non-throwing 
> exception specification. Any other
> functions defined in the C ++ standard library that do not have an 
> exception-specification may throw
> implementation-defined exceptions unless otherwise specified. [188] An 
> implementation may strengthen this
> implicit exception-specification by adding an explicit one.

And the relevant note:

> 188) In particular, they can report a failure to allocate storage by throwing 
> an exception of type bad_alloc, or a class derived
> from bad_alloc (18.6.3.1). Library implementations should report errors by 
> throwing exceptions of or derived from the standard
> exception classes (18.6.3.1, 18.8, 19.2).

So throwing `std::bad_alloc` is allowed, as it is almost everywhere in the STL. 
Also the function already [allocates memory to store the new 
entry](https://github.com/llvm-mirror/libcxx/blob/master/src/thread.cpp#L202).










https://reviews.llvm.org/D24159



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to