Avoid magic macros in Futures FSM

Summary: [Folly] Avoid magic macros in Futures `FSM` - just use normal C++ types and functions instead.

Reviewed By: LeeHowes

Differential Revision: D7725582

fbshipit-source-id: 822e4ca6391a37dc4007833c975fe283cfe5105e

folly/futures/detail/Core.h

@@ -30,6 +30,7 @@
 #include <folly/Utility.h>
 #include <folly/futures/FutureException.h>
 #include <folly/futures/detail/FSM.h>
+#include <folly/lang/Assume.h>
 #include <folly/lang/Exception.h>
 #include <folly/synchronization/MicroSpinLock.h>
 
@@ -161,21 +162,23 @@ class Core final {
       callback_ = std::forward<F>(func);
     };
 
-    FSM_START(fsm_)
-      case State::Start:
-        FSM_UPDATE(fsm_, State::OnlyCallback, setCallback_);
-        break;
+    fsm_.transition([&](State state) {
+      switch (state) {
+        case State::Start:
+          return fsm_.tryUpdateState(state, State::OnlyCallback, setCallback_);
 
-      case State::OnlyResult:
-        FSM_UPDATE(fsm_, State::Armed, setCallback_);
-        transitionToArmed = true;
-        break;
+        case State::OnlyResult:
+          return fsm_.tryUpdateState(state, State::Armed, setCallback_, [&] {
+            transitionToArmed = true;
+          });
 
-      case State::OnlyCallback:
-      case State::Armed:
-      case State::Done:
-        throw_exception<std::logic_error>("setCallback called twice");
-    FSM_END
+        case State::OnlyCallback:
+        case State::Armed:
+        case State::Done:
+          throw_exception<std::logic_error>("setCallback called twice");
+      }
+      assume_unreachable();
+    });
 
     // we could always call this, it is an optimization to only call it when
     // it might be needed.
@@ -188,21 +191,23 @@ class Core final {
   void setResult(Try<T>&& t) {
     bool transitionToArmed = false;
     auto setResult_ = [&]{ result_ = std::move(t); };
-    FSM_START(fsm_)
-      case State::Start:
-        FSM_UPDATE(fsm_, State::OnlyResult, setResult_);
-        break;
-
-      case State::OnlyCallback:
-        FSM_UPDATE(fsm_, State::Armed, setResult_);
-        transitionToArmed = true;
-        break;
+    fsm_.transition([&](State state) {
+      switch (state) {
+        case State::Start:
+          return fsm_.tryUpdateState(state, State::OnlyResult, setResult_);
+
+        case State::OnlyCallback:
+          return fsm_.tryUpdateState(state, State::Armed, setResult_, [&] {
+            transitionToArmed = true;
+          });
 
-      case State::OnlyResult:
-      case State::Armed:
-      case State::Done:
-      throw_exception<std::logic_error>("setResult called twice");
-    FSM_END
+        case State::OnlyResult:
+        case State::Armed:
+        case State::Done:
+          throw_exception<std::logic_error>("setResult called twice");
+      }
+      assume_unreachable();
+    });
 
     if (transitionToArmed) {
       maybeCallback();
@@ -321,16 +326,19 @@ class Core final {
   };
 
   void maybeCallback() {
-    FSM_START(fsm_)
-      case State::Armed:
-        if (active_.load(std::memory_order_acquire)) {
-          FSM_UPDATE2(fsm_, State::Done, []{}, [this]{ this->doCallback(); });
-        }
-        FSM_BREAK
-
-      default:
-        FSM_BREAK
-    FSM_END
+    fsm_.transition([&](State state) {
+      switch (state) {
+        case State::Armed:
+          if (active_.load(std::memory_order_acquire)) {
+            return fsm_.tryUpdateState(
+                state, State::Done, [] {}, [&] { doCallback(); });
+          }
+          return true;
+
+        default:
+          return true;
+      }
+    });
   }
 
   void doCallback() {

folly/futures/detail/FSM.h

@@ -99,28 +99,13 @@ class FSM {
     }
     return result;
   }
-};
-
-#define FSM_START(fsm) {\
-    bool done = false; \
-    while (!done) { auto state = fsm.getState(); switch (state) {
-#define FSM_UPDATE2(fsm, b, protectedAction, unprotectedAction) \
-  done = fsm.tryUpdateState(state, (b), (protectedAction), (unprotectedAction));
-
-#define FSM_UPDATE(fsm, b, action) FSM_UPDATE2(fsm, (b), (action), []{})
-
-#define FSM_CASE(fsm, a, b, action) \
-  case (a): \
-    FSM_UPDATE(fsm, (b), (action)); \
-    break;
-
-#define FSM_CASE2(fsm, a, b, protectedAction, unprotectedAction) \
-  case (a): \
-    FSM_UPDATE2(fsm, (b), (protectedAction), (unprotectedAction)); \
-    break;
 
-#define FSM_BREAK done = true; break;
-#define FSM_END }}}
+  template <class F>
+  void transition(F f) {
+    while (!f(getState())) {
+    }
+  }
+};
 
 } // namespace detail
 } // namespace futures

folly/futures/test/FSMTest.cpp

@@ -15,6 +15,8 @@
  */
 
 #include <folly/futures/detail/FSM.h>
+
+#include <folly/lang/Assume.h>
 #include <folly/portability/GTest.h>
 
 using namespace folly::futures::detail;
@@ -29,13 +31,13 @@ TEST(FSM, example) {
   // somebody set up us the switch
   auto tryTransition = [&]{
     switch (fsm.getState()) {
-    case State::A:
-      return fsm.tryUpdateState(State::A, State::B, [&] { count++; });
-    case State::B:
-      return fsm.tryUpdateState(
-          State::B, State::A, [&] { count--; }, [&] { unprotectedCount--; });
+      case State::A:
+        return fsm.tryUpdateState(State::A, State::B, [&] { count++; });
+      case State::B:
+        return fsm.tryUpdateState(
+            State::B, State::A, [&] { count--; }, [&] { unprotectedCount--; });
     }
-    return false; // unreachable
+    folly::assume_unreachable();
   };
 
   // keep retrying until success (like a cas)
@@ -54,18 +56,23 @@ TEST(FSM, example) {
   EXPECT_EQ(-1, unprotectedCount);
 }
 
-TEST(FSM, magicMacrosExample) {
+TEST(FSM, transition) {
   struct MyFSM {
     FSM<State, std::mutex> fsm_;
     int count = 0;
     int unprotectedCount = 0;
     MyFSM() : fsm_(State::A) {}
     void twiddle() {
-      FSM_START(fsm_)
-        FSM_CASE(fsm_, State::A, State::B, [&]{ count++; });
-        FSM_CASE2(fsm_, State::B, State::A,
-                  [&]{ count--; }, [&]{ unprotectedCount--; });
-      FSM_END
+      fsm_.transition([&](State state) {
+        switch (state) {
+          case State::A:
+            return fsm_.tryUpdateState(state, State::B, [&] { count++; });
+          case State::B:
+            return fsm_.tryUpdateState(
+                state, State::A, [&] { count--; }, [&] { unprotectedCount--; });
+        }
+        folly::assume_unreachable();
+      });
     }
   };