Invoke Futures then-continuations with the same cvref as passed

Summary:
[Folly] Invoke Futures `then`-continuations with the same cvref as passed.

for example, if passed as `const&&`, invoke as `const&&`, and if passed as `volatile&`, invoke as `volatile&`.

This brings a little more consistency to the library.

Reviewed By: LeeHowes, phoad

Differential Revision: D8109784

fbshipit-source-id: 8dd56c6197211091ed53eba94aeec9d604268d15

folly/futures/Future-inl.h

@@ -59,18 +59,19 @@ namespace detail {
 //  may be fulfilled. Assumes the stored functor to be noexcept-destructible.
 template <typename T, typename F>
 class CoreCallbackState {
+  using DF = _t<std::decay<F>>;
+
  public:
-  template <typename FF>
-  CoreCallbackState(Promise<T>&& promise, FF&& func) noexcept(
-      noexcept(F(std::declval<FF>())))
-      : func_(std::forward<FF>(func)), promise_(std::move(promise)) {
+  CoreCallbackState(Promise<T>&& promise, F&& func) noexcept(
+      noexcept(DF(std::declval<F&&>())))
+      : func_(std::forward<F>(func)), promise_(std::move(promise)) {
     assert(before_barrier());
   }
 
   CoreCallbackState(CoreCallbackState&& that) noexcept(
-      noexcept(F(std::declval<F>()))) {
+      noexcept(DF(std::declval<F&&>()))) {
     if (that.before_barrier()) {
-      new (&func_) F(std::move(that.func_));
+      new (&func_) DF(std::forward<F>(that.func_));
       promise_ = that.stealPromise();
     }
   }
@@ -87,7 +88,7 @@ class CoreCallbackState {
   auto invoke(Args&&... args) noexcept(
       noexcept(std::declval<F&&>()(std::declval<Args&&>()...))) {
     assert(before_barrier());
-    return std::move(func_)(std::forward<Args>(args)...);
+    return std::forward<F>(func_)(std::forward<Args>(args)...);
   }
 
   template <typename... Args>
@@ -105,7 +106,7 @@ class CoreCallbackState {
 
   Promise<T> stealPromise() noexcept {
     assert(before_barrier());
-    func_.~F();
+    func_.~DF();
     return std::move(promise_);
   }
 
@@ -115,18 +116,22 @@ class CoreCallbackState {
   }
 
   union {
-    F func_;
+    DF func_;
   };
   Promise<T> promise_{Promise<T>::makeEmpty()};
 };
 
 template <typename T, typename F>
 auto makeCoreCallbackState(Promise<T>&& p, F&& f) noexcept(
-    noexcept(CoreCallbackState<T, _t<std::decay<F>>>(
+    noexcept(CoreCallbackState<T, F>(
         std::declval<Promise<T>&&>(),
         std::declval<F&&>()))) {
-  return CoreCallbackState<T, _t<std::decay<F>>>(
-      std::move(p), std::forward<F>(f));
+  return CoreCallbackState<T, F>(std::move(p), std::forward<F>(f));
+}
+
+template <typename T, typename R, typename... Args>
+auto makeCoreCallbackState(Promise<T>&& p, R (&f)(Args...)) noexcept {
+  return CoreCallbackState<T, R (*)(Args...)>(std::move(p), &f);
 }
 
 template <class T>

folly/futures/test/FutureTest.cpp

@@ -1116,10 +1116,11 @@ TEST(Future, invokeCallbackReturningValueAsRvalue) {
   Foo foo;
   Foo const cfoo;
 
-  // The callback will be copied when given as lvalue or const ref, and moved
-  // if provided as rvalue. Either way, it should be executed as rvalue.
-  EXPECT_EQ(103, makeFuture<int>(100).then(foo).value());
-  EXPECT_EQ(203, makeFuture<int>(200).then(cfoo).value());
+  // The continuation will be forward-constructed - copied if given as & and
+  // moved if given as && - everywhere construction is required.
+  // The continuation will be invoked with the same cvref as it is passed.
+  EXPECT_EQ(101, makeFuture<int>(100).then(foo).value());
+  EXPECT_EQ(202, makeFuture<int>(200).then(cfoo).value());
   EXPECT_EQ(303, makeFuture<int>(300).then(Foo()).value());
 }
 
@@ -1139,10 +1140,11 @@ TEST(Future, invokeCallbackReturningFutureAsRvalue) {
   Foo foo;
   Foo const cfoo;
 
-  // The callback will be copied when given as lvalue or const ref, and moved
-  // if provided as rvalue. Either way, it should be executed as rvalue.
-  EXPECT_EQ(103, makeFuture<int>(100).then(foo).value());
-  EXPECT_EQ(203, makeFuture<int>(200).then(cfoo).value());
+  // The continuation will be forward-constructed - copied if given as & and
+  // moved if given as && - everywhere construction is required.
+  // The continuation will be invoked with the same cvref as it is passed.
+  EXPECT_EQ(101, makeFuture<int>(100).then(foo).value());
+  EXPECT_EQ(202, makeFuture<int>(200).then(cfoo).value());
   EXPECT_EQ(303, makeFuture<int>(300).then(Foo()).value());
 }