This is an archive of the discontinued LLVM Phabricator instance.

Make exception-throwing from a noexcept build `abort()`.
ClosedPublic

Authored by mclow.lists on Aug 24 2016, 3:36 PM.

Details

Summary

This is a follow on to D21232, which marked a bunch of exception-throwing helper routines as noreturn.

Now, make them really never return. Either they throw, or they call abort.

Diff Detail

Event Timeline

mclow.lists retitled this revision from to Make exception-throwing from a noexcept build `abort()`..
mclow.lists updated this object.
mclow.lists added reviewers: EricWF, hiraditya, kparzysz.
mclow.lists added reviewers: laxmansole, sebpop.
mclow.lists added a subscriber: cfe-commits.
sebpop edited edge metadata.Aug 24 2016, 7:47 PM

I like the patch.
Thanks for removing #ifdefs from the code: it improves readability in general.
Would it be possible to move the __throw_* functions in a same .h file to avoid having them all over?

include/array
212

Maybe you can use __throw_out_of_range?
Fewer #ifdefs in the code is better.

226

__throw_out_of_range

include/bitset
218

To improve readability, could you please avoid the double negation by turning around the clauses:

#ifdef _LIBCPP_NO_EXCEPTIONS
  _VSTD::abort();
#else
  throw overflow_error(__msg);
#endif

And the same below in all the __throw_* functions. Thanks!

include/deque
909

Maybe you can add another function for this one:
__throw_length_error

920

__throw_out_of_range

include/experimental/any
106

Maybe add a new function __throw_bad_any_cast?

include/experimental/dynarray
145

New function: __throw_bad_array_length?

286

__throw_out_of_range

302

__throw_out_of_range

mclow.lists updated this revision to Diff 69201.EditedAug 24 2016, 11:33 PM
mclow.lists edited edge metadata.

I like this revision better; I need to check, but I think this hits all the exception classes.

Goal: Every exception class in the standard should have a VSTD::__throw_XXX function, marked _LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE, that does the throwing.

Note #1: __throw_bad_alloc is already in namespace std.
Note #2: __throw_runtime_error is already in the dylib.

mclow.lists added inline comments.Aug 24 2016, 11:37 PM
include/stdexcept
174

This comment belongs to __throw_runtime_error

mclow.lists added inline comments.Aug 24 2016, 11:39 PM
src/thread.cpp
59

Need to fix these, too.

sebpop accepted this revision.Aug 25 2016, 4:57 AM
sebpop edited edge metadata.

Very nice cleanup.
Maybe you can move some more #ifdefs into __throw_* functions, although as is LGTM.
Thanks!

This revision is now accepted and ready to land.Aug 25 2016, 4:57 AM
mclow.lists closed this revision.Aug 25 2016, 8:18 AM

Committed as revision 279744

I don't know how I missed this. Thanks for the patch!!!