Remove executor-taking form of then

Summary:
Remove calls to Future::then(executor, callback).

This form of Future::then is ambiguous, and does not yet implement the stronger
typing of thenValue and thenTry. It is also tempting to use instead of via,
where it is not obvious that it has the behaviour of wrapping a via call in a
push and pop of the current executor:
.pushCurrentExecutor().via(executor).then(callback).popCurrentExecutor().

With the addition of inline continuations, we can instead make the nesting
explicit at low cost by making it an inline continuation that launches an
asynchronous task on the passed executor.

Reviewed By: yfeldblum

Differential Revision: D15100419

fbshipit-source-id: e929e2eaf2fc30313cb099150be3a9535ae729a6

folly/futures/Future.h

@@ -1204,39 +1204,7 @@ class Future : private futures::detail::FutureBase<T> {
   ///   i.e., as if `*this` was moved into RESULT.
   /// - `RESULT.valid() == true`
   template <class Arg>
-  // clang-format off
-  [[deprecated("then forms that take an executor are ambiguous. "
-               "Replace with nested tasks, or for executor enforcement use "
-               "SemiFuture-returning functions.")]]
-  // clang-format on
-  auto
-  then(Executor::KeepAlive<> x, Arg&& arg) && {
-    auto oldX = getKeepAliveToken(this->getExecutor());
-    this->setExecutor(std::move(x));
-    // TODO(T29171940): thenImplementation here is ambiguous
-    // as then used to be but that is better than keeping then in the public
-    // API.
-    auto lambdaFunc =
-        futures::detail::makeExecutorLambda<T>(std::forward<Arg>(arg));
-    using R = futures::detail::executorCallableResult<T, decltype(lambdaFunc)>;
-    return std::move(*this)
-        .thenImplementation(
-            std::move(lambdaFunc),
-            R{},
-            futures::detail::InlineContinuation::forbid)
-        .via(std::move(oldX));
-  }
-
-  template <typename R, typename... Args>
-  // clang-format off
-  [[deprecated("then forms that take an executor are ambiguous. "
-               "Replace with nested tasks, or for executor enforcement use "
-               "SemiFuture-returning functions.")]]
-  // clang-format on
-  auto
-  then(Executor::KeepAlive<>&& x, R (&func)(Args...)) && {
-    return std::move(*this).then(std::move(x), &func);
-  }
+  auto then(Executor::KeepAlive<> x, Arg&& arg) && = delete;
 
   /// When this Future has completed, execute func which is a function that
   /// can be called with `Try<T>&&` (often a lambda with parameter type

folly/futures/test/FutureTest.cpp

@@ -1334,7 +1334,8 @@ TEST(Future, makePromiseContract) {
 }
 
 Future<bool> call(int depth, Executor* executor) {
-  return makeFuture().then(executor, [=] { return depth == 0; });
+  return makeFuture().thenValueInline(
+      makeAsyncTask(executor, [=](auto&&) { return depth == 0; }));
 }
 
 Future<int> recursion(Executor* executor, int depth) {

folly/futures/test/ViaTest.cpp

@@ -219,12 +219,11 @@ TEST(Via, priority) {
 
 TEST(Via, then2) {
   ManualExecutor x1, x2;
-  bool a = false, b = false, c = false, d = false;
+  bool a = false, b = false, c = false;
   via(&x1)
       .thenValue([&](auto&&) { a = true; })
-      .then(&x2, [&](auto&&) { b = true; })
-      .thenValue([&](auto&&) { c = true; })
-      .thenValueInline(folly::makeAsyncTask(&x2, [&](auto&&) { d = true; }));
+      .thenValue([&](auto&&) { b = true; })
+      .thenValueInline(folly::makeAsyncTask(&x2, [&](auto&&) { c = true; }));
 
   EXPECT_FALSE(a);
   EXPECT_FALSE(b);
@@ -234,16 +233,12 @@ TEST(Via, then2) {
   EXPECT_FALSE(b);
   EXPECT_FALSE(c);
 
-  x2.run();
+  x1.run();
   EXPECT_TRUE(b);
   EXPECT_FALSE(c);
 
-  x1.run();
-  EXPECT_TRUE(c);
-  EXPECT_FALSE(d);
-
   x2.run();
-  EXPECT_TRUE(d);
+  EXPECT_TRUE(c);
 }
 
 TEST(Via, allowInline) {