Page MenuHomePhabricator

[libcxx] guard throw with exception enabling check
ClosedPublic

Authored by weimingz on Jun 23 2016, 9:02 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

weimingz updated this revision to Diff 61755.Jun 23 2016, 9:02 PM
weimingz retitled this revision from to [libcxx] guard throw with exception enabling check.
weimingz updated this object.
weimingz added a subscriber: cfe-commits.
rmaprath accepted this revision.Jun 23 2016, 11:49 PM
rmaprath added a reviewer: rmaprath.
rmaprath added a subscriber: rmaprath.

That looks fine to me.

This revision is now accepted and ready to land.Jun 23 2016, 11:49 PM
grandinj added inline comments.
include/experimental/optional
524 ↗(On Diff #61755)

If this kind of code is going to show up in lots of places, then maybe wrap it up in a macro:

_LIBCPP_THROW_OR_ASSERT(bad_optional_access(), "bad optional access")

?

With some extra preprocessor magic, and if LLVM didn't care about the string in the assert, could even leave out the second parameter

rmaprath added inline comments.Jun 24 2016, 1:36 AM
include/experimental/optional
524 ↗(On Diff #61755)

In D14653 (which did not land), I had used a _LIBCPP_THROW macro for a similar purpose (see changes in config/include). Currently there is no uniform mechanism adopted in the library for throwing exceptions.

I agree that it would be nice to fix this. I will revive that part of D14653 soon.

Thanks for the reminder!

It's a good idea. Currently, there are about 600+ "throws" being
guarded by _LIBCPP_NO_EXCEPTIONS macro.

How about let's merge the patch now and I can do the conversion of
existing code to the wrapper in background?

Weiming

It's a good idea. Currently, there are about 600+ "throws" being
guarded by _LIBCPP_NO_EXCEPTIONS macro.

How about let's merge the patch now and I can do the conversion of
existing code to the wrapper in background?

Weiming

Sounds good to me.

Thanks!

This revision was automatically updated to reflect the committed changes.

Just a reminder - the people who can approve patches for libc++ are @mclow.lists and @EricWF.

Just a reminder - the people who can approve patches for libc++ are @mclow.lists and @EricWF.

My bad, I thought it was too trivial. Won't happen again :)

/ Asiri