bcraig added a comment.
LGTM
================
Comment at: src/condition_variable.cpp:86
@@ +85,3 @@
+ if (tl_ptr.get() == nullptr) {
+ tl_ptr.set_pointer(new __thread_struct);
+ }
----------------
EricWF wrote:
> 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).
>
>
>
>
>
>
>
>
Yet again, I find that reading comprehension with regards to the standard isn't
my strong suit :). I got to "Destructor operations" in p4 and stopped reading.
I will still wish for a noexcept version of this function, but it's a pretty
impractical wish. The implementation you have here is fine.
https://reviews.llvm.org/D24159
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
