From ad710a5699933b02d647adfd416ffc2ac93b7807 Mon Sep 17 00:00:00 2001 From: Hans Fugal Date: Thu, 26 Jun 2014 14:31:48 -0700 Subject: [PATCH] nuke executeWith Summary: Removing this crufty API. The only callsite for `executeWith` was `Later::via` so I just folded it in there. Test Plan: unit tests contbuild Reviewed By: hannesr@fb.com Subscribers: net-systems@, fugalh, exa FB internal diff: D1406592 Tasks: 4480567 --- folly/wangle/Future-inl.h | 16 ---------------- folly/wangle/Future.h | 32 +++++++------------------------- folly/wangle/Later-inl.h | 13 ++++++++++--- 3 files changed, 17 insertions(+), 44 deletions(-) diff --git a/folly/wangle/Future-inl.h b/folly/wangle/Future-inl.h index 933cbe6e..47bede0e 100644 --- a/folly/wangle/Future-inl.h +++ b/folly/wangle/Future-inl.h @@ -199,22 +199,6 @@ inline Future Future::via(Executor* executor) { return f; } -template -template -inline void Future::executeWith( - Executor* executor, Promise&& cont_promise) { - throwIfInvalid(); - - folly::MoveWrapper> p(std::move(cont_promise)); - - setContinuation([executor, p](Try&& t) mutable { - folly::MoveWrapper> tt(std::move(t)); - executor->add([p, tt]() mutable { - p->fulfilTry(std::move(*tt)); - }); - }); -} - template bool Future::isReady() const { throwIfInvalid(); diff --git a/folly/wangle/Future.h b/folly/wangle/Future.h index 0c866215..5390705d 100644 --- a/folly/wangle/Future.h +++ b/folly/wangle/Future.h @@ -71,28 +71,6 @@ class Future { template Future via(Executor* executor); - /// Deprecated alias for via - template - Future executeWithSameThread(Executor* executor) { - return via(executor); - } - - /** - Thread-safe version of executeWith - - Since an executor would likely start executing the Future chain - right away, it would be a race condition to call: - Future.executeWith(...).then(...), as there would be race - condition between the then and the running Future. - Instead, you may pass in a Promise so that we can set up - the rest of the chain in advance, without any racey - modifications of the continuation - - Deprecated. Use a Later. - */ - template - void executeWith(Executor* executor, Promise&& cont_promise); - /** True when the result (or exception) is ready. */ bool isReady() const; @@ -111,7 +89,8 @@ class Future { */ /* TODO n3428 and other async frameworks have something like then(scheduler, Future), we probably want to support a similar API (instead of - executeWith). */ + via. or rather, via should return a cold future (Later) and we provide + then(scheduler, Future) ). */ template typename std::enable_if< !isFuture&&)>::type>::value, @@ -196,8 +175,11 @@ class Future { /// Exceptions still propagate. Future then(); - /// Use of this method is advanced wizardry. - /// XXX should this be protected? + /// This is not the method you're looking for. + /// + /// This needs to be public because it's used by make* and when*, and it's + /// not worth listing all those and their fancy template signatures as + /// friends. But it's not for public consumption. template void setContinuation(F&& func); diff --git a/folly/wangle/Later-inl.h b/folly/wangle/Later-inl.h index aaba5c94..4f00e268 100644 --- a/folly/wangle/Later-inl.h +++ b/folly/wangle/Later-inl.h @@ -130,10 +130,17 @@ Later::then(F&& fn) { template Later Later::via(Executor* executor) { - Promise promise; + folly::MoveWrapper> promise; Later later(std::move(starter_)); - later.future_ = promise.getFuture(); - future_->executeWith(executor, std::move(promise)); + later.future_ = promise->getFuture(); + + future_->setContinuation([executor, promise](Try&& t) mutable { + folly::MoveWrapper> tt(std::move(t)); + executor->add([promise, tt]() mutable { + promise->fulfilTry(std::move(*tt)); + }); + }); + return later; } -- 2.34.1