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

diff --git a/folly/ExceptionWrapper.h b/folly/ExceptionWrapper.h
index c06197273f9c76f708428048f29af9ff6d984abf..953ce4d479b1ec0127e9d2927b7d8226cd79daa6 100644 (file)
--- a/folly/ExceptionWrapper.h
+++ b/folly/ExceptionWrapper.h
@@ -169,29 +169,12 @@ class exception_wrapper {
 
   template <class Ex, class F>
   bool with_exception(F f) {
-    if (item_) {
-      if (auto ex = dynamic_cast<Ex*>(getCopied())) {
-        f(*ex);
-        return true;
-      }
-    } else if (eptr_) {
-      try {
-        std::rethrow_exception(eptr_);
-      } catch (std::exception& e) {
-        if (auto ex = dynamic_cast<Ex*>(&e)) {
-          f(*ex);
-          return true;
-        }
-      } catch (...) {
-        // fall through
-      }
-    }
-    return false;
+    return with_exception1<Ex>(f, this);
   }
 
   template <class Ex, class F>
   bool with_exception(F f) const {
-    return with_exception<const Ex>(f);
+    return with_exception1<const Ex>(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 <class T, class... Args>
   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 <class Ex, class F, class T>
+  static bool with_exception1(F f, T* that) {
+    if (that->item_) {
+      if (auto ex = dynamic_cast<Ex*>(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<Ex*>(&e)) {
+          f(*ex);
+          return true;
+        }
+      } catch (...) {
+        // fall through
+      }
+    }
+    return false;
+  }
 };
 
 template <class T, class... Args>

diff --git a/folly/test/ExceptionWrapperTest.cpp b/folly/test/ExceptionWrapperTest.cpp
index ad8c43cef0c6c54faea6ce6b8e085cea4be184a8..5c99c68405444c347f009295413c6b3e10cb5d54 100644 (file)
--- 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<IntException>([&](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>([&](IntException& ie) {
+      SUCCEED();
+    });
+*/
 }
 
 TEST(ExceptionWrapper, non_std_exception_test) {