From 1d42dc0bff8e4701c9b04a9b5e3717d5d7aef664 Mon Sep 17 00:00:00 2001 From: Hans Fugal Date: Mon, 30 Jun 2014 13:22:56 -0700 Subject: [PATCH] (wangle) s/FutureObject/detail::State/ Summary: codemod I never was satisfied with this name. `State` feels a little better. More descriptive. But still pretty generic, so I'm open to alternative ideas. Test Plan: It still builds and tests pass. Reviewed By: hannesr@fb.com Subscribers: jsedgwick, net-systems@, fugalh, exa FB internal diff: D1411506 --- folly/wangle/Future-inl.h | 32 +++++++++--------- folly/wangle/Future.h | 6 ++-- folly/wangle/Promise-inl.h | 40 +++++++++++------------ folly/wangle/Promise.h | 4 +-- folly/wangle/{detail.h => detail/State.h} | 21 ++++++------ 5 files changed, 52 insertions(+), 51 deletions(-) rename folly/wangle/{detail.h => detail/State.h} (92%) diff --git a/folly/wangle/Future-inl.h b/folly/wangle/Future-inl.h index aa45cb26..27af013e 100644 --- a/folly/wangle/Future-inl.h +++ b/folly/wangle/Future-inl.h @@ -16,7 +16,7 @@ #pragma once -#include "detail.h" +#include "detail/State.h" #include namespace folly { namespace wangle { @@ -32,26 +32,26 @@ struct isFuture > { }; template -Future::Future(Future&& other) noexcept : obj_(other.obj_) { - other.obj_ = nullptr; +Future::Future(Future&& other) noexcept : state_(other.state_) { + other.state_ = nullptr; } template Future& Future::operator=(Future&& other) { - std::swap(obj_, other.obj_); + std::swap(state_, other.state_); return *this; } template Future::~Future() { - if (obj_) { + if (state_) { setCallback_([](Try&&) {}); // detach } } template void Future::throwIfInvalid() const { - if (!obj_) + if (!state_) throw NoState(); } @@ -59,8 +59,8 @@ template template void Future::setCallback_(F&& func) { throwIfInvalid(); - obj_->setCallback_(std::move(func)); - obj_ = nullptr; + state_->setCallback_(std::move(func)); + state_ = nullptr; } template @@ -87,10 +87,10 @@ Future::then(F&& func) { sophisticated that avoids making a new Future object when it can, as an optimization. But this is correct. - obj_ can't be moved, it is explicitly disallowed (as is copying). But + state_ can't be moved, it is explicitly disallowed (as is copying). But if there's ever a reason to allow it, this is one place that makes that assumption and would need to be fixed. We use a standard shared pointer - for obj_ (by copying it in), which means in essence obj holds a shared + for state_ (by copying it in), which means in essence obj holds a shared pointer to itself. But this shouldn't leak because Promise will not outlive the continuation, because Promise will setException() with a broken Promise if it is destructed before completed. We could use a @@ -102,11 +102,11 @@ Future::then(F&& func) { We have to move in the Promise and func using the MoveWrapper hack. (func could be copied but it's a big drag on perf). - Two subtle but important points about this design. FutureObject has no + Two subtle but important points about this design. detail::State has no back pointers to Future or Promise, so if Future or Promise get moved (and they will be moved in performant code) we don't have to do anything fancy. And because we store the continuation in the - FutureObject, not in the Future, we can execute the continuation even + detail::State, not in the Future, we can execute the continuation even after the Future has gone out of scope. This is an intentional design decision. It is likely we will want to be able to cancel a continuation in some circumstances, but I think it should be explicit not implicit @@ -164,21 +164,21 @@ template typename std::add_lvalue_reference::type Future::value() { throwIfInvalid(); - return obj_->value(); + return state_->value(); } template typename std::add_lvalue_reference::type Future::value() const { throwIfInvalid(); - return obj_->value(); + return state_->value(); } template Try& Future::getTry() { throwIfInvalid(); - return obj_->getTry(); + return state_->getTry(); } template @@ -191,7 +191,7 @@ inline Later Future::via(Executor* executor) { template bool Future::isReady() const { throwIfInvalid(); - return obj_->ready(); + return state_->ready(); } // makeFuture diff --git a/folly/wangle/Future.h b/folly/wangle/Future.h index 6a75f829..fcec4353 100644 --- a/folly/wangle/Future.h +++ b/folly/wangle/Future.h @@ -181,13 +181,13 @@ class Future { void setCallback_(F&& func); private: - typedef detail::FutureObject* objPtr; + typedef detail::State* statePtr; // shared state object - objPtr obj_; + statePtr state_; explicit - Future(objPtr obj) : obj_(obj) {} + Future(statePtr obj) : state_(obj) {} void throwIfInvalid() const; diff --git a/folly/wangle/Promise-inl.h b/folly/wangle/Promise-inl.h index 49060270..190d9cd0 100644 --- a/folly/wangle/Promise-inl.h +++ b/folly/wangle/Promise-inl.h @@ -20,30 +20,30 @@ #include #include "WangleException.h" -#include "detail.h" +#include "detail/State.h" namespace folly { namespace wangle { template -Promise::Promise() : retrieved_(false), obj_(new detail::FutureObject()) +Promise::Promise() : retrieved_(false), state_(new detail::State()) {} template Promise::Promise(Promise&& other) : -retrieved_(other.retrieved_), obj_(other.obj_) { - other.obj_ = nullptr; +retrieved_(other.retrieved_), state_(other.state_) { + other.state_ = nullptr; } template Promise& Promise::operator=(Promise&& other) { - std::swap(obj_, other.obj_); + std::swap(state_, other.state_); std::swap(retrieved_, other.retrieved_); return *this; } template void Promise::throwIfFulfilled() { - if (!obj_) + if (!state_) throw PromiseAlreadySatisfied(); } @@ -55,7 +55,7 @@ void Promise::throwIfRetrieved() { template Promise::~Promise() { - if (obj_) { + if (state_) { setException(BrokenPromise()); } } @@ -65,7 +65,7 @@ Future Promise::getFuture() { throwIfRetrieved(); throwIfFulfilled(); retrieved_ = true; - return Future(obj_); + return Future(state_); } template @@ -78,21 +78,21 @@ void Promise::setException(E const& e) { template void Promise::setException(std::exception_ptr const& e) { throwIfFulfilled(); - obj_->setException(e); + state_->setException(e); if (!retrieved_) { - delete obj_; + delete state_; } - obj_ = nullptr; + state_ = nullptr; } template void Promise::fulfilTry(Try&& t) { throwIfFulfilled(); - obj_->fulfil(std::move(t)); + state_->fulfil(std::move(t)); if (!retrieved_) { - delete obj_; + delete state_; } - obj_ = nullptr; + state_ = nullptr; } template @@ -102,11 +102,11 @@ void Promise::setValue(M&& v) { "Use setValue() instead"); throwIfFulfilled(); - obj_->fulfil(Try(std::forward(v))); + state_->fulfil(Try(std::forward(v))); if (!retrieved_) { - delete obj_; + delete state_; } - obj_ = nullptr; + state_ = nullptr; } template @@ -115,11 +115,11 @@ void Promise::setValue() { "Use setValue(value) instead"); throwIfFulfilled(); - obj_->fulfil(Try()); + state_->fulfil(Try()); if (!retrieved_) { - delete obj_; + delete state_; } - obj_ = nullptr; + state_ = nullptr; } template diff --git a/folly/wangle/Promise.h b/folly/wangle/Promise.h index aae306c0..c47cc98e 100644 --- a/folly/wangle/Promise.h +++ b/folly/wangle/Promise.h @@ -76,13 +76,13 @@ public: void fulfil(F&& func); private: - typedef typename Future::objPtr objPtr; + typedef typename Future::statePtr statePtr; // Whether the Future has been retrieved (a one-time operation). bool retrieved_; // shared state object - objPtr obj_; + statePtr state_; void throwIfFulfilled(); void throwIfRetrieved(); diff --git a/folly/wangle/detail.h b/folly/wangle/detail/State.h similarity index 92% rename from folly/wangle/detail.h rename to folly/wangle/detail/State.h index 105ebc3a..ccba9acd 100644 --- a/folly/wangle/detail.h +++ b/folly/wangle/detail/State.h @@ -17,29 +17,30 @@ #pragma once #include -#include #include #include -#include "Try.h" -#include "Promise.h" -#include "Future.h" +#include + +#include +#include +#include namespace folly { namespace wangle { namespace detail { /** The shared state object for Future and Promise. */ template -class FutureObject { +class State { public: - FutureObject() = default; + State() = default; // not copyable - FutureObject(FutureObject const&) = delete; - FutureObject& operator=(FutureObject const&) = delete; + State(State const&) = delete; + State& operator=(State const&) = delete; // not movable (see comment in the implementation of Future::then) - FutureObject(FutureObject&&) noexcept = delete; - FutureObject& operator=(FutureObject&&) = delete; + State(State&&) noexcept = delete; + State& operator=(State&&) = delete; Try& getTry() { return *value_; -- 2.34.1