fix(common): returnable argument to .then()

google/cloud/future_generic.h

@@ -15,16 +15,10 @@
 #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_FUTURE_GENERIC_H
 #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_FUTURE_GENERIC_H
 
-/**
- * @file
- *
- * Implement `future<T>` and `promise<T>`.
- */
-
 #include "google/cloud/internal/future_base.h"
 #include "google/cloud/internal/future_fwd.h"
 #include "google/cloud/internal/future_impl.h"
-#include "google/cloud/internal/future_then_meta.h"
+#include "google/cloud/internal/invoke_result.h"
 #include "google/cloud/version.h"
 #include "absl/meta/type_traits.h"
 #include <future>
@@ -71,8 +65,7 @@ class future final : private internal::future_base<T> {
    */
   template <class U, typename Enable =
                          absl::enable_if_t<std::is_constructible<T, U>::value>>
-  explicit future(future<U>&& rhs)
-      : future<T>(rhs.then([](future<U> other) { return T(other.get()); })) {}
+  explicit future(future<U>&& rhs);
 
   /**
    * Waits until the shared state becomes ready, then retrieves the value stored
@@ -116,31 +109,21 @@ class future final : private internal::future_base<T> {
    * Side effects: `valid() == false` if the operation is successful.
    */
   template <typename F>
-  typename internal::then_helper<F, T>::future_t then(F&& func) {
-    this->check_valid();
-    using requires_unwrap_t =
-        typename internal::then_helper<F, T>::requires_unwrap_t;
-    return then_impl(std::forward<F>(func), requires_unwrap_t{});
-  }
+  auto then(F&& func) -> future<
+      /// @cond implementation_details
+      internal::UnwrappedType<internal::invoke_result_t<F, future<T>>>
+      /// @endcond
+      >;
 
   explicit future(std::shared_ptr<shared_state_type> state)
       : internal::future_base<T>(std::move(state)) {}
 
  private:
-  /// Implement `then()` if the result does not require unwrapping.
-  template <typename F>
-  typename internal::then_helper<F, T>::future_t then_impl(F&& functor,
-                                                           std::false_type);
-
-  /// Implement `then()` if the result requires unwrapping.
-  template <typename F>
-  typename internal::then_helper<F, T>::future_t then_impl(F&& functor,
-                                                           std::true_type);
-
   template <typename U>
   friend class future;
   friend class future<void>;
 
+  friend struct internal::FutureThenImpl;
   friend struct internal::CoroutineSupport;
 };
 
