Support co_return; in coro Expected of empty trivial types like `…

…Unit`

Summary:
Allow `Expected<Unit, E>` coros to return void -- aka `co_return;` or reaching the `}` closing the function. To do this, I add an `expected_detail::Promise` specialization for `Value` that is both trivial and empty, and refactor the methods that can be shared with the value-returning case into `PromiseBase`.

This is an interface change for existing `Expected` coroutines -- they need to use `co_await makeUnexpected(...)` instead of `co_return makeUnexpected(...)`.

Unlike `co_return`, it's possible for `co_await` to suspend the coroutine, which is potentially slower. In D51537604, yfeldblum measured / optimized this case a bit, so this is (hopefully) now acceptable in exchange for improved readability.

Reviewed By: yfeldblum

Differential Revision: D49253265

fbshipit-source-id: a18b407fa023fed22ef86a18116da2bacd93f30a

folly/Expected.h

@@ -213,7 +213,7 @@ using ExpectedErrorType =
 // Details...
 namespace expected_detail {
 
-template <typename Value, typename Error>
+template <typename Value, typename Error, typename = void>
 struct Promise;
 template <typename Value, typename Error>
 struct PromiseReturn;
@@ -1563,14 +1563,14 @@ bool operator>(const Value& other, const Expected<Value, Error>&) = delete;
 namespace folly {
 namespace expected_detail {
 template <typename Value, typename Error>
-struct Promise;
+struct PromiseBase;
 
 template <typename Value, typename Error>
 struct PromiseReturn {
   Expected<Value, Error> storage_{EmptyTag{}};
   Expected<Value, Error>*& pointer_;
 
-  /* implicit */ PromiseReturn(Promise<Value, Error>& p) noexcept
+  /* implicit */ PromiseReturn(PromiseBase<Value, Error>& p) noexcept
       : pointer_{p.value_} {
     pointer_ = &storage_;
   }
@@ -1592,25 +1592,48 @@ struct PromiseReturn {
 };
 
 template <typename Value, typename Error>
-struct Promise {
+struct PromiseBase {
   Expected<Value, Error>* value_ = nullptr;
 
-  Promise() = default;
-  Promise(Promise const&) = delete;
+  PromiseBase() = default;
+  PromiseBase(PromiseBase const&) = delete;
+  void operator=(PromiseBase const&) = delete;
+
+  [[nodiscard]] coro::suspend_never initial_suspend() const noexcept {
+    return {};
+  }
+  [[nodiscard]] coro::suspend_never final_suspend() const noexcept {
+    return {};
+  }
+  [[noreturn]] void unhandled_exception() {
+    // Technically, throwing from unhandled_exception is underspecified:
+    // https://github.com/GorNishanov/CoroutineWording/issues/17
+    rethrow_current_exception();
+  }
+
   PromiseReturn<Value, Error> get_return_object() noexcept { return *this; }
-  coro::suspend_never initial_suspend() const noexcept { return {}; }
-  coro::suspend_never final_suspend() const noexcept { return {}; }
+};
+
+template <typename Value>
+inline constexpr bool ReturnsVoid =
+    std::is_trivial_v<Value> && std::is_empty_v<Value>;
+
+template <typename Value, typename Error>
+struct Promise<Value, Error, typename std::enable_if<!ReturnsVoid<Value>>::type>
+    : public PromiseBase<Value, Error> {
   template <typename U = Value>
   void return_value(U&& u) {
-    auto& v = *value_;
+    auto& v = *this->value_;
     ExpectedHelper::assume_empty(v);
     v = static_cast<U&&>(u);
   }
-  void unhandled_exception() {
-    // Technically, throwing from unhandled_exception is underspecified:
-    // https://github.com/GorNishanov/CoroutineWording/issues/17
-    rethrow_current_exception();
-  }
+};
+
+template <typename Value, typename Error>
+struct Promise<Value, Error, typename std::enable_if<ReturnsVoid<Value>>::type>
+    : public PromiseBase<Value, Error> {
+  // When the coroutine uses `return;` you can fail via `co_await err`.
+  void return_void() { this->value_->emplace(Value{}); }
 };
 
 template <typename Error>

folly/test/ExpectedCoroutinesTest.cpp

@@ -121,6 +121,43 @@ TEST(Expected, CoroutineReturnUnexpected) {
   EXPECT_EQ(Err::badder(), r1.error());
 }
 
+TEST(Expected, CoroutineReturnsVoid) {
+  int x = 0;
+  auto r = [&]() -> Expected<folly::Unit, Err> {
+    x = co_await f1();
+    co_return;
+  }();
+  EXPECT_TRUE(r.hasValue());
+  EXPECT_EQ(folly::unit, *r);
+  EXPECT_EQ(7, x);
+}
+
+TEST(Expected, CoroutineReturnsVoidThrows) {
+  auto fnThrows = [&]() -> Expected<folly::Unit, Err> {
+    throws();
+    co_return;
+  };
+  ASSERT_THROW(({ fnThrows(); }), Exn);
+}
+
+TEST(Expected, CoroutineReturnsVoidError) {
+  auto fnErr = [&]() -> Expected<folly::Unit, Err> {
+    return makeUnexpected(Err::bad());
+  };
+  auto r = [&]() -> Expected<folly::Unit, Err> { co_await fnErr(); }();
+  EXPECT_TRUE(r.hasError());
+  EXPECT_EQ(Err::bad(), r.error());
+}
+
+TEST(Expected, VoidCoroutineAwaitsError) {
+  auto r = []() -> Expected<folly::Unit, Err> {
+    co_await makeUnexpected(Err::badder());
+    ADD_FAILURE();
+  }();
+  EXPECT_TRUE(r.hasError());
+  EXPECT_EQ(Err::badder(), r.error());
+}
+
 TEST(Expected, CoroutineException) {
   EXPECT_THROW(
       ([]() -> Expected<int, Err> {