switch order of method/object in Future::then to match std::bind

Summary:
I have half a mind to just rip this out and let people use std::bind if they need this. But I won't be so cruel.

Why isn't this just implemented as `then(std::bind(method, object))` anyway? Is the template soup we have now faster?

Test Plan:
Fixed the unit tests to use the new format.
Will look to contbuild to catch all the things this might break (if anyone is using it at all?), and will fix them.

Reviewed By: hannesr@fb.com

Subscribers: trunkagent, exa, folly-diffs@, yfeldblum, jsedgwick, davejwatson

FB internal diff: D1831118

Signature: t1:1831118:1423243771:65db9a89daf14d8bd88331c503ba1ea7ab03b679

diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h
index 578646819fd1033692952482b7932054e31abac8..a6f35c26643fdfbbc9dacffdca5d3dd7c179d464 100644 (file)
--- a/folly/futures/Future-inl.h
+++ b/folly/futures/Future-inl.h
@@ -201,9 +201,9 @@ Future<T>::thenImplementation(F func, detail::argResult<isTry, F, Args...>) {
 }
 
 template <typename T>
-template <typename Caller, typename R, typename... Args>
+template <typename R, typename Caller, typename... Args>
   Future<typename isFuture<R>::Inner>
-Future<T>::then(Caller *instance, R(Caller::*func)(Args...)) {
+Future<T>::then(R(Caller::*func)(Args...), Caller *instance) {
   typedef typename std::remove_cv<
     typename std::remove_reference<
       typename detail::ArgType<Args...>::FirstArg>::type>::type FirstArg;

diff --git a/folly/futures/Future.h b/folly/futures/Future.h
index c018a9e7806f7514b7a1ac57be3e416807478b6b..1ac7e652d19eb426fb0749cf62bb5b7db5e4c0a5 100644 (file)
--- a/folly/futures/Future.h
+++ b/folly/futures/Future.h
@@ -299,14 +299,17 @@ class Future {
 
   /// Variant where func is an member function
   ///
-  ///   struct Worker {
-  ///     R doWork(Try<T>&&); }
+  ///   struct Worker { R doWork(Try<T>); }
   ///
   ///   Worker *w;
-  ///   Future<R> f2 = f1.then(w, &Worker::doWork);
-  template <typename Caller, typename R, typename... Args>
-    Future<typename isFuture<R>::Inner>
-  then(Caller *instance, R(Caller::*func)(Args...));
+  ///   Future<R> f2 = f1.then(&Worker::doWork, w);
+  ///
+  /// This is just sugar for
+  ///
+  ///   f1.then(std::bind(&Worker::doWork, w));
+  template <typename R, typename Caller, typename... Args>
+  Future<typename isFuture<R>::Inner>
+  then(R(Caller::*func)(Args...), Caller *instance);
 
   /// Convenience method for ignoring the value and creating a Future<void>.
   /// Exceptions still propagate.

diff --git a/folly/futures/test/FutureTest.cpp b/folly/futures/test/FutureTest.cpp
index bd8926ce06a171e13de2726f59a66e4817e4a4a1..8d42128596f4f8cc87b36c62b47819b97501cf22 100644 (file)
--- a/folly/futures/test/FutureTest.cpp
+++ b/folly/futures/test/FutureTest.cpp
@@ -378,7 +378,7 @@ TEST(Future, thenFunction) {
   auto f = makeFuture<string>("start")
     .then(doWorkStatic)
     .then(Worker::doWorkStatic)
-    .then(&w, &Worker::doWork);
+    .then(&Worker::doWork, &w);
 
   EXPECT_EQ(f.value(), "start;static;class-static;class");
 }
@@ -400,7 +400,7 @@ TEST(Future, thenFunctionFuture) {
   auto f = makeFuture<string>("start")
     .then(doWorkStaticFuture)
     .then(Worker::doWorkStaticFuture)
-    .then(&w, &Worker::doWorkFuture);
+    .then(&Worker::doWorkFuture, &w);
 
   EXPECT_EQ(f.value(), "start;static;class-static;class");
 }

diff --git a/folly/futures/test/Thens.cpp b/folly/futures/test/Thens.cpp
index 716c27ad66262879ab6a650f8105841546470c52..3d136b56ed63b7d6c22574b90d282b555f55b460 100644 (file)
--- a/folly/futures/test/Thens.cpp
+++ b/folly/futures/test/Thens.cpp
@@ -8,83 +8,83 @@ TEST(Future, thenVariants) {
 
   {Future<B> f = someFuture<A>().then(&aFunction<Future<B>, Try<A>&&>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<Future<B>, Try<A>&&>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<Future<B>, Try<A>&&>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<Future<B>, Try<A>&&>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<Future<B>, Try<A>&&>());}
   {Future<B> f = someFuture<A>().then([&](Try<A>&&){return someFuture<B>();});}
   {Future<B> f = someFuture<A>().then(&aFunction<Future<B>, Try<A> const&>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<Future<B>, Try<A> const&>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<Future<B>, Try<A> const&>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<Future<B>, Try<A> const&>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<Future<B>, Try<A> const&>());}
   {Future<B> f = someFuture<A>().then([&](Try<A> const&){return someFuture<B>();});}
   {Future<B> f = someFuture<A>().then(&aFunction<Future<B>, Try<A>>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<Future<B>, Try<A>>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<Future<B>, Try<A>>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<Future<B>, Try<A>>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<Future<B>, Try<A>>());}
   {Future<B> f = someFuture<A>().then([&](Try<A>){return someFuture<B>();});}
   {Future<B> f = someFuture<A>().then(&aFunction<Future<B>, Try<A>&>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<Future<B>, Try<A>&>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<Future<B>, Try<A>&>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<Future<B>, Try<A>&>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<Future<B>, Try<A>&>());}
   {Future<B> f = someFuture<A>().then([&](Try<A>&){return someFuture<B>();});}
   {Future<B> f = someFuture<A>().then(&aFunction<Future<B>, A&&>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<Future<B>, A&&>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<Future<B>, A&&>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<Future<B>, A&&>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<Future<B>, A&&>());}
   {Future<B> f = someFuture<A>().then([&](A&&){return someFuture<B>();});}
   {Future<B> f = someFuture<A>().then(&aFunction<Future<B>, A const&>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<Future<B>, A const&>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<Future<B>, A const&>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<Future<B>, A const&>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<Future<B>, A const&>());}
   {Future<B> f = someFuture<A>().then([&](A const&){return someFuture<B>();});}
   {Future<B> f = someFuture<A>().then(&aFunction<Future<B>, A>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<Future<B>, A>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<Future<B>, A>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<Future<B>, A>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<Future<B>, A>());}
   {Future<B> f = someFuture<A>().then([&](A){return someFuture<B>();});}
   {Future<B> f = someFuture<A>().then(&aFunction<Future<B>, A&>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<Future<B>, A&>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<Future<B>, A&>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<Future<B>, A&>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<Future<B>, A&>());}
   {Future<B> f = someFuture<A>().then([&](A&){return someFuture<B>();});}
   {Future<B> f = someFuture<A>().then([&](){return someFuture<B>();});}
   {Future<B> f = someFuture<A>().then(&aFunction<B, Try<A>&&>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<B, Try<A>&&>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<B, Try<A>&&>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<B, Try<A>&&>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<B, Try<A>&&>());}
   {Future<B> f = someFuture<A>().then([&](Try<A>&&){return B();});}
   {Future<B> f = someFuture<A>().then(&aFunction<B, Try<A> const&>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<B, Try<A> const&>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<B, Try<A> const&>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<B, Try<A> const&>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<B, Try<A> const&>());}
   {Future<B> f = someFuture<A>().then([&](Try<A> const&){return B();});}
   {Future<B> f = someFuture<A>().then(&aFunction<B, Try<A>>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<B, Try<A>>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<B, Try<A>>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<B, Try<A>>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<B, Try<A>>());}
   {Future<B> f = someFuture<A>().then([&](Try<A>){return B();});}
   {Future<B> f = someFuture<A>().then(&aFunction<B, Try<A>&>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<B, Try<A>&>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<B, Try<A>&>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<B, Try<A>&>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<B, Try<A>&>());}
   {Future<B> f = someFuture<A>().then([&](Try<A>&){return B();});}
   {Future<B> f = someFuture<A>().then(&aFunction<B, A&&>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<B, A&&>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<B, A&&>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<B, A&&>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<B, A&&>());}
   {Future<B> f = someFuture<A>().then([&](A&&){return B();});}
   {Future<B> f = someFuture<A>().then(&aFunction<B, A const&>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<B, A const&>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<B, A const&>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<B, A const&>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<B, A const&>());}
   {Future<B> f = someFuture<A>().then([&](A const&){return B();});}
   {Future<B> f = someFuture<A>().then(&aFunction<B, A>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<B, A>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<B, A>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<B, A>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<B, A>());}
   {Future<B> f = someFuture<A>().then([&](A){return B();});}
   {Future<B> f = someFuture<A>().then(&aFunction<B, A&>);}
   {Future<B> f = someFuture<A>().then(&SomeClass::aStaticMethod<B, A&>);}
-  {Future<B> f = someFuture<A>().then(&anObject, &SomeClass::aMethod<B, A&>);}
+  {Future<B> f = someFuture<A>().then(&SomeClass::aMethod<B, A&>, &anObject);}
   {Future<B> f = someFuture<A>().then(aStdFunction<B, A&>());}
   {Future<B> f = someFuture<A>().then([&](A&){return B();});}
   {Future<B> f = someFuture<A>().then([&](){return B();});}

diff --git a/folly/futures/test/ViaTest.cpp b/folly/futures/test/ViaTest.cpp
index 5cb66827879122038f8b007a75c5eb35de37fe2d..4a78ce7dbc0e83367d817aa20f2096ce0a541c69 100644 (file)
--- a/folly/futures/test/ViaTest.cpp
+++ b/folly/futures/test/ViaTest.cpp
@@ -114,7 +114,7 @@ TEST(Via, then_function) {
   auto f = makeFuture(std::string("start"))
     .then(doWorkStatic)
     .then(Worker::doWorkStatic)
-    .then(&w, &Worker::doWork)
+    .then(&Worker::doWork, &w)
     ;
 
   EXPECT_EQ(f.value(), "start;static;class-static;class");

diff --git a/folly/futures/test/thens.rb b/folly/futures/test/thens.rb
index fa73a8e723aadcfc1434c7338ea3876fecf8fd0b..4eeffd8979472e37a3cae05e7869209c2d413b94 100755 (executable)
--- a/folly/futures/test/thens.rb
+++ b/folly/futures/test/thens.rb
@@ -50,8 +50,7 @@ tests = (
         [
           ["&aFunction<#{both}>"],
           ["&SomeClass::aStaticMethod<#{both}>"],
-          # TODO switch these around (std::bind-style)
-          ["&anObject", "&SomeClass::aMethod<#{both}>"],
+          ["&SomeClass::aMethod<#{both}>", "&anObject"],
           ["aStdFunction<#{both}>()"],
           ["[&](#{param}){return #{retval(ret)};}"],
         ]