From: Yedidya Feldblum <yfeldblum@fb.com>
Date: Thu, 29 Dec 2016 23:47:55 +0000 (-0800)
Subject: Const-correctness for folly::exception_wrapper::with_exception with non-std::exception
X-Git-Tag: v2017.03.06.00~150
X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=686092cb22fbe4ddb4ffefec9b9bf0ae000d2f68

Const-correctness for folly::exception_wrapper::with_exception with non-std::exception

Summary:
[Folly] Const-correctness for `folly::exception_wrapper::with_exception` with non-`std::exception`.

This also lets us unify the various flavors of `with_exception` and `is_compatible_with`. And fix `is_compatible_with` for non-`exception` types.

Reviewed By: ericniebler

Differential Revision: D4364474

fbshipit-source-id: 417edfd45f7cfba952ce961559da67769b7c41bc
---

diff --git a/folly/ExceptionWrapper.h b/folly/ExceptionWrapper.h
index c6003818..6677dae6 100644
--- a/folly/ExceptionWrapper.h
+++ b/folly/ExceptionWrapper.h
@@ -190,18 +190,7 @@ class exception_wrapper {
 
   template <class Ex>
   bool is_compatible_with() const {
-    if (item_) {
-      return dynamic_cast<const Ex*>(item_.get());
-    } else if (eptr_) {
-      try {
-        std::rethrow_exception(eptr_);
-      } catch (typename std::decay<Ex>::type&) {
-        return true;
-      } catch (...) {
-        // fall through
-      }
-    }
-    return false;
+    return with_exception<Ex>([](const Ex&) {});
   }
 
   template <class F>
@@ -213,46 +202,21 @@ class exception_wrapper {
   template <class F>
   bool with_exception(F&& f) const {
     using arg_type = typename functor_traits<F>::arg_type_decayed;
-    return with_exception<const arg_type>(std::forward<F>(f));
+    return with_exception<arg_type>(std::forward<F>(f));
   }
 
   // If this exception wrapper wraps an exception of type Ex, with_exception
   // will call f with the wrapped exception as an argument and return true, and
   // will otherwise return false.
   template <class Ex, class F>
-  typename std::enable_if<
-    std::is_base_of<std::exception, typename std::decay<Ex>::type>::value,
-    bool>::type
-  with_exception(F f) {
+  bool with_exception(F f) {
     return with_exception1<typename std::decay<Ex>::type>(f, this);
   }
 
   // Const overload
   template <class Ex, class F>
-  typename std::enable_if<
-    std::is_base_of<std::exception, typename std::decay<Ex>::type>::value,
-    bool>::type
-  with_exception(F f) const {
-    return with_exception1<const typename std::decay<Ex>::type>(f, this);
-  }
-
-  // Overload for non-exceptions. Always rethrows.
-  template <class Ex, class F>
-  typename std::enable_if<
-    !std::is_base_of<std::exception, typename std::decay<Ex>::type>::value,
-    bool>::type
-  with_exception(F f) const {
-    try {
-      if (*this) {
-        throwException();
-      }
-    } catch (typename std::decay<Ex>::type& e) {
-      f(e);
-      return true;
-    } catch (...) {
-      // fall through
-    }
-    return false;
+  bool with_exception(F f) const {
+    return with_exception1<typename std::decay<Ex>::type>(f, this);
   }
 
   std::exception_ptr getExceptionPtr() const {
@@ -335,20 +299,38 @@ class exception_wrapper {
     }
   };
 
+  template <typename T>
+  using is_exception_ = std::is_base_of<std::exception, T>;
+
+  template <bool V, typename T, typename F>
+  using conditional_t_ = typename std::conditional<V, T, F>::type;
+
+  template <typename T, typename F>
+  static typename std::enable_if<is_exception_<T>::value, T*>::type
+  try_dynamic_cast_exception(F* from) {
+    return dynamic_cast<T*>(from);
+  }
+  template <typename T, typename F>
+  static typename std::enable_if<!is_exception_<T>::value, T*>::type
+  try_dynamic_cast_exception(F*) {
+    return nullptr;
+  }
+
   // 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->item_.get())) {
+    using CEx = conditional_t_<std::is_const<T>::value, const Ex, Ex>;
+    if (is_exception_<Ex>::value && that->item_) {
+      if (auto ex = try_dynamic_cast_exception<CEx>(that->item_.get())) {
         f(*ex);
         return true;
       }
     } else if (that->eptr_) {
       try {
         std::rethrow_exception(that->eptr_);
-      } catch (Ex& e) {
+      } catch (CEx& e) {
         f(e);
         return true;
       } catch (...) {
diff --git a/folly/test/ExceptionWrapperTest.cpp b/folly/test/ExceptionWrapperTest.cpp
index 26090a76..83903bc8 100644
--- a/folly/test/ExceptionWrapperTest.cpp
+++ b/folly/test/ExceptionWrapperTest.cpp
@@ -197,13 +197,15 @@ TEST(ExceptionWrapper, with_exception_test) {
   EXPECT_FALSE(
       empty_ew.with_exception([&](const std::exception& /* ie */) { FAIL(); }));
 
+  // Testing with const exception_wrapper; sanity check first:
+  EXPECT_FALSE(cew.with_exception([&](const std::runtime_error&) {}));
+  EXPECT_FALSE(cew.with_exception([&](const int&) {}));
   // 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();
-    });
-*/
+  /*
+  EXPECT_FALSE(cew.with_exception([&](std::runtime_error&) {}));
+  EXPECT_FALSE(cew.with_exception([&](int&) {}));
+  */
 }
 
 TEST(ExceptionWrapper, getExceptionPtr_test) {
@@ -282,6 +284,7 @@ TEST(ExceptionWrapper, non_std_exception_test) {
     });
   EXPECT_TRUE(bool(ew));
   EXPECT_FALSE(ew.is_compatible_with<std::exception>());
+  EXPECT_TRUE(ew.is_compatible_with<int>());
   EXPECT_EQ(ew.what(), kIntClassName);
   EXPECT_EQ(ew.class_name(), kIntClassName);
   // non-std::exception types are supported, but the only way to