From 9d4d4b000cc218ee0c9c430257222dd63ad766ff Mon Sep 17 00:00:00 2001 From: Marc Horowitz Date: Wed, 10 Sep 2014 17:39:19 -0700 Subject: [PATCH] make with_exception() const work in all cases Summary: Abstract the with_exception() logic into a static template function which will work with a const or non-const object. Test Plan: exception_wrapper_test Reviewed By: vloh@fb.com Subscribers: njormrod FB internal diff: D1549175 Tasks: 5127267 --- folly/ExceptionWrapper.h | 49 +++++++++++++++++------------ folly/test/ExceptionWrapperTest.cpp | 15 +++++++++ 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/folly/ExceptionWrapper.h b/folly/ExceptionWrapper.h index c0619727..953ce4d4 100644 --- a/folly/ExceptionWrapper.h +++ b/folly/ExceptionWrapper.h @@ -169,29 +169,12 @@ class exception_wrapper { template bool with_exception(F f) { - if (item_) { - if (auto ex = dynamic_cast(getCopied())) { - f(*ex); - return true; - } - } else if (eptr_) { - try { - std::rethrow_exception(eptr_); - } catch (std::exception& e) { - if (auto ex = dynamic_cast(&e)) { - f(*ex); - return true; - } - } catch (...) { - // fall through - } - } - return false; + return with_exception1(f, this); } template bool with_exception(F f) const { - return with_exception(f); + return with_exception1(f, this); } std::exception_ptr getExceptionPtr() const { @@ -207,7 +190,7 @@ class exception_wrapper { return std::exception_ptr(); } - protected: +protected: // Optimized case: if we know what type the exception is, we can // store a copy of the concrete type, and a helper function so we // can rethrow it. @@ -221,6 +204,32 @@ class exception_wrapper { template friend exception_wrapper make_exception_wrapper(Args&&... args); + +private: + // What makes this useful is that T can be exception_wrapper* or + // const exception_wrapper*, and the compiler will use the + // instantiation which works with F. + template + static bool with_exception1(F f, T* that) { + if (that->item_) { + if (auto ex = dynamic_cast(that->getCopied())) { + f(*ex); + return true; + } + } else if (that->eptr_) { + try { + std::rethrow_exception(that->eptr_); + } catch (std::exception& e) { + if (auto ex = dynamic_cast(&e)) { + f(*ex); + return true; + } + } catch (...) { + // fall through + } + } + return false; + } }; template diff --git a/folly/test/ExceptionWrapperTest.cpp b/folly/test/ExceptionWrapperTest.cpp index ad8c43ce..5c99c684 100644 --- a/folly/test/ExceptionWrapperTest.cpp +++ b/folly/test/ExceptionWrapperTest.cpp @@ -170,6 +170,21 @@ TEST(ExceptionWrapper, with_exception_test) { EXPECT_EQ(ie.getInt(), expected); EXPECT_EQ(typeid(ie), typeid(IntException)); }); + + // 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(); + }); + + // This won't even compile. You can't use a function which takes a + // non-const reference with a const exception_wrapper. +/* + cew.with_exception([&](IntException& ie) { + SUCCEED(); + }); +*/ } TEST(ExceptionWrapper, non_std_exception_test) { -- 2.34.1