Make sure SemiFuture can't be used as a inline task

Summary:
Before this change one could write:
  SemiFuture<void> f() {
    co_await f1();
    co_await f2();
  }
where SemiFuture coroutine would have semantics of an InlineTask (f1 called inline, f2 would be called on the executor which completes f1). This doesn't match the semantics of SemiFuture with deferred work (both f1() and f2() called on the executor that was passed to SemiFuture's via).

Drop support for SemiFuture coroutines, because that isn't used anywhere except for toSemiFuture function.

Reviewed By: lewissbaker

Differential Revision: D13501140

fbshipit-source-id: d77f491821e6a77cef0c92d83839bff538552b32

folly/experimental/coro/Task.h

@@ -27,6 +27,7 @@
 #include <folly/experimental/coro/Traits.h>
 #include <folly/experimental/coro/Utils.h>
 #include <folly/experimental/coro/ViaIfAsync.h>
+#include <folly/experimental/coro/detail/InlineTask.h>
 #include <folly/futures/Future.h>
 
 namespace folly {
@@ -126,8 +127,8 @@ class TaskPromise : public TaskPromiseBase {
     result_.emplace(static_cast<U&&>(value));
   }
 
-  T getResult() {
-    return static_cast<T&&>(std::move(result_).value());
+  Try<T>& result() {
+    return result_;
   }
 
  private:
@@ -153,8 +154,8 @@ class TaskPromise<void> : public TaskPromiseBase {
 
   void return_void() noexcept {}
 
-  void getResult() {
-    return std::move(result_).value();
+  Try<void>& result() {
+    return result_;
   }
 
  private:
@@ -199,9 +200,24 @@ class FOLLY_NODISCARD TaskWithExecutor {
   // Start execution of this task eagerly and return a folly::SemiFuture<T>
   // that will complete with the result.
   auto start() && {
-    return folly::coro::toSemiFuture(std::move(*this));
+    Promise<lift_unit_t<T>> p;
+    auto sf = p.getSemiFuture();
+
+    [](Promise<lift_unit_t<T>> p,
+       TaskWithExecutor task) -> detail::InlineTaskDetached {
+      try {
+        p.setTry(co_await std::move(task).co_awaitTry());
+      } catch (const std::exception& e) {
+        p.setException(exception_wrapper(std::current_exception(), e));
+      } catch (...) {
+        p.setException(exception_wrapper(std::current_exception()));
+      }
+    }(std::move(p), std::move(*this));
+
+    return sf;
   }
 
+  template <typename ResultCreator>
   class Awaiter {
    public:
     explicit Awaiter(handle_t coro) noexcept : coro_(coro) {}
@@ -231,15 +247,32 @@ class FOLLY_NODISCARD TaskWithExecutor {
       SCOPE_EXIT {
         std::exchange(coro_, {}).destroy();
       };
-      return coro_.promise().getResult();
+      ResultCreator resultCreator;
+      return resultCreator(std::move(coro_.promise().result()));
     }
 
    private:
     handle_t coro_;
   };
 
-  Awaiter operator co_await() && noexcept {
-    return Awaiter{std::exchange(coro_, {})};
+  struct ValueCreator {
+    T operator()(Try<T>&& t) {
+      return std::move(t).value();
+    }
+  };
+
+  struct TryCreator {
+    Try<T> operator()(Try<T>&& t) {
+      return std::move(t);
+    }
+  };
+
+  auto operator co_await() && noexcept {
+    return Awaiter<ValueCreator>{std::exchange(coro_, {})};
+  }
+
+  auto co_awaitTry() && noexcept {
+    return Awaiter<TryCreator>{std::exchange(coro_, {})};
   }
 
  private:
@@ -349,11 +382,11 @@ auto detail::TaskPromiseBase::await_transform(Task<T>&& t) noexcept {
       return coro_;
     }
 
-    decltype(auto) await_resume() {
+    T await_resume() {
       SCOPE_EXIT {
         std::exchange(coro_, {}).destroy();
       };
-      return coro_.promise().getResult();
+      return std::move(coro_.promise().result()).value();
     }
 
    private:

folly/experimental/coro/detail/InlineTask.h

@@ -229,6 +229,41 @@ inline InlineTask<void> InlineTaskPromise<void>::get_return_object() noexcept {
       InlineTaskPromise<void>>::from_promise(*this)};
 }
 
+/// InlineTaskDetached is a coroutine-return type where the coroutine is
+/// launched in the current execution context when it is created and the
+/// task's continuation is launched inline in the execution context that the
+/// task completed on.
+///
+/// This task type is primarily intended as a building block for certain
+/// coroutine operators. It is not intended for general use in application
+/// code or in library interfaces exposed to library code as it can easily be
+/// abused to accidentally run logic on the wrong execution context.
+///
+/// For this reason, the InlineTaskDetached type has been placed inside the
+/// folly::coro::detail namespace to discourage general usage.
+struct InlineTaskDetached {
+  class promise_type {
+   public:
+    InlineTaskDetached get_return_object() {
+      return {};
+    }
+
+    std::experimental::suspend_never initial_suspend() {
+      return {};
+    }
+
+    std::experimental::suspend_never final_suspend() {
+      return {};
+    }
+
+    void return_void() {}
+
+    [[noreturn]] void unhandled_exception() {
+      std::terminate();
+    }
+  };
+};
+
 } // namespace detail
 } // namespace coro
 } // namespace folly

folly/futures/Future.h

@@ -834,41 +834,6 @@ class SemiFuture : private futures::detail::FutureBase<T> {
   Future<T> toUnsafeFuture() &&;
 
 #if FOLLY_HAS_COROUTINES
-  class promise_type {
-   public:
-    SemiFuture get_return_object() {
-      return promise_.getSemiFuture();
-    }
-
-    std::experimental::suspend_never initial_suspend() {
-      return {};
-    }
-
-    std::experimental::suspend_never final_suspend() {
-      return {};
-    }
-
-    void return_value(const T& value) {
-      promise_.setValue(value);
-    }
-
-    void return_value(T&& value) {
-      promise_.setValue(std::move(value));
-    }
-
-    void unhandled_exception() {
-      try {
-        std::rethrow_exception(std::current_exception());
-      } catch (std::exception& e) {
-        promise_.setException(exception_wrapper(std::current_exception(), e));
-      } catch (...) {
-        promise_.setException(exception_wrapper(std::current_exception()));
-      }
-    }
-
-   private:
-    folly::Promise<T> promise_;
-  };
 
   // Customise the co_viaIfAsync() operator so that SemiFuture<T> can be
   // directly awaited within a folly::coro::Task coroutine.
@@ -2085,30 +2050,6 @@ inline detail::FutureAwaitable<T>
   return detail::FutureAwaitable<T>(std::move(future));
 }
 
-namespace coro {
-
-/// Convert an awaitable type into a SemiFuture that can then be consumed by
-/// APIs that use folly::Future/SemiFuture.
-///
-/// This will eagerly start execution of 'co_await awaitable' and will make
-/// the eventual result available via the returned SemiFuture.
-template <typename Awaitable>
-inline auto toSemiFuture(Awaitable awaitable) -> std::enable_if_t<
-    !std::is_void<folly::coro::await_result_t<Awaitable>>::value,
-    SemiFuture<await_result_t<Awaitable>>> {
-  co_return co_await static_cast<Awaitable&&>(awaitable);
-}
-
-template <typename Awaitable>
-inline auto toSemiFuture(Awaitable awaitable) -> std::enable_if_t<
-    std::is_void<folly::coro::await_result_t<Awaitable>>::value,
-    SemiFuture<Unit>> {
-  co_await static_cast<Awaitable&&>(awaitable);
-  co_return Unit{};
-}
-
-} // namespace coro
-
 } // namespace folly
 #endif