From ff18deaf720fbe59551a7ff275b09003a61c4351 Mon Sep 17 00:00:00 2001 From: Lee Howes Date: Tue, 2 Jan 2018 20:29:49 -0800 Subject: [PATCH] Make consistent set of get and getTry methods on SemiFuture. Summary: Complete set of get and getVia methods including addition of a result operation on FutureBase that provides functionality of the old getTry on Future by returning a Try by reference. Reviewed By: yfeldblum Differential Revision: D6640056 fbshipit-source-id: 3ac01f7bc4b128e641f08d9a99280a18ffce82f9 --- folly/futures/Future-inl.h | 61 +++++++++++++++++------- folly/futures/Future.h | 16 ++++++- folly/futures/test/SemiFutureTest.cpp | 67 ++++++++++++++++++++++++++- 3 files changed, 122 insertions(+), 22 deletions(-) diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h index 25f8f464..56b98410 100644 --- a/folly/futures/Future-inl.h +++ b/folly/futures/Future-inl.h @@ -1,5 +1,5 @@ /* - * Copyright 2017 Facebook, Inc. + * Copyright 2017-present Facebook, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - #pragma once #include @@ -174,30 +173,50 @@ FutureBase::~FutureBase() { template T& FutureBase::value() & { + return result().value(); +} + +template +T const& FutureBase::value() const& { + return result().value(); +} + +template +T&& FutureBase::value() && { + return std::move(result().value()); +} + +template +T const&& FutureBase::value() const&& { + return std::move(result().value()); +} + +template +Try& FutureBase::result() & { throwIfInvalid(); - return core_->getTry().value(); + return core_->getTry(); } template -T const& FutureBase::value() const& { +Try const& FutureBase::result() const& { throwIfInvalid(); - return core_->getTry().value(); + return core_->getTry(); } template -T&& FutureBase::value() && { +Try&& FutureBase::result() && { throwIfInvalid(); - return std::move(core_->getTry().value()); + return std::move(core_->getTry()); } template -T const&& FutureBase::value() const&& { +Try const&& FutureBase::result() const&& { throwIfInvalid(); - return std::move(core_->getTry().value()); + return std::move(core_->getTry()); } template @@ -1443,7 +1462,7 @@ SemiFuture&& SemiFuture::waitVia(DrivableExecutor* e) && { template T SemiFuture::get() && { - return std::move(wait().value()); + return std::move(wait()).value(); } template @@ -1458,19 +1477,27 @@ T SemiFuture::get(Duration dur) && { template Try SemiFuture::getTry() && { - wait(); - return std::move(this->core_->getTry()); + return std::move(wait()).result(); +} + +template +Try SemiFuture::getTry(Duration dur) && { + wait(dur); + if (this->isReady()) { + return std::move(this->result()); + } else { + throwTimedOut(); + } } template T SemiFuture::getVia(DrivableExecutor* e) && { - return std::move(waitVia(e).value()); + return std::move(waitVia(e)).value(); } template Try SemiFuture::getTryVia(DrivableExecutor* e) && { - waitVia(e); - return std::move(this->core_->getTry()); + return std::move(waitVia(e)).result(); } template @@ -1526,9 +1553,7 @@ T Future::get(Duration dur) { template Try& Future::getTry() { - throwIfInvalid(); - - return this->core_->getTry(); + return result(); } template diff --git a/folly/futures/Future.h b/folly/futures/Future.h index 4818e1ed..5b2dc82f 100644 --- a/folly/futures/Future.h +++ b/folly/futures/Future.h @@ -1,5 +1,5 @@ /* - * Copyright 2017 Facebook, Inc. + * Copyright 2017-present Facebook, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - #pragma once #include @@ -97,6 +96,13 @@ class FutureBase { T&& value() &&; T const&& value() const&&; + /// Returns a reference to the try of the result. Throws as for value if + /// future is not valid. + Try& result() &; + Try const& result() const&; + Try&& result() &&; + Try const&& result() const&&; + /** True when the result (or exception) is ready. */ bool isReady() const; @@ -238,6 +244,7 @@ class SemiFuture : private futures::detail::FutureBase { using Base::raise; using Base::setCallback_; using Base::value; + using Base::result; SemiFuture& operator=(SemiFuture const&) = delete; SemiFuture& operator=(SemiFuture&&) noexcept; @@ -256,6 +263,10 @@ class SemiFuture : private futures::detail::FutureBase { /// Try of the value (moved out). Try getTry() &&; + /// Block until the future is fulfilled, or until timed out. Returns the + /// Try of the value (moved out) or may throw a TimedOut exception. + Try getTry(Duration dur) &&; + /// Call e->drive() repeatedly until the future is fulfilled. Examples /// of DrivableExecutor include EventBase and ManualExecutor. Returns the /// value (moved out), or throws the exception. @@ -419,6 +430,7 @@ class Future : private futures::detail::FutureBase { using Base::raise; using Base::setCallback_; using Base::value; + using Base::result; static Future makeEmpty(); // equivalent to moved-from diff --git a/folly/futures/test/SemiFutureTest.cpp b/folly/futures/test/SemiFutureTest.cpp index 1c44fe8d..36942645 100644 --- a/folly/futures/test/SemiFutureTest.cpp +++ b/folly/futures/test/SemiFutureTest.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2017 Facebook, Inc. + * Copyright 2017-present Facebook, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - #include #include #include @@ -264,6 +263,70 @@ TEST(SemiFuture, MakeFutureFromSemiFutureLValue) { ASSERT_EQ(result, 42); } +TEST(SemiFuture, SimpleGet) { + EventBase e2; + Promise p; + auto sf = p.getSemiFuture(); + p.setValue(3); + auto v = std::move(sf).get(); + ASSERT_EQ(v, 3); +} + +TEST(SemiFuture, SimpleGetTry) { + EventBase e2; + Promise p; + auto sf = p.getSemiFuture(); + p.setValue(3); + auto v = std::move(sf).getTry(); + ASSERT_EQ(v.value(), 3); +} + +TEST(SemiFuture, SimpleTimedGet) { + EventBase e2; + Promise p; + auto sf = p.getSemiFuture(); + EXPECT_THROW(std::move(sf).get(std::chrono::seconds(1)), TimedOut); +} + +TEST(SemiFuture, SimpleTimedGetTry) { + EventBase e2; + Promise p; + auto sf = p.getSemiFuture(); + EXPECT_THROW(std::move(sf).getTry(std::chrono::seconds(1)), TimedOut); +} + +TEST(SemiFuture, SimpleValue) { + EventBase e2; + Promise p; + auto sf = p.getSemiFuture(); + p.setValue(3); + auto v = std::move(sf).value(); + ASSERT_EQ(v, 3); +} + +TEST(SemiFuture, SimpleValueThrow) { + EventBase e2; + Promise p; + auto sf = p.getSemiFuture(); + EXPECT_THROW(std::move(sf).value(), FutureNotReady); +} + +TEST(SemiFuture, SimpleResult) { + EventBase e2; + Promise p; + auto sf = p.getSemiFuture(); + p.setValue(3); + auto v = std::move(sf).result(); + ASSERT_EQ(v.value(), 3); +} + +TEST(SemiFuture, SimpleResultThrow) { + EventBase e2; + Promise p; + auto sf = p.getSemiFuture(); + EXPECT_THROW(std::move(sf).result(), FutureNotReady); +} + TEST(SemiFuture, SimpleDefer) { std::atomic innerResult{0}; Promise p; -- 2.34.1