Commit 4c7a8438 authored by Hans Fugal's avatar Hans Fugal Committed by dcsommer

(wangle) Core cleanup

Summary:
Just some cleaning hose in `detail::Core`.
- rename `fulfil` to `setResult`
- and `value_` to `result_`
- remove unnecessary helper methods (I can be convinced to keep them if you like them)

Test Plan: mechanical changes, tests still pass

Reviewed By: davejwatson@fb.com

Subscribers: trunkagent, folly-diffs@, net-systems@, fugalh, exa, njormrod

FB internal diff: D1618161
parent 72984487
...@@ -172,7 +172,7 @@ template <class T> ...@@ -172,7 +172,7 @@ template <class T>
typename std::add_lvalue_reference<T>::type Future<T>::value() { typename std::add_lvalue_reference<T>::type Future<T>::value() {
throwIfInvalid(); throwIfInvalid();
return core_->value(); return core_->getTry().value();
} }
template <class T> template <class T>
......
...@@ -85,13 +85,13 @@ void Promise<T>::setException(E const& e) { ...@@ -85,13 +85,13 @@ void Promise<T>::setException(E const& e) {
template <class T> template <class T>
void Promise<T>::setException(std::exception_ptr const& e) { void Promise<T>::setException(std::exception_ptr const& e) {
throwIfFulfilled(); throwIfFulfilled();
core_->setException(e); core_->setResult(Try<T>(e));
} }
template <class T> template <class T>
void Promise<T>::fulfilTry(Try<T>&& t) { void Promise<T>::fulfilTry(Try<T>&& t) {
throwIfFulfilled(); throwIfFulfilled();
core_->fulfil(std::move(t)); core_->setResult(std::move(t));
} }
template <class T> template <class T>
......
...@@ -58,7 +58,11 @@ class Core { ...@@ -58,7 +58,11 @@ class Core {
Core& operator=(Core&&) = delete; Core& operator=(Core&&) = delete;
Try<T>& getTry() { Try<T>& getTry() {
return *value_; if (ready()) {
return *result_;
} else {
throw FutureNotReady();
}
} }
template <typename F> template <typename F>
...@@ -76,39 +80,23 @@ class Core { ...@@ -76,39 +80,23 @@ class Core {
maybeCallback(); maybeCallback();
} }
void fulfil(Try<T>&& t) { void setResult(Try<T>&& t) {
{ {
std::lock_guard<decltype(mutex_)> lock(mutex_); std::lock_guard<decltype(mutex_)> lock(mutex_);
if (ready()) { if (ready()) {
throw std::logic_error("fulfil called twice"); throw std::logic_error("setResult called twice");
} }
value_ = std::move(t); result_ = std::move(t);
assert(ready()); assert(ready());
} }
maybeCallback(); maybeCallback();
} }
void setException(std::exception_ptr const& e) {
fulfil(Try<T>(e));
}
template <class E> void setException(E const& e) {
fulfil(Try<T>(std::make_exception_ptr<E>(e)));
}
bool ready() const { bool ready() const {
return value_.hasValue(); return result_.hasValue();
}
typename std::add_lvalue_reference<T>::type value() {
if (ready()) {
return value_->value();
} else {
throw FutureNotReady();
}
} }
// Called by a destructing Future // Called by a destructing Future
...@@ -123,7 +111,7 @@ class Core { ...@@ -123,7 +111,7 @@ class Core {
// Called by a destructing Promise // Called by a destructing Promise
void detachPromise() { void detachPromise() {
if (!ready()) { if (!ready()) {
setException(BrokenPromise()); setResult(Try<T>(std::make_exception_ptr(BrokenPromise())));
} }
detachOne(); detachOne();
} }
...@@ -152,17 +140,17 @@ class Core { ...@@ -152,17 +140,17 @@ class Core {
void maybeCallback() { void maybeCallback() {
std::unique_lock<decltype(mutex_)> lock(mutex_); std::unique_lock<decltype(mutex_)> lock(mutex_);
if (!calledBack_ && if (!calledBack_ &&
value_ && callback_ && isActive()) { result_ && callback_ && isActive()) {
// TODO(5306911) we should probably try/catch here // TODO(5306911) we should probably try/catch here
if (executor_) { if (executor_) {
MoveWrapper<folly::Optional<Try<T>>> val(std::move(value_)); MoveWrapper<folly::Optional<Try<T>>> val(std::move(result_));
MoveWrapper<std::function<void(Try<T>&&)>> cb(std::move(callback_)); MoveWrapper<std::function<void(Try<T>&&)>> cb(std::move(callback_));
executor_->add([cb, val]() mutable { (*cb)(std::move(**val)); }); executor_->add([cb, val]() mutable { (*cb)(std::move(**val)); });
calledBack_ = true; calledBack_ = true;
} else { } else {
calledBack_ = true; calledBack_ = true;
lock.unlock(); lock.unlock();
callback_(std::move(*value_)); callback_(std::move(*result_));
} }
} }
} }
...@@ -183,7 +171,7 @@ class Core { ...@@ -183,7 +171,7 @@ class Core {
} }
} }
folly::Optional<Try<T>> value_; folly::Optional<Try<T>> result_;
std::function<void(Try<T>&&)> callback_; std::function<void(Try<T>&&)> callback_;
bool calledBack_ = false; bool calledBack_ = false;
unsigned char detached_ = 0; unsigned char detached_ = 0;
...@@ -191,7 +179,7 @@ class Core { ...@@ -191,7 +179,7 @@ class Core {
Executor* executor_ = nullptr; Executor* executor_ = nullptr;
// this lock isn't meant to protect all accesses to members, only the ones // this lock isn't meant to protect all accesses to members, only the ones
// that need to be threadsafe: the act of setting value_ and callback_, and // that need to be threadsafe: the act of setting result_ and callback_, and
// seeing if they are set and whether we should then continue. // seeing if they are set and whether we should then continue.
folly::MicroSpinLock mutex_ {0}; folly::MicroSpinLock mutex_ {0};
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment