From 449e25960a2c635eea8fe2be9a8f090dccdfcb7e Mon Sep 17 00:00:00 2001 From: James Sedgwick Date: Wed, 29 Oct 2014 16:33:14 -0700 Subject: [PATCH] Future::then([](T&&)), aka next() Summary: variants of then() that bypass Try and forward any exceptions up to the next future. like next() from http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n3865.pdf this could go a long way towards reducing spurious rethrows wherever people don't ever actually catch exceptions in their continuations, which is probably often enough. anyone know if there's something in folly or standard library that does what my silly VoidWrapper struct does? there's a lot of copypasta here but i'm not sure consolidating into helpers would actually be useful considering the amount of template crap, i don't feel strongly about it though. Test Plan: added unit Reviewed By: hans@fb.com Subscribers: trunkagent, sammerat, fugalh, njormrod, folly-diffs@, bmatheny FB internal diff: D1641776 Signature: t1:1641776:1414610434:c742563b8061a748fca9292fc2765081edcf9d52 --- folly/wangle/Future-inl.h | 137 +++++++++++++++++++++++++++++++ folly/wangle/Future.h | 104 +++++++++++++++++++++++ folly/wangle/Try.h | 18 ++++ folly/wangle/test/FutureTest.cpp | 40 ++++++++- folly/wangle/test/Thens.cpp | 19 ++++- folly/wangle/test/thens.rb | 7 +- 6 files changed, 319 insertions(+), 6 deletions(-) diff --git a/folly/wangle/Future-inl.h b/folly/wangle/Future-inl.h index f462189c..6dd2125c 100644 --- a/folly/wangle/Future-inl.h +++ b/folly/wangle/Future-inl.h @@ -71,6 +71,7 @@ void Future::setCallback_(F&& func) { core_->setCallback(std::move(func)); } +// Variant: f.then([](Try&& t){ return t.value(); }); template template typename std::enable_if< @@ -130,6 +131,69 @@ Future::then(F&& func) { return std::move(f); } +// Variant: f.then([](T&& t){ return t; }); +template +template +typename std::enable_if< + !std::is_same::value && + !isFuture::type&&)>::type>::value, + Future::type&&)>::type> >::type +Future::then(F&& func) { + typedef typename std::result_of::type B; + + throwIfInvalid(); + + folly::MoveWrapper> p; + folly::MoveWrapper funcm(std::forward(func)); + auto f = p->getFuture(); + + setCallback_( + [p, funcm](Try&& t) mutable { + if (t.hasException()) { + p->setException(t.getException()); + } else { + p->fulfil([&]() { + return (*funcm)(std::move(t.value())); + }); + } + }); + + return std::move(f); +} + +// Variant: f.then([](){ return; }); +template +template +typename std::enable_if< + std::is_same::value && + !isFuture::type>::value, + Future::type> >::type +Future::then(F&& func) { + typedef typename std::result_of::type B; + + throwIfInvalid(); + + folly::MoveWrapper> p; + folly::MoveWrapper funcm(std::forward(func)); + auto f = p->getFuture(); + + setCallback_( + [p, funcm](Try&& t) mutable { + if (t.hasException()) { + p->setException(t.getException()); + } else { + p->fulfil([&]() { + return (*funcm)(); + }); + } + }); + + return std::move(f); +} + +// Variant: f.then([](Try&& t){ return makeFuture(t.value()); }); template template typename std::enable_if< @@ -163,6 +227,79 @@ Future::then(F&& func) { return std::move(f); } +// Variant: f.then([](T&& t){ return makeFuture(t); }); +template +template +typename std::enable_if< + !std::is_same::value && + isFuture::type&&)>::type>::value, + Future::type&&)>::type::value_type> >::type +Future::then(F&& func) { + typedef typename std::result_of::type::value_type B; + + throwIfInvalid(); + + folly::MoveWrapper> p; + folly::MoveWrapper funcm(std::forward(func)); + auto f = p->getFuture(); + + setCallback_( + [p, funcm](Try&& t) mutable { + if (t.hasException()) { + p->setException(t.getException()); + } else { + try { + auto f2 = (*funcm)(std::move(t.value())); + f2.setCallback_([p](Try&& b) mutable { + p->fulfilTry(std::move(b)); + }); + } catch (...) { + p->setException(std::current_exception()); + } + } + }); + + return std::move(f); +} + +// Variant: f.then([](){ return makeFuture(); }); +template +template +typename std::enable_if< + std::is_same::value && + isFuture::type>::value, + Future::type::value_type> >::type +Future::then(F&& func) { + typedef typename std::result_of::type::value_type B; + + throwIfInvalid(); + + folly::MoveWrapper> p; + folly::MoveWrapper funcm(std::forward(func)); + + auto f = p->getFuture(); + + setCallback_( + [p, funcm](Try&& t) mutable { + if (t.hasException()) { + p->setException(t.getException()); + } else { + try { + auto f2 = (*funcm)(); + f2.setCallback_([p](Try&& b) mutable { + p->fulfilTry(std::move(b)); + }); + } catch (...) { + p->setException(std::current_exception()); + } + } + }); + + return std::move(f); +} + template Future Future::then() { return then([] (Try&& t) {}); diff --git a/folly/wangle/Future.h b/folly/wangle/Future.h index f3d50ce0..c01aff5c 100644 --- a/folly/wangle/Future.h +++ b/folly/wangle/Future.h @@ -32,7 +32,16 @@ namespace folly { namespace wangle { namespace detail { template struct Core; template struct VariadicContext; + + template + struct AliasIfVoid { + typedef typename std::conditional< + std::is_same::value, + int, + T>::type type; + }; } + template struct Promise; template struct isFuture; @@ -113,6 +122,30 @@ class Future { Future&&)>::type> >::type then(F&& func); + /// Variant where func takes a T directly, bypassing a try. Any exceptions + /// will be implicitly passed on to the resultant Future. + /// + /// Future f = makeFuture(42).then([](int i) { return i+1; }); + template + typename std::enable_if< + !std::is_same::value && + !isFuture::type&&)>::type>::value, + Future::type&&)>::type> >::type + then(F&& func); + + /// Like the above variant, but for void futures. That is, func takes no + /// argument. + /// + /// Future f = makeFuture().then([] { return 42; }); + template + typename std::enable_if< + std::is_same::value && + !isFuture::type>::value, + Future::type> >::type + then(F&& func); + /// Variant where func returns a Future instead of a T. e.g. /// /// Future f2 = f1.then( @@ -123,6 +156,33 @@ class Future { Future&&)>::type::value_type> >::type then(F&& func); + /// Variant where func returns a Future and takes a T directly, bypassing + /// a Try. Any exceptions will be implicitly passed on to the resultant + /// Future. For example, + /// + /// Future f = makeFuture(42).then( + /// [](int i) { return makeFuture(i+1); }); + template + typename std::enable_if< + !std::is_same::value && + isFuture::type&&)>::type>::value, + Future::type&&)>::type::value_type> >::type + then(F&& func); + + /// Like the above variant, but for void futures. That is, func takes no + /// argument and returns a future. + /// + /// Future f = makeFuture().then( + /// [] { return makeFuture(42); }); + template + typename std::enable_if< + std::is_same::value && + isFuture::type>::value, + Future::type::value_type> >::type + then(F&& func); + /// Variant where func is an ordinary function (static method, method) /// /// R doWork(Try&&); @@ -172,6 +232,28 @@ class Future { }); } + // Same as above, but func takes void instead of Try&& + template + typename std::enable_if< + std::is_same::value && !isFuture::value, Future>::type + inline then(Caller *instance, R(Caller::*func)()) { + return then([instance, func]() { + return (instance->*func)(); + }); + } + + // Same as above, but func takes T&& instead of Try&& + template + typename std::enable_if< + !std::is_same::value && !isFuture::value, Future>::type + inline then( + Caller *instance, + R(Caller::*func)(typename detail::AliasIfVoid::type&&)) { + return then([instance, func](T&& t) { + return (instance->*func)(std::move(t)); + }); + } + /// Variant where func returns a Future instead of a R. e.g. /// /// struct Worker { @@ -187,6 +269,28 @@ class Future { }); } + // Same as above, but func takes void instead of Try&& + template + typename std::enable_if< + std::is_same::value && isFuture::value, R>::type + inline then(Caller *instance, R(Caller::*func)()) { + return then([instance, func]() { + return (instance->*func)(); + }); + } + + // Same as above, but func takes T&& instead of Try&& + template + typename std::enable_if< + !std::is_same::value && isFuture::value, R>::type + inline then( + Caller *instance, + R(Caller::*func)(typename detail::AliasIfVoid::type&&)) { + return then([instance, func](T&& t) { + return (instance->*func)(std::move(t)); + }); + } + /// Convenience method for ignoring the value and creating a Future. /// Exceptions still propagate. Future then(); diff --git a/folly/wangle/Try.h b/folly/wangle/Try.h index 68d3ef92..bc7800d8 100644 --- a/folly/wangle/Try.h +++ b/folly/wangle/Try.h @@ -19,6 +19,8 @@ #include #include #include +#include +#include namespace folly { namespace wangle { @@ -65,6 +67,14 @@ class Try { bool hasValue() const { return contains_ == Contains::VALUE; } bool hasException() const { return contains_ == Contains::EXCEPTION; } + std::exception_ptr getException() const { + if (UNLIKELY(!hasException())) { + throw WangleException( + "getException(): Try does not contain an exception"); + } + return e_; + } + private: Contains contains_; union { @@ -87,6 +97,14 @@ class Try { bool hasValue() const { return hasValue_; } bool hasException() const { return !hasValue_; } + std::exception_ptr getException() const { + if (UNLIKELY(!hasException())) { + throw WangleException( + "getException(): Try does not contain an exception"); + } + return e_; + } + private: bool hasValue_; std::exception_ptr e_; diff --git a/folly/wangle/test/FutureTest.cpp b/folly/wangle/test/FutureTest.cpp index 0feaa50f..8d1f95d3 100644 --- a/folly/wangle/test/FutureTest.cpp +++ b/folly/wangle/test/FutureTest.cpp @@ -69,7 +69,7 @@ TEST(Future, special) { EXPECT_TRUE(std::is_move_assignable>::value); } -TEST(Future, then) { +TEST(Future, thenTry) { bool flag = false; makeFuture(42).then([&](Try&& t) { @@ -95,6 +95,44 @@ TEST(Future, then) { EXPECT_TRUE(f.isReady()); } +TEST(Future, thenValue) { + bool flag = false; + makeFuture(42).then([&](int i){ + EXPECT_EQ(42, i); + flag = true; + }); + EXPECT_TRUE(flag); flag = false; + + makeFuture(42) + .then([](int i){ return i; }) + .then([&](int i) { flag = true; EXPECT_EQ(42, i); }); + EXPECT_TRUE(flag); flag = false; + + makeFuture().then([&]{ + flag = true; + }); + EXPECT_TRUE(flag); flag = false; + + auto f = makeFuture(eggs).then([&](int i){}); + EXPECT_THROW(f.value(), eggs_t); + + f = makeFuture(eggs).then([&]{}); + EXPECT_THROW(f.value(), eggs_t); +} + +TEST(Future, thenValueFuture) { + bool flag = false; + makeFuture(42) + .then([](int i){ return makeFuture(std::move(i)); }) + .then([&](Try&& t) { flag = true; EXPECT_EQ(42, t.value()); }); + EXPECT_TRUE(flag); flag = false; + + makeFuture() + .then([]{ return makeFuture(); }) + .then([&](Try&& t) { flag = true; }); + EXPECT_TRUE(flag); flag = false; +} + static string doWorkStatic(Try&& t) { return t.value() + ";static"; } diff --git a/folly/wangle/test/Thens.cpp b/folly/wangle/test/Thens.cpp index a0e1be32..2a9cdca1 100644 --- a/folly/wangle/test/Thens.cpp +++ b/folly/wangle/test/Thens.cpp @@ -1,24 +1,37 @@ -// This file is @generated by thens.rb +// This file is @generated by thens.rb. Do not edit directly. // TODO: fails to compile with clang:dev. See task #4412111 #ifndef __clang__ #include +#ifndef __clang__ +// TODO: fails to compile with clang:dev. See task #4412111 + TEST(Future, thenVariants) { SomeClass anObject; Executor* anExecutor; - {Future f = someFuture().then(aFunction, Try&&>);} + {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(aStdFunction, Try&&>());} {Future f = someFuture().then([&](Try&&){return someFuture();});} - {Future f = someFuture().then(aFunction&&>);} + {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(aStdFunction, A&&>());} + {Future f = someFuture().then([&](A&&){return someFuture();});} + {Future f = someFuture().then(&aFunction&&>);} {Future f = someFuture().then(&SomeClass::aStaticMethod&&>);} {Future f = someFuture().then(&anObject, &SomeClass::aMethod&&>);} {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(aStdFunction());} + {Future f = someFuture().then([&](A&&){return B();});} } #endif diff --git a/folly/wangle/test/thens.rb b/folly/wangle/test/thens.rb index e45b2c31..7de0e4cc 100755 --- a/folly/wangle/test/thens.rb +++ b/folly/wangle/test/thens.rb @@ -35,7 +35,7 @@ param_types = [ #"Try const&", #"Try", #"Try&", - #"A&&", + "A&&", #"A const&", #"A", #"A&", @@ -47,7 +47,7 @@ tests = ( param_types.map { |param| both = "#{ret}, #{param}" [ - ["aFunction<#{both}>"], + ["&aFunction<#{both}>"], ["&SomeClass::aStaticMethod<#{both}>"], # TODO switch these around (std::bind-style) ["&anObject", "&SomeClass::aMethod<#{both}>"], @@ -68,6 +68,9 @@ print < +#ifndef __clang__ +// TODO: fails to compile with clang:dev. See task #4412111 + TEST(Future, thenVariants) { SomeClass anObject; Executor* anExecutor; -- 2.34.1