From d8bc4210d2feacbe01a24cd4fe2ad38a9c966dc8 Mon Sep 17 00:00:00 2001 From: Hans Fugal Date: Tue, 10 Feb 2015 12:32:04 -0800 Subject: [PATCH] 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 --- folly/futures/Future-inl.h | 4 ++-- folly/futures/Future.h | 15 +++++++++------ folly/futures/test/FutureTest.cpp | 4 ++-- folly/futures/test/Thens.cpp | 32 +++++++++++++++---------------- folly/futures/test/ViaTest.cpp | 2 +- folly/futures/test/thens.rb | 3 +-- 6 files changed, 31 insertions(+), 29 deletions(-) diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h index 57864681..a6f35c26 100644 --- a/folly/futures/Future-inl.h +++ b/folly/futures/Future-inl.h @@ -201,9 +201,9 @@ Future::thenImplementation(F func, detail::argResult) { } template -template +template Future::Inner> -Future::then(Caller *instance, R(Caller::*func)(Args...)) { +Future::then(R(Caller::*func)(Args...), Caller *instance) { typedef typename std::remove_cv< typename std::remove_reference< typename detail::ArgType::FirstArg>::type>::type FirstArg; diff --git a/folly/futures/Future.h b/folly/futures/Future.h index c018a9e7..1ac7e652 100644 --- 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&&); } + /// struct Worker { R doWork(Try); } /// /// Worker *w; - /// Future f2 = f1.then(w, &Worker::doWork); - template - Future::Inner> - then(Caller *instance, R(Caller::*func)(Args...)); + /// Future f2 = f1.then(&Worker::doWork, w); + /// + /// This is just sugar for + /// + /// f1.then(std::bind(&Worker::doWork, w)); + template + Future::Inner> + then(R(Caller::*func)(Args...), Caller *instance); /// Convenience method for ignoring the value and creating a Future. /// Exceptions still propagate. diff --git a/folly/futures/test/FutureTest.cpp b/folly/futures/test/FutureTest.cpp index bd8926ce..8d421285 100644 --- a/folly/futures/test/FutureTest.cpp +++ b/folly/futures/test/FutureTest.cpp @@ -378,7 +378,7 @@ TEST(Future, thenFunction) { auto f = makeFuture("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("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 716c27ad..3d136b56 100644 --- a/folly/futures/test/Thens.cpp +++ b/folly/futures/test/Thens.cpp @@ -8,83 +8,83 @@ TEST(Future, thenVariants) { {Future f = someFuture().then(&aFunction, Try&&>);} {Future f = someFuture().then(&SomeClass::aStaticMethod, Try&&>);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod, Try&&>);} + {Future f = someFuture().then(&SomeClass::aMethod, Try&&>, &anObject);} {Future f = someFuture().then(aStdFunction, Try&&>());} {Future f = someFuture().then([&](Try&&){return someFuture();});} {Future f = someFuture().then(&aFunction, Try const&>);} {Future f = someFuture().then(&SomeClass::aStaticMethod, Try const&>);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod, Try const&>);} + {Future f = someFuture().then(&SomeClass::aMethod, Try const&>, &anObject);} {Future f = someFuture().then(aStdFunction, Try const&>());} {Future f = someFuture().then([&](Try const&){return someFuture();});} {Future f = someFuture().then(&aFunction, Try>);} {Future f = someFuture().then(&SomeClass::aStaticMethod, Try>);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod, Try>);} + {Future f = someFuture().then(&SomeClass::aMethod, Try>, &anObject);} {Future f = someFuture().then(aStdFunction, Try>());} {Future f = someFuture().then([&](Try){return someFuture();});} {Future f = someFuture().then(&aFunction, Try&>);} {Future f = someFuture().then(&SomeClass::aStaticMethod, Try&>);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod, Try&>);} + {Future f = someFuture().then(&SomeClass::aMethod, Try&>, &anObject);} {Future f = someFuture().then(aStdFunction, Try&>());} {Future f = someFuture().then([&](Try&){return someFuture();});} {Future f = someFuture().then(&aFunction, A&&>);} {Future f = someFuture().then(&SomeClass::aStaticMethod, A&&>);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod, A&&>);} + {Future f = someFuture().then(&SomeClass::aMethod, A&&>, &anObject);} {Future f = someFuture().then(aStdFunction, A&&>());} {Future f = someFuture().then([&](A&&){return someFuture();});} {Future f = someFuture().then(&aFunction, A const&>);} {Future f = someFuture().then(&SomeClass::aStaticMethod, A const&>);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod, A const&>);} + {Future f = someFuture().then(&SomeClass::aMethod, A const&>, &anObject);} {Future f = someFuture().then(aStdFunction, A const&>());} {Future f = someFuture().then([&](A const&){return someFuture();});} {Future f = someFuture().then(&aFunction, A>);} {Future f = someFuture().then(&SomeClass::aStaticMethod, A>);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod, A>);} + {Future f = someFuture().then(&SomeClass::aMethod, A>, &anObject);} {Future f = someFuture().then(aStdFunction, A>());} {Future f = someFuture().then([&](A){return someFuture();});} {Future f = someFuture().then(&aFunction, A&>);} {Future f = someFuture().then(&SomeClass::aStaticMethod, A&>);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod, A&>);} + {Future f = someFuture().then(&SomeClass::aMethod, A&>, &anObject);} {Future f = someFuture().then(aStdFunction, A&>());} {Future f = someFuture().then([&](A&){return someFuture();});} {Future f = someFuture().then([&](){return someFuture();});} {Future f = someFuture().then(&aFunction&&>);} {Future f = someFuture().then(&SomeClass::aStaticMethod&&>);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod&&>);} + {Future f = someFuture().then(&SomeClass::aMethod&&>, &anObject);} {Future f = someFuture().then(aStdFunction&&>());} {Future f = someFuture().then([&](Try&&){return B();});} {Future f = someFuture().then(&aFunction const&>);} {Future f = someFuture().then(&SomeClass::aStaticMethod const&>);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod const&>);} + {Future f = someFuture().then(&SomeClass::aMethod const&>, &anObject);} {Future f = someFuture().then(aStdFunction const&>());} {Future f = someFuture().then([&](Try const&){return B();});} {Future f = someFuture().then(&aFunction>);} {Future f = someFuture().then(&SomeClass::aStaticMethod>);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod>);} + {Future f = someFuture().then(&SomeClass::aMethod>, &anObject);} {Future f = someFuture().then(aStdFunction>());} {Future f = someFuture().then([&](Try){return B();});} {Future f = someFuture().then(&aFunction&>);} {Future f = someFuture().then(&SomeClass::aStaticMethod&>);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod&>);} + {Future f = someFuture().then(&SomeClass::aMethod&>, &anObject);} {Future f = someFuture().then(aStdFunction&>());} {Future f = someFuture().then([&](Try&){return B();});} {Future f = someFuture().then(&aFunction);} {Future f = someFuture().then(&SomeClass::aStaticMethod);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod);} + {Future f = someFuture().then(&SomeClass::aMethod, &anObject);} {Future f = someFuture().then(aStdFunction());} {Future f = someFuture().then([&](A&&){return B();});} {Future f = someFuture().then(&aFunction);} {Future f = someFuture().then(&SomeClass::aStaticMethod);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod);} + {Future f = someFuture().then(&SomeClass::aMethod, &anObject);} {Future f = someFuture().then(aStdFunction());} {Future f = someFuture().then([&](A const&){return B();});} {Future f = someFuture().then(&aFunction);} {Future f = someFuture().then(&SomeClass::aStaticMethod);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod);} + {Future f = someFuture().then(&SomeClass::aMethod, &anObject);} {Future f = someFuture().then(aStdFunction());} {Future f = someFuture().then([&](A){return B();});} {Future f = someFuture().then(&aFunction);} {Future f = someFuture().then(&SomeClass::aStaticMethod);} - {Future f = someFuture().then(&anObject, &SomeClass::aMethod);} + {Future f = someFuture().then(&SomeClass::aMethod, &anObject);} {Future f = someFuture().then(aStdFunction());} {Future f = someFuture().then([&](A&){return B();});} {Future f = someFuture().then([&](){return B();});} diff --git a/folly/futures/test/ViaTest.cpp b/folly/futures/test/ViaTest.cpp index 5cb66827..4a78ce7d 100644 --- 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 fa73a8e7..4eeffd89 100755 --- 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)};}"], ] -- 2.34.1