From 1df7efd261444bb8e9d8f51332fb2cd6cd83d598 Mon Sep 17 00:00:00 2001
From: Xavier Deguillard <xavierd@fb.com>
Date: Thu, 14 Jul 2022 13:51:39 -0700
Subject: [PATCH] utils: fix compilation in ImmediateFuture

Summary:
When the ImmediateFuture is holding a SemiFuture, the compiler complains about
the use of `std::forward`, let's fix that and add few tests to verify that it
now compiles well.

Reviewed By: fanzeyi

Differential Revision: D37861191

fbshipit-source-id: 590f22f1fdbfda7e7106730a7d77ddfd5b08d815
---
 eden/fs/utils/ImmediateFuture-inl.h        |  4 ++--
 eden/fs/utils/test/ImmediateFutureTest.cpp | 27 ++++++++++++++++++++--
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/eden/fs/utils/ImmediateFuture-inl.h b/eden/fs/utils/ImmediateFuture-inl.h
index c0f39a1a64..e49865bd72 100644
--- a/eden/fs/utils/ImmediateFuture-inl.h
+++ b/eden/fs/utils/ImmediateFuture-inl.h
@@ -99,10 +99,10 @@ makeImmediateFutureFromImmediate(Func&& func, Args... args) {
     // In the case where Func returns void, force the return value to
     // be folly::unit.
     if constexpr (std::is_same_v<FuncRetType, void>) {
-      func(std::forward<Args...>(args...));
+      func(std::forward<Args>(args)...);
       return folly::unit;
     } else {
-      return func(std::forward<Args...>(args...));
+      return func(std::forward<Args>(args)...);
     }
   } catch (std::exception& ex) {
     return folly::Try<NewType>(
diff --git a/eden/fs/utils/test/ImmediateFutureTest.cpp b/eden/fs/utils/test/ImmediateFutureTest.cpp
index 31746085c5..6489a2f564 100644
--- a/eden/fs/utils/test/ImmediateFutureTest.cpp
+++ b/eden/fs/utils/test/ImmediateFutureTest.cpp
@@ -549,7 +549,30 @@ TEST(ImmediateFuture, thenError) {
 
 TEST(ImmediateFuture, thenErrorVoid) {
   ImmediateFuture<folly::Unit> unitFut{folly::unit};
-  std::move(unitFut).thenError(
+  auto fut = std::move(unitFut).thenError(
       [](folly::exception_wrapper exc) { exc.throw_exception(); });
-  EXPECT_EQ(std::move(unitFut).get(), folly::unit);
+  EXPECT_EQ(std::move(fut).get(), folly::unit);
+}
+
+TEST(ImmediateFuture, thenErrorSemiValue) {
+  auto [promise, semiFut] = folly::makePromiseContract<folly::Unit>();
+  ImmediateFuture<folly::Unit> fut{std::move(semiFut)};
+  auto thenErrorFut = std::move(fut).thenError(
+      [](folly::exception_wrapper exc) { exc.throw_exception(); });
+  promise.setValue(folly::unit);
+  EXPECT_EQ(std::move(thenErrorFut).get(), folly::unit);
+}
+
+TEST(ImmediateFuture, thenErrorSemiError) {
+  auto [promise, semiFut] = folly::makePromiseContract<folly::Unit>();
+  ImmediateFuture<folly::Unit> fut{std::move(semiFut)};
+  auto thenErrorFut =
+      std::move(fut).thenError([](folly::exception_wrapper exc) {
+        // Re-throw with a different type so we can test that the original
+        // exception was caught.
+        throw std::runtime_error(folly::exceptionStr(exc).toStdString());
+      });
+  promise.setException(std::logic_error("Test exception"));
+  EXPECT_THROW_RE(
+      std::move(thenErrorFut).get(), std::runtime_error, "Test exception");
 }