exception_wrapper: now without undefined behavior

Summary:
Converting from std::exception* to void* to T* (where T is not std::exception
but a derived type) is undefined behavior (and will break with multiple or
virtual inheritance). Luckily, there's no need for void* there at all.

Also, don't force make_exception_wrapper to capture by value.

Test Plan: exception_wrapper_test

Reviewed By: marccelani@fb.com

FB internal diff: D1308251

@override-unit-failures

diff --git a/folly/ExceptionWrapper.h b/folly/ExceptionWrapper.h
index 647c870f22b580d8ce2d1a2373fc072bc0fbdbce..0a232728b1be0174ef376127a2db44b28e26540a 100644 (file)
--- a/folly/ExceptionWrapper.h
+++ b/folly/ExceptionWrapper.h
@@ -17,6 +17,7 @@
 #ifndef FOLLY_EXCEPTIONWRAPPER_H
 #define FOLLY_EXCEPTIONWRAPPER_H
 
+#include <cassert>
 #include <exception>
 #include <memory>
 #include "folly/detail/ExceptionWrapper.h"
@@ -33,23 +34,30 @@ class exception_wrapper {
     }
   }
 
-  std::exception* get() {
-    return item_.get();
-  }
+  std::exception* get() { return item_.get(); }
+  const std::exception* get() const { return item_.get(); }
+
+  std::exception* operator->() { return get(); }
+  const std::exception* operator->() const { return get(); }
+
+  std::exception& operator*() { assert(get()); return *get(); }
+  const std::exception& operator*() const { assert(get()); return *get(); }
+
+  explicit operator bool() const { return get(); }
 
  private:
   std::shared_ptr<std::exception> item_;
-  void (*throwfn_)(void*);
+  void (*throwfn_)(std::exception*);
 
   template <class T, class... Args>
-  friend exception_wrapper make_exception_wrapper(Args... args);
+  friend exception_wrapper make_exception_wrapper(Args&&... args);
 };
 
 template <class T, class... Args>
-exception_wrapper make_exception_wrapper(Args... args) {
+exception_wrapper make_exception_wrapper(Args&&... args) {
   exception_wrapper ew;
   ew.item_ = std::make_shared<T>(std::forward<Args>(args)...);
-  ew.throwfn_ = folly::detail::thrower<T>::doThrow;
+  ew.throwfn_ = folly::detail::Thrower<T>::doThrow;
   return ew;
 }
 

diff --git a/folly/detail/ExceptionWrapper.h b/folly/detail/ExceptionWrapper.h
index bc88bf8921f6ae938bd29ae45ded26969df5fab5..4c12834b3b4533491eef80c674480894234a4224 100644 (file)
--- a/folly/detail/ExceptionWrapper.h
+++ b/folly/detail/ExceptionWrapper.h
@@ -20,10 +20,10 @@
 namespace folly { namespace detail {
 
 template <class T>
-class thrower {
+class Thrower {
  public:
-  static void doThrow(void* obj) {
-    throw *((T*)(obj));
+  static void doThrow(std::exception* obj) {
+    throw *static_cast<T*>(obj);
   }
 };
 

diff --git a/folly/test/ExceptionWrapperTest.cpp b/folly/test/ExceptionWrapperTest.cpp
index 6aad3f157e05e201c5b5c3c61c502f92f50581b2..c20535eeaa937187e47ab668f8d1fa364e565d1c 100644 (file)
--- a/folly/test/ExceptionWrapperTest.cpp
+++ b/folly/test/ExceptionWrapperTest.cpp
@@ -36,3 +36,10 @@ TEST(ExceptionWrapper, throw_test) {
     EXPECT_EQ(expected, actual);
   }
 }
+
+TEST(ExceptionWrapper, boolean) {
+  auto ew = exception_wrapper();
+  EXPECT_FALSE(bool(ew));
+  ew = make_exception_wrapper<std::runtime_error>("payload");
+  EXPECT_TRUE(bool(ew));
+}