Commit a62c9841 authored by Phil Willoughby's avatar Phil Willoughby Committed by Facebook Github Bot

Make exception_wrapper::throwException() strict

Summary:
The definition of `throwException()` is changed to add the `[[noreturn]]`
attribute for consistency with `std::rethrow_exception`. If the
`exception_wrapper` contains an exception, that is rethrown; if not, then
`std::terminate` is called.

An alternative design which threw an exception if the `exception_wrapper` did not contain an exception was rejected because of the ambiguity it introduced as to whether the exception thrown was wrapped or not.

Benchmarks before:

```

============================================================================
folly/test/ExceptionWrapperBenchmark.cpp        relative  time/iter  iters/s
============================================================================
exception_ptr_create_and_test                                1.45us  689.01K
exception_wrapper_create_and_test               4337.80%    33.46ns   29.89M
----------------------------------------------------------------------------
exception_ptr_create_and_test_concurrent                   342.99us    2.92K
exception_wrapper_create_and_test_concurrent     101.41%   338.21us    2.96K
----------------------------------------------------------------------------
exception_ptr_create_and_throw                               3.05us  327.89K
exception_wrapper_create_and_throw               140.22%     2.17us  459.77K
exception_wrapper_create_and_cast               8956.80%    34.05ns   29.37M
----------------------------------------------------------------------------
exception_ptr_create_and_throw_concurrent                  372.68us    2.68K
exception_wrapper_create_and_throw_concurrent    102.54%   363.44us    2.75K
exception_wrapper_create_and_cast_concurrent     110.93%   335.97us    2.98K
============================================================================
```

and after:

```

============================================================================
folly/test/ExceptionWrapperBenchmark.cpp        relative  time/iter  iters/s
============================================================================
exception_ptr_create_and_test                                1.46us  684.09K
exception_wrapper_create_and_test               4368.84%    33.46ns   29.89M
----------------------------------------------------------------------------
exception_ptr_create_and_test_concurrent                   341.20us    2.93K
exception_wrapper_create_and_test_concurrent      99.88%   341.60us    2.93K
----------------------------------------------------------------------------
exception_ptr_create_and_throw                               3.08us  324.93K
exception_wrapper_create_and_throw               106.93%     2.88us  347.46K
exception_wrapper_create_and_cast               9532.05%    32.29ns   30.97M
----------------------------------------------------------------------------
exception_ptr_create_and_throw_concurrent                  363.73us    2.75K
exception_wrapper_create_and_throw_concurrent    101.04%   360.00us    2.78K
exception_wrapper_create_and_cast_concurrent     104.50%   348.07us    2.87K
============================================================================
```

Reviewed By: yfeldblum

Differential Revision: D3923939

fbshipit-source-id: 6fc58ac571e59e4122f1fbd194c678e3a3841057
parent ba22e52f
......@@ -148,12 +148,15 @@ class exception_wrapper {
assign_eptr(eptr);
}
void throwException() const {
// If the exception_wrapper does not contain an exception, std::terminate()
// is invoked to assure the [[noreturn]] behaviour.
[[noreturn]] void throwException() const {
if (throwfn_) {
throwfn_(item_.get());
} else if (eptr_) {
std::rethrow_exception(eptr_);
}
std::terminate();
}
explicit operator bool() const {
......@@ -260,7 +263,9 @@ class exception_wrapper {
bool>::type
with_exception(F f) const {
try {
throwException();
if (*this) {
throwException();
}
} catch (typename std::decay<Ex>::type& e) {
f(e);
return true;
......@@ -276,7 +281,9 @@ class exception_wrapper {
}
try {
throwException();
if (*this) {
throwException();
}
} catch (...) {
return std::current_exception();
}
......
......@@ -170,9 +170,8 @@ TEST(ExceptionWrapper, with_exception_test) {
EXPECT_TRUE(bool(ew));
EXPECT_EQ(ew.what(), kIntExceptionClassName + ": int == 23");
EXPECT_EQ(ew.class_name(), kIntExceptionClassName);
ew.with_exception([&](const IntException& ie) {
EXPECT_EQ(ie.getInt(), expected);
});
EXPECT_TRUE(ew.with_exception(
[&](const IntException& ie) { EXPECT_EQ(ie.getInt(), expected); }));
// I can try_and_catch a non-copyable base class. This will use
// std::exception_ptr internally.
......@@ -183,15 +182,21 @@ TEST(ExceptionWrapper, with_exception_test) {
EXPECT_TRUE(bool(ew2));
EXPECT_EQ(ew2.what(), kIntExceptionClassName + ": int == 23");
EXPECT_EQ(ew2.class_name(), kIntExceptionClassName);
ew2.with_exception([&](AbstractIntException& ie) {
EXPECT_EQ(ie.getInt(), expected);
EXPECT_TRUE(dynamic_cast<IntException*>(&ie));
});
EXPECT_TRUE(ew2.with_exception([&](AbstractIntException& ie) {
EXPECT_EQ(ie.getInt(), expected);
EXPECT_TRUE(dynamic_cast<IntException*>(&ie));
}));
// Test with const this. If this compiles and does not crash due to
// infinite loop when it runs, it succeeds.
const exception_wrapper& cew = ew;
cew.with_exception([&](const IntException& /* ie */) { SUCCEED(); });
EXPECT_TRUE(
cew.with_exception([&](const IntException& /* ie */) { SUCCEED(); }));
// Test with empty ew.
exception_wrapper empty_ew;
EXPECT_FALSE(
empty_ew.with_exception([&](const std::exception& /* ie */) { FAIL(); }));
// This won't even compile. You can't use a function which takes a
// non-const reference with a const exception_wrapper.
......@@ -202,6 +207,33 @@ TEST(ExceptionWrapper, with_exception_test) {
*/
}
TEST(ExceptionWrapper, getExceptionPtr_test) {
int expected = 23;
// This works, and doesn't slice.
exception_wrapper ew = try_and_catch<std::exception, std::runtime_error>(
[=]() { throw IntException(expected); });
std::exception_ptr eptr = ew.getExceptionPtr();
EXPECT_THROW(std::rethrow_exception(eptr), IntException);
// I can try_and_catch a non-copyable base class. This will use
// std::exception_ptr internally.
exception_wrapper ew2 = try_and_catch<AbstractIntException>(
[=]() { throw IntException(expected); });
eptr = ew2.getExceptionPtr();
EXPECT_THROW(std::rethrow_exception(eptr), IntException);
// Test with const this.
const exception_wrapper& cew = ew;
eptr = cew.getExceptionPtr();
EXPECT_THROW(std::rethrow_exception(eptr), IntException);
// Test with empty ew.
exception_wrapper empty_ew;
eptr = empty_ew.getExceptionPtr();
EXPECT_DEATH(std::rethrow_exception(eptr), "exception");
}
TEST(ExceptionWrapper, with_exception_deduction) {
auto ew = make_exception_wrapper<std::runtime_error>("hi");
EXPECT_TRUE(ew.with_exception([](std::runtime_error&) {}));
......@@ -268,6 +300,11 @@ TEST(ExceptionWrapper, exceptionStr) {
EXPECT_EQ(kRuntimeErrorClassName + ": argh", exceptionStr(ew));
}
TEST(ExceptionWrapper, throwException_noException) {
exception_wrapper ew;
ASSERT_DEATH(ew.throwException(), "exception");
}
namespace {
class TestException : public std::exception { };
void testEW(const exception_wrapper& ew) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment