SCOPE_SUCCESS should be able to throw, the others shouldn't

Summary: SCOPE_SUCCESS may legitimately throw, so changed the `noexcept` attribute to a conditional one. Also added the noexcept promise to the lambdas in the hope the compiler will check code during compilation appropriately. (It currently doesn't.)

Test Plan: Added a unittest that failed, not passes.

Reviewed By: simpkins@fb.com

FB internal diff: D1093328

@override-unit-failures

diff --git a/folly/ScopeGuard.h b/folly/ScopeGuard.h
index bb9ad86728857679a0e4616ddfc70501af8da209..8da5af50e43b1f2a38570c0cea5d2fe225e3ac6e 100644 (file)
--- a/folly/ScopeGuard.h
+++ b/folly/ScopeGuard.h
@@ -157,9 +157,9 @@ class ScopeGuardForNewException {
       , exceptionCounter_(std::move(other.exceptionCounter_)) {
   }
 
-  ~ScopeGuardForNewException() noexcept {
+  ~ScopeGuardForNewException() noexcept(executeOnException) {
     if (executeOnException == exceptionCounter_.isNewUncaughtException()) {
-      execute();
+      function_();
     }
   }
 
@@ -168,8 +168,6 @@ class ScopeGuardForNewException {
 
   void* operator new(size_t) = delete;
 
-  void execute() noexcept { function_(); }
-
   FunctionType function_;
   UncaughtExceptionCounter exceptionCounter_;
 };
@@ -219,17 +217,17 @@ operator+(detail::ScopeGuardOnExit, FunctionType&& fn) {
 
 #define SCOPE_EXIT \
   auto FB_ANONYMOUS_VARIABLE(SCOPE_EXIT_STATE) \
-  = ::folly::detail::ScopeGuardOnExit() + [&]
+  = ::folly::detail::ScopeGuardOnExit() + [&]() noexcept
 
 #if defined(FOLLY_EXCEPTION_COUNT_USE_CXA_GET_GLOBALS) || \
     defined(FOLLY_EXCEPTION_COUNT_USE_GETPTD)
 #define SCOPE_FAIL \
   auto FB_ANONYMOUS_VARIABLE(SCOPE_FAIL_STATE) \
-  = ::folly::detail::ScopeGuardOnFail() + [&]
+  = ::folly::detail::ScopeGuardOnFail() + [&]() noexcept
 
 #define SCOPE_SUCCESS \
   auto FB_ANONYMOUS_VARIABLE(SCOPE_SUCCESS_STATE) \
-  = ::folly::detail::ScopeGuardOnSuccess() + [&]
+  = ::folly::detail::ScopeGuardOnSuccess() + [&]()
 #endif // native uncaught_exception() supported
 
 #endif // FOLLY_SCOPEGUARD_H_

diff --git a/folly/test/ScopeGuardTest.cpp b/folly/test/ScopeGuardTest.cpp
index f8e10c5cdf374340da501cec0b2b7c7f27fcdc8d..f03e9a082b8aba38fe0a14a90e2d68de79fdf2e9 100644 (file)
--- a/folly/test/ScopeGuardTest.cpp
+++ b/folly/test/ScopeGuardTest.cpp
@@ -284,6 +284,13 @@ TEST(ScopeGuard, TEST_SCOPE_FAIL_AND_SCOPE_SUCCESS) {
   testScopeFailAndScopeSuccess(ErrorBehavior::UNHANDLED_ERROR, true);
 }
 
+TEST(ScopeGuard, TEST_SCOPE_SUCCESS_THROW) {
+  auto lambda = []() {
+    SCOPE_SUCCESS { throw std::runtime_error("ehm"); };
+  };
+  EXPECT_THROW(lambda(), std::runtime_error);
+}
+
 int main(int argc, char** argv) {
   testing::InitGoogleTest(&argc, argv);
   google::ParseCommandLineFlags(&argc, &argv, true);