@@ -162,8 +145,7 @@ class promise final : private internal::promise_base<T> {
       : internal::promise_base<T>(std::move(x)) {}
 
   /// Constructs a new promise and transfer any shared state from @p rhs.
-  // NOLINTNEXTLINE(performance-noexcept-move-constructor)
-  promise(promise&&) = default;
+  promise(promise&&) noexcept = default;
 
   /// Abandons the shared state in `*this`, if any, and transfers the shared
   /// state from @p rhs.

google/cloud/future_generic_test.cc

@@ -769,6 +769,7 @@ struct FromInt {
 TEST(FutureTestConvertingConstructor, ConvertFuture) {
   promise<int> p0;
   future<FromInt> f0{p0.get_future()};
+  EXPECT_FALSE(f0.is_ready());
 }
 
 static_assert(!std::is_constructible<future<FromInt>, future<int*>>::value,

google/cloud/future_generic_then_test.cc

@@ -50,6 +50,10 @@ TEST(FutureTestInt, ThenSimple) {
   EXPECT_FALSE(next.valid());
 }
 
+// With exceptions disabled this test is not very useful. Yes,
+// `internal::ThrowRuntimeError()` is called and that terminates the
+// application. We knew that already.
+#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
 TEST(FutureTestInt, ThenException) {
   promise<int> p;
   future<int> fut = p.get_future();
@@ -68,7 +72,6 @@ TEST(FutureTestInt, ThenException) {
   EXPECT_TRUE(next.valid());
   EXPECT_FALSE(called);
 
-#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
   p.set_value(42);
   EXPECT_TRUE(called);
   EXPECT_TRUE(next.valid());
@@ -81,10 +84,8 @@ TEST(FutureTestInt, ThenException) {
       },
       std::runtime_error);
   EXPECT_FALSE(next.valid());
-#else
-  EXPECT_DEATH_IF_SUPPORTED(p.set_value(42), "test message");
-#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
 }
+#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
 
 TEST(FutureTestInt, ThenUnwrap) {
   promise<int> p;
@@ -188,6 +189,75 @@ TEST(FutureTestInt, CancelThroughContinuation) {
   EXPECT_EQ(84, f1.get());
 }
 
+TEST(FutureTestInt, CancelThroughUnwrappingContinuation) {
+  bool cancelled = false;
+  promise<int> p0([&cancelled] { cancelled = true; });
+  auto f0 = p0.get_future();
+  auto f1 = f0.then(
+      [](auto g) { return g.then([](auto h) { return h.get() * 2; }); });
+  EXPECT_TRUE(f1.cancel());
+  EXPECT_TRUE(cancelled);
+  p0.set_value(42);
+  EXPECT_EQ(f1.get(), 84);
+}
+
+TEST(FutureTestInt, CancelThroughConverted) {
+  bool cancelled = false;
+  promise<int> p0([&cancelled] { cancelled = true; });
+  future<std::int64_t> f1(
+      p0.get_future().then([](auto h) { return h.get() * 2; }));
+  EXPECT_TRUE(f1.cancel());
+  EXPECT_TRUE(cancelled);
+  p0.set_value(42);
+  EXPECT_EQ(f1.get(), 84);
+}
+
+/// @test Verify that `.then()` can return its argument.
+TEST(FutureTestInt, ThenReturnsArgument) {
+  promise<int> p;
+  int counter = 0;
+  auto f = p.get_future().then([&counter](auto g) {
+    ++counter;
+    return g;
+  });
+  p.set_value(42);
+  EXPECT_TRUE(f.is_ready());
+  EXPECT_EQ(counter, 1);
+  EXPECT_EQ(f.get(), 42);
+}
+
+/// @test Verify that `.then()` can return its argument.
+TEST(FutureTestInt, ThenAttachesContinuationToArgument) {
+  promise<int> p;
+  auto f = p.get_future().then(
+      [](auto g) { return g.then([](auto h) { return h.get() * 2; }); });
+  p.set_value(42);
+  EXPECT_TRUE(f.is_ready());
+  EXPECT_EQ(f.get(), 84);
+}
+
+/// @test Verify that `.then()` continuations are notified on abandoned futures.
+TEST(FutureTestInt, AbandonNotifiesContinuation) {
+  future<int> f;
+  {
+    promise<int> p;
+    f = p.get_future().then([](auto g) {
+#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
+      EXPECT_THROW(
+          try { g.get(); } catch (std::future_error const& ex) {
+            EXPECT_EQ(std::future_errc::broken_promise, ex.code());
+            throw;
+          },
+          std::future_error);
+#else
+      EXPECT_DEATH_IF_SUPPORTED(g.get(), "exceptions are disabled");
+#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
+      return 42;
+    });
+  }
+  EXPECT_EQ(f.get(), 42);
+}
+
 // The following tests reference the technical specification:
 //   http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0159r0.html
 // The test names match the section and paragraph from the TS.
@@ -234,9 +304,9 @@ TEST(FutureTestInt, conform_2_3_3_b) {
   EXPECT_TRUE(unwrapped.valid());
   EXPECT_FALSE(unwrapped.is_ready());
 
-#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
   p.set_exception(std::make_exception_ptr(std::runtime_error("test message")));
   EXPECT_TRUE(unwrapped.is_ready());
+#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
   EXPECT_THROW(
       try { unwrapped.get(); } catch (std::runtime_error const& ex) {
         EXPECT_THAT(ex.what(), HasSubstr("test message"));
@@ -245,8 +315,7 @@ TEST(FutureTestInt, conform_2_3_3_b) {
       std::runtime_error);
 #else
   EXPECT_DEATH_IF_SUPPORTED(
-      p.set_exception(
-          std::make_exception_ptr(std::runtime_error("test message"))),
+      unwrapped.get(),
       "future<T>::get\\(\\) had an exception but exceptions are disabled");
 #endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
 }
@@ -266,9 +335,9 @@ TEST(FutureTestInt, conform_2_3_3_c) {
   p.set_value(p2.get_future());
   EXPECT_FALSE(unwrapped.is_ready());
 
-#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
   p2.set_exception(std::make_exception_ptr(std::runtime_error("test message")));
   EXPECT_TRUE(unwrapped.is_ready());
+#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
   EXPECT_THROW(
       try { unwrapped.get(); } catch (std::runtime_error const& ex) {
         EXPECT_THAT(ex.what(), HasSubstr("test message"));
@@ -277,8 +346,7 @@ TEST(FutureTestInt, conform_2_3_3_c) {
       std::runtime_error);
 #else
   EXPECT_DEATH_IF_SUPPORTED(
-      p2.set_exception(
-          std::make_exception_ptr(std::runtime_error("test message"))),
+      unwrapped.get(),
       "future<T>::get\\(\\) had an exception but exceptions are disabled");
 #endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
 }
@@ -434,6 +502,7 @@ TEST(FutureTestInt, conform_2_3_8_d) {
   EXPECT_EQ(84, next.get());
 }
 
+#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
 /// @test Verify conformance with section 2.3 of the Concurrency TS.
 // NOLINTNEXTLINE(google-readability-avoid-underscore-in-googletest-name)
 TEST(FutureTestInt, conform_2_3_8_e) {
@@ -446,7 +515,6 @@ TEST(FutureTestInt, conform_2_3_8_e) {
     internal::ThrowRuntimeError("test exception in functor");
   });
   EXPECT_TRUE(next.valid());
-#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
   p.set_value(42);
   ASSERT_EQ(std::future_status::ready, next.wait_for(0_ms));
   EXPECT_THROW(
@@ -456,10 +524,8 @@ TEST(FutureTestInt, conform_2_3_8_e) {
       },
       std::runtime_error);
   EXPECT_FALSE(next.valid());
-#else
-  EXPECT_DEATH_IF_SUPPORTED(p.set_value(42), "test exception in functor");
-#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
 }
+#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
 
 /// @test Verify conformance with section 2.3 of the Concurrency TS.
 // NOLINTNEXTLINE(google-readability-avoid-underscore-in-googletest-name)
@@ -540,6 +606,7 @@ TEST(FutureTestInt, conform_2_3_9_c) {
   EXPECT_EQ(42, r.get());
 }
 
+#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
 /// @test Verify conformance with section 2.3 of the Concurrency TS.
 // NOLINTNEXTLINE(google-readability-avoid-underscore-in-googletest-name)
 TEST(FutureTestInt, conform_2_3_9_d) {
@@ -558,7 +625,6 @@ TEST(FutureTestInt, conform_2_3_9_d) {
   EXPECT_FALSE(r.is_ready());
   EXPECT_FALSE(f.valid());
 
-#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
   p.set_exception(std::make_exception_ptr(std::runtime_error("test message")));
   EXPECT_TRUE(called);
   EXPECT_TRUE(r.is_ready());
@@ -568,14 +634,6 @@ TEST(FutureTestInt, conform_2_3_9_d) {
         throw;
       },
       std::runtime_error);
-#else
-  // With exceptions disabled the program terminates as soon as the exception is
-  // set.
-  EXPECT_DEATH_IF_SUPPORTED(
-      p.set_exception(
-          std::make_exception_ptr(std::runtime_error("test message"))),
-      "future<T>::get\\(\\) had an exception but exceptions are disabled");
-#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
 }
 
 /// @test Verify conformance with section 2.3 of the Concurrency TS.
@@ -600,19 +658,14 @@ TEST(FutureTestInt, conform_2_3_9_e) {
   EXPECT_TRUE(called);
   EXPECT_TRUE(r.is_ready());
 
-#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
   EXPECT_THROW(
       try { r.get(); } catch (std::future_error const& ex) {
         EXPECT_EQ(std::future_errc::broken_promise, ex.code());
         throw;
       },
       std::future_error);
-#else
-  EXPECT_DEATH_IF_SUPPORTED(
-      r.get(),
-      "future<T>::get\\(\\) had an exception but exceptions are disabled");
-#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
 }
+#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
 
 /// @test Verify conformance with section 2.3 of the Concurrency TS.
 // NOLINTNEXTLINE(google-readability-avoid-underscore-in-googletest-name)

google/cloud/future_void.h

@@ -15,16 +15,10 @@
 #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_FUTURE_VOID_H
 #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_FUTURE_VOID_H
 
-/**
- * @file
- *
- * Specialize `future<T>` and `promise<T>` for void.
- */
-
 #include "google/cloud/internal/future_base.h"
 #include "google/cloud/internal/future_fwd.h"
 #include "google/cloud/internal/future_impl.h"
-#include "google/cloud/internal/future_then_meta.h"
+#include "google/cloud/internal/invoke_result.h"
 #include "google/cloud/version.h"
 #include <future>
 
@@ -68,7 +62,7 @@ class future<void> final : private internal::future_base<internal::FutureVoid> {
    * future's result type.
    */
   template <class T>
-  explicit future(future<T>&& rhs) : future<void>(rhs.then([](future<T>) {})) {}
+  explicit future(future<T>&& rhs);
 
   /**
    * Waits until the shared state becomes ready, then retrieves the value stored
@@ -109,30 +103,20 @@ class future<void> final : private internal::future_base<internal::FutureVoid> {
    * Side effects: `valid() == false` if the operation is successful.
    */
   template <typename F>
-  typename internal::then_helper<F, void>::future_t then(F&& func) {
-    check_valid();
-    using requires_unwrap_t =
-        typename internal::then_helper<F, void>::requires_unwrap_t;
-    return then_impl(std::forward<F>(func), requires_unwrap_t{});
-  }
+  auto then(F&& func) -> future<
+      /// @cond implementation_details
+      internal::UnwrappedType<internal::invoke_result_t<F, future<void>>>
+      /// @endcond
+      >;
 
   explicit future(std::shared_ptr<shared_state_type> state)
       : future_base<internal::FutureVoid>(std::move(state)) {}
 
  private:
-  /// Implement `then()` if the result does not require unwrapping.
-  template <typename F>
-  typename internal::then_helper<F, void>::future_t then_impl(F&& functor,
-                                                              std::false_type);
-
-  /// Implement `then()` if the result requires unwrapping.
-  template <typename F>
-  typename internal::then_helper<F, void>::future_t then_impl(F&& functor,
-                                                              std::true_type);
-
   template <typename U>
   friend class future;
 
+  friend struct internal::FutureThenImpl;
   friend struct internal::CoroutineSupport;
 };