nuke executeWith
authorHans Fugal <fugalh@fb.com>
Thu, 26 Jun 2014 21:31:48 +0000 (14:31 -0700)
committerNicholas Ormrod <njormrod@fb.com>
Fri, 27 Jun 2014 22:06:20 +0000 (15:06 -0700)
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
folly/wangle/Future.h
folly/wangle/Later-inl.h

index 933cbe6eae1c6876f6de0c531aa2ce45667fa375..47bede0eaf7df5334f63649b03393cbebbf22ea0 100644 (file)
@@ -199,22 +199,6 @@ inline Future<T> Future<T>::via(Executor* executor) {
   return f;
 }
 
-template <class T>
-template <typename Executor>
-inline void Future<T>::executeWith(
-    Executor* executor, Promise<T>&& cont_promise) {
-  throwIfInvalid();
-
-  folly::MoveWrapper<Promise<T>> p(std::move(cont_promise));
-
-  setContinuation([executor, p](Try<T>&& t) mutable {
-      folly::MoveWrapper<Try<T>> tt(std::move(t));
-      executor->add([p, tt]() mutable {
-          p->fulfilTry(std::move(*tt));
-        });
-    });
-}
-
 template <class T>
 bool Future<T>::isReady() const {
   throwIfInvalid();
index 0c8662159e2b5b7b9ecb7d691bcc922a7edd1a33..5390705de0f3f28550e8e096cbff1a0e46ea6c32 100644 (file)
@@ -71,28 +71,6 @@ class Future {
   template <typename Executor>
   Future<T> via(Executor* executor);
 
-  /// Deprecated alias for via
-  template <typename Executor>
-  Future<T> 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 <typename Executor>
-  void executeWith(Executor* executor, Promise<T>&& 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 <class F>
   typename std::enable_if<
     !isFuture<typename std::result_of<F(Try<T>&&)>::type>::value,
@@ -196,8 +175,11 @@ class Future {
   /// Exceptions still propagate.
   Future<void> 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 <class F>
   void setContinuation(F&& func);
 
index aaba5c94395d79be032a16dbe2ad6ee7e6839628..4f00e268f99e65ad93fe90d1f881cae249bd4f45 100644 (file)
@@ -130,10 +130,17 @@ Later<T>::then(F&& fn) {
 
 template <class T>
 Later<T> Later<T>::via(Executor* executor) {
-  Promise<T> promise;
+  folly::MoveWrapper<Promise<T>> promise;
   Later<T> later(std::move(starter_));
-  later.future_ = promise.getFuture();
-  future_->executeWith(executor, std::move(promise));
+  later.future_ = promise->getFuture();
+
+  future_->setContinuation([executor, promise](Try<T>&& t) mutable {
+    folly::MoveWrapper<Try<T>> tt(std::move(t));
+    executor->add([promise, tt]() mutable {
+      promise->fulfilTry(std::move(*tt));
+    });
+  });
+
   return later;
 }