From ff25b9504aa16da11906eb2807fd37127cb7c538 Mon Sep 17 00:00:00 2001 From: Hans Fugal Date: Wed, 15 Oct 2014 09:31:52 -0700 Subject: [PATCH] (wangle) s/State/Core/ Summary: codemod `State` is such an overloaded term, and not really the best to describe this backing future/promise object. Yes, it holds the state but it's more than that and it gets in the way of calling the states of the state machines `State`s. Test Plan: builds and tests pass Reviewed By: davejwatson@fb.com Subscribers: trunkagent, net-systems@, fugalh, exa, njormrod FB internal diff: D1615707 --- folly/wangle/Future-inl.h | 34 ++++++++++++------------- folly/wangle/Future.h | 14 +++++----- folly/wangle/Promise-inl.h | 26 +++++++++---------- folly/wangle/Promise.h | 8 +++--- folly/wangle/detail/{State.h => Core.h} | 14 +++++----- 5 files changed, 48 insertions(+), 48 deletions(-) rename folly/wangle/detail/{State.h => Core.h} (97%) diff --git a/folly/wangle/Future-inl.h b/folly/wangle/Future-inl.h index e66e504c..cf10e9f4 100644 --- a/folly/wangle/Future-inl.h +++ b/folly/wangle/Future-inl.h @@ -19,7 +19,7 @@ #include #include -#include +#include #include namespace folly { namespace wangle { @@ -35,13 +35,13 @@ struct isFuture > { }; template -Future::Future(Future&& other) noexcept : state_(nullptr) { +Future::Future(Future&& other) noexcept : core_(nullptr) { *this = std::move(other); } template Future& Future::operator=(Future&& other) { - std::swap(state_, other.state_); + std::swap(core_, other.core_); return *this; } @@ -52,15 +52,15 @@ Future::~Future() { template void Future::detach() { - if (state_) { - state_->detachFuture(); - state_ = nullptr; + if (core_) { + core_->detachFuture(); + core_ = nullptr; } } template void Future::throwIfInvalid() const { - if (!state_) + if (!core_) throw NoState(); } @@ -68,7 +68,7 @@ template template void Future::setCallback_(F&& func) { throwIfInvalid(); - state_->setCallback(std::move(func)); + core_->setCallback(std::move(func)); } template @@ -95,10 +95,10 @@ Future::then(F&& func) { sophisticated that avoids making a new Future object when it can, as an optimization. But this is correct. - state_ can't be moved, it is explicitly disallowed (as is copying). But + core_ 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 state_ (by copying it in), which means in essence obj holds a shared + for core_ (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 @@ -110,11 +110,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. detail::State has no + Two subtle but important points about this design. detail::Core 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 - detail::State, not in the Future, we can execute the continuation even + detail::Core, 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 @@ -172,21 +172,21 @@ template typename std::add_lvalue_reference::type Future::value() { throwIfInvalid(); - return state_->value(); + return core_->value(); } template typename std::add_lvalue_reference::type Future::value() const { throwIfInvalid(); - return state_->value(); + return core_->value(); } template Try& Future::getTry() { throwIfInvalid(); - return state_->getTry(); + return core_->getTry(); } template @@ -195,7 +195,7 @@ inline Future Future::via(Executor* executor) { throwIfInvalid(); this->deactivate(); - state_->setExecutor(executor); + core_->setExecutor(executor); return std::move(*this); } @@ -203,7 +203,7 @@ inline Future Future::via(Executor* executor) { template bool Future::isReady() const { throwIfInvalid(); - return state_->ready(); + return core_->ready(); } // makeFuture diff --git a/folly/wangle/Future.h b/folly/wangle/Future.h index df30efc5..3f46034b 100644 --- a/folly/wangle/Future.h +++ b/folly/wangle/Future.h @@ -199,23 +199,23 @@ class Future { /// /// Inactive Futures will activate upon destruction. void activate() { - state_->activate(); + core_->activate(); } void deactivate() { - state_->deactivate(); + core_->deactivate(); } bool isActive() { - return state_->isActive(); + return core_->isActive(); } private: - typedef detail::State* statePtr; + typedef detail::Core* corePtr; - // shared state object - statePtr state_; + // shared core state object + corePtr core_; explicit - Future(statePtr obj) : state_(obj) {} + Future(corePtr obj) : core_(obj) {} void detach(); diff --git a/folly/wangle/Promise-inl.h b/folly/wangle/Promise-inl.h index 04cc9a3f..98993c7c 100644 --- a/folly/wangle/Promise-inl.h +++ b/folly/wangle/Promise-inl.h @@ -20,31 +20,31 @@ #include #include -#include +#include namespace folly { namespace wangle { template -Promise::Promise() : retrieved_(false), state_(new detail::State()) +Promise::Promise() : retrieved_(false), core_(new detail::Core()) {} template -Promise::Promise(Promise&& other) : state_(nullptr) { +Promise::Promise(Promise&& other) : core_(nullptr) { *this = std::move(other); } template Promise& Promise::operator=(Promise&& other) { - std::swap(state_, other.state_); + std::swap(core_, other.core_); std::swap(retrieved_, other.retrieved_); return *this; } template void Promise::throwIfFulfilled() { - if (!state_) + if (!core_) throw NoState(); - if (state_->ready()) + if (core_->ready()) throw PromiseAlreadySatisfied(); } @@ -61,11 +61,11 @@ Promise::~Promise() { template void Promise::detach() { - if (state_) { + if (core_) { if (!retrieved_) - state_->detachFuture(); - state_->detachPromise(); - state_ = nullptr; + core_->detachFuture(); + core_->detachPromise(); + core_ = nullptr; } } @@ -73,7 +73,7 @@ template Future Promise::getFuture() { throwIfRetrieved(); retrieved_ = true; - return Future(state_); + return Future(core_); } template @@ -85,13 +85,13 @@ void Promise::setException(E const& e) { template void Promise::setException(std::exception_ptr const& e) { throwIfFulfilled(); - state_->setException(e); + core_->setException(e); } template void Promise::fulfilTry(Try&& t) { throwIfFulfilled(); - state_->fulfil(std::move(t)); + core_->fulfil(std::move(t)); } template diff --git a/folly/wangle/Promise.h b/folly/wangle/Promise.h index e14350e3..a4eb94b1 100644 --- a/folly/wangle/Promise.h +++ b/folly/wangle/Promise.h @@ -38,7 +38,7 @@ public: Promise(Promise&&); Promise& operator=(Promise&&); - /** Return a Future tied to the shared state. This can be called only + /** Return a Future tied to the shared core state. This can be called only once, thereafter Future already retrieved exception will be raised. */ Future getFuture(); @@ -76,13 +76,13 @@ public: void fulfil(F&& func); private: - typedef typename Future::statePtr statePtr; + typedef typename Future::corePtr corePtr; // Whether the Future has been retrieved (a one-time operation). bool retrieved_; - // shared state object - statePtr state_; + // shared core state object + corePtr core_; void throwIfFulfilled(); void throwIfRetrieved(); diff --git a/folly/wangle/detail/State.h b/folly/wangle/detail/Core.h similarity index 97% rename from folly/wangle/detail/State.h rename to folly/wangle/detail/Core.h index c06e2236..f237c619 100644 --- a/folly/wangle/detail/State.h +++ b/folly/wangle/detail/Core.h @@ -37,24 +37,24 @@ void empty_callback(Try&&) { } /** The shared state object for Future and Promise. */ template -class State { +class Core { public: // This must be heap-constructed. There's probably a way to enforce that in // code but since this is just internal detail code and I don't know how // off-hand, I'm punting. - State() = default; - ~State() { + Core() = default; + ~Core() { assert(calledBack_); assert(detached_ == 2); } // not copyable - State(State const&) = delete; - State& operator=(State const&) = delete; + Core(Core const&) = delete; + Core& operator=(Core const&) = delete; // not movable (see comment in the implementation of Future::then) - State(State&&) noexcept = delete; - State& operator=(State&&) = delete; + Core(Core&&) noexcept = delete; + Core& operator=(Core&&) = delete; Try& getTry() { return *value_; -- 2.34.1