SERVER-52939 Expand Promise::setFrom()

Promise<void> can now be set from a Status and Promise<T> now rejects
being implicitly set from a Status or T.

11 files changed, 129 insertions, 37 deletions

diff --git a/src/mongo/db/s/active_shard_collection_registry.cpp b/src/mongo/db/s/active_shard_collection_registry.cpp
index bd6e9f0f824..837c6118cd5 100644
--- a/src/mongo/db/s/active_shard_collection_registry.cpp
+++ b/src/mongo/db/s/active_shard_collection_registry.cpp
@@ -127,7 +127,7 @@ void ActiveShardCollectionRegistry::_setUUIDOrError(std::string nss,
     auto iter = _activeShardCollectionMap.find(nss);
     invariant(iter != _activeShardCollectionMap.end());
     auto activeShardCollectionState = iter->second;
-    activeShardCollectionState->_uuidPromise.setFromStatusWith(swUUID);
+    activeShardCollectionState->_uuidPromise.setFrom(swUUID);
 }
 
 Status ActiveShardCollectionRegistry::ActiveShardCollectionState::constructErrorStatus(
diff --git a/src/mongo/db/vector_clock_mongod.cpp b/src/mongo/db/vector_clock_mongod.cpp
index 90c7261bbb7..c369a49382a 100644
--- a/src/mongo/db/vector_clock_mongod.cpp
+++ b/src/mongo/db/vector_clock_mongod.cpp
@@ -380,7 +380,7 @@ Future<void> VectorClockMongoD::_doWhileQueueNotEmptyOrError(ServiceContext* ser
         })
         .getAsync([this, promise = std::move(p)](
                       StatusWith<std::pair<uint64_t, VectorTime>> swResult) mutable {
-            promise.setFromStatusWith(std::move(swResult));
+            promise.setFrom(std::move(swResult));
         });
 
     return future;
diff --git a/src/mongo/executor/network_interface_tl.cpp b/src/mongo/executor/network_interface_tl.cpp
index 28ef98de1e1..7d3a5fb39fa 100644
--- a/src/mongo/executor/network_interface_tl.cpp
+++ b/src/mongo/executor/network_interface_tl.cpp
@@ -608,7 +608,7 @@ void NetworkInterfaceTL::CommandStateBase::doMetadataHook(
 
 void NetworkInterfaceTL::CommandState::fulfillFinalPromise(
     StatusWith<RemoteCommandOnAnyResponse> response) {
-    promise.setFromStatusWith(std::move(response));
+    promise.setFrom(std::move(response));
 }
 
 NetworkInterfaceTL::RequestManager::RequestManager(CommandStateBase* cmdState_)
diff --git a/src/mongo/executor/scoped_task_executor.cpp b/src/mongo/executor/scoped_task_executor.cpp
index 4d314dabeee..c5b60e81913 100644
--- a/src/mongo/executor/scoped_task_executor.cpp
+++ b/src/mongo/executor/scoped_task_executor.cpp
@@ -68,7 +68,7 @@ public:
                 // We are guaranteed that no more callbacks can be added to _cbHandles after
                 // _inShutdown is set to true. If there aren't any callbacks outstanding, then it is
                 // shutdown()'s responsibility to make the futures returned by joinAll() ready.
-                _promise.setWith([] {});
+                _promise.emplaceValue();
             }
             _inShutdown = true;
 
@@ -341,7 +341,7 @@ private:
             // We are guaranteed that no more callbacks can be added to _cbHandles after _inShutdown
             // is set to true. If there are no more callbacks outstanding, then it is the last
             // callback's responsibility to make the futures returned by joinAll() ready.
-            _promise.setWith([] {});
+            _promise.emplaceValue();
         }
     }
 
diff --git a/src/mongo/util/future.h b/src/mongo/util/future.h
index 39547609de4..9bd8ffe9167 100644
--- a/src/mongo/util/future.h
+++ b/src/mongo/util/future.h
@@ -755,6 +755,7 @@ ExecutorFuture(ExecutorPtr)->ExecutorFuture<void>;
 template <typename T>
 class Promise {
     using SharedStateT = future_details::SharedState<T>;
+    using T_unless_void = typename SemiFuture<T>::T_unless_void;
 
 public:
     using value_type = T;
@@ -810,11 +811,30 @@ public:
      * involved.
      */
     void setFrom(Future<T>&& future) noexcept {
-        setImpl([&](boost::intrusive_ptr<future_details::SharedState<T>>&& sharedState) {
+        setImpl([&](boost::intrusive_ptr<SharedStateT>&& sharedState) {
             std::move(future).propagateResultTo(sharedState.get());
         });
     }
 
+    /**
+     * Sets the value into this Promise immediately.
+     *
+     * This accepts a Status for Promises<void> or a StatusWith<T> for Promise<T>.
+     */
+    void setFrom(StatusOrStatusWith<T> sosw) noexcept {
+        setImpl([&](boost::intrusive_ptr<SharedStateT>&& sharedState) {
+            sharedState->setFrom(std::move(sosw));
+        });
+    }
+
+    // Use emplaceValue(Args&&...) instead.
+    REQUIRES_FOR_NON_TEMPLATE(!std::is_void_v<T>)
+    void setFrom(T_unless_void val) noexcept = delete;
+
+    // Use setError(Status) instead.
+    REQUIRES_FOR_NON_TEMPLATE(!std::is_void_v<T>)
+    void setFrom(Status) noexcept = delete;
+
     TEMPLATE(typename... Args)
     REQUIRES(std::is_constructible_v<T, Args...> || (std::is_void_v<T> && sizeof...(Args) == 0))
     void emplaceValue(Args&&... args) noexcept {
@@ -830,13 +850,6 @@ public:
         });
     }
 
-    // TODO rename to not XXXWith and handle void
-    void setFromStatusWith(StatusWith<T> sw) noexcept {
-        setImpl([&](boost::intrusive_ptr<SharedStateT>&& sharedState) {
-            sharedState->setFromStatusWith(std::move(sw));
-        });
-    }
-
     static auto makePromiseFutureImpl() {
         struct PromiseAndFuture {
             Promise<T> promise = Promise(make_intrusive<SharedStateT>());
@@ -1038,6 +1051,9 @@ SharedSemiFuture(StatusWith<T>)->SharedSemiFuture<T>;
  */
 template <typename T>
 class SharedPromise {
+    using SharedStateT = future_details::SharedState<T>;
+    using T_unless_void = std::conditional_t<std::is_void_v<T>, future_details::FakeVoid, T>;
+
 public:
     using value_type = T;
 
@@ -1071,11 +1087,33 @@ public:
         setFrom(Future<void>::makeReady().then(std::forward<Func>(func)));
     }
 
+    /**
+     * Sets the value into this SharedPromise when the passed-in Future completes, which may have
+     * already happened.
+     */
     void setFrom(Future<T>&& future) noexcept {
         invariant(!std::exchange(_haveCompleted, true));
         std::move(future).propagateResultTo(_sharedState.get());
     }
 
+    /**
+     * Sets the value into this SharedPromise immediately.
+     *
+     * This accepts a Status for SharedPromises<void> or a StatusWith<T> for SharedPromise<T>.
+     */
+    void setFrom(StatusOrStatusWith<T> sosw) noexcept {
+        invariant(!std::exchange(_haveCompleted, true));
+        _sharedState->setFrom(std::move(sosw));
+    }
+
+    // Use emplaceValue(Args&&...) instead.
+    REQUIRES_FOR_NON_TEMPLATE(!std::is_void_v<T>)
+    void setFrom(T_unless_void val) noexcept = delete;
+
+    // Use setError(Status) instead.
+    REQUIRES_FOR_NON_TEMPLATE(!std::is_void_v<T>)
+    void setFrom(Status) noexcept = delete;
+
     TEMPLATE(typename... Args)
     REQUIRES(std::is_constructible_v<T, Args...> || (std::is_void_v<T> && sizeof...(Args) == 0))
     void emplaceValue(Args&&... args) noexcept {
@@ -1089,20 +1127,13 @@ public:
         _sharedState->setError(std::move(status));
     }
 
-    // TODO rename to not XXXWith and handle void
-    void setFromStatusWith(StatusWith<T> sw) noexcept {
-        invariant(!std::exchange(_haveCompleted, true));
-        _sharedState->setFromStatusWith(std::move(sw));
-    }
-
 private:
     friend class Future<void>;
 
     // This is slightly different from whether the SharedState is in kFinished, because this
     // SharedPromise may have been completed with a Future that isn't ready yet.
     bool _haveCompleted = false;
-    const boost::intrusive_ptr<future_details::SharedState<T>> _sharedState =
-        make_intrusive<future_details::SharedState<T>>();
+    const boost::intrusive_ptr<SharedStateT> _sharedState = make_intrusive<SharedStateT>();
 };
 
 /**
diff --git a/src/mongo/util/future_impl.h b/src/mongo/util/future_impl.h
index ed0e17296df..a23b5e75114 100644
--- a/src/mongo/util/future_impl.h
+++ b/src/mongo/util/future_impl.h
@@ -592,11 +592,20 @@ struct SharedStateImpl final : SharedStateBase {
         transitionToFinished();
     }
 
-    void setFromStatusWith(StatusWith<T> sw) {
-        if (sw.isOK()) {
-            emplaceValue(std::move(sw.getValue()));
+    void setFrom(StatusWith<T> sosw) {
+        if (sosw.isOK()) {
+            emplaceValue(std::move(sosw.getValue()));
         } else {
-            setError(std::move(sw.getStatus()));
+            setError(std::move(sosw.getStatus()));
+        }
+    }
+
+    REQUIRES_FOR_NON_TEMPLATE(std::is_same_v<T, FakeVoid>)
+    void setFrom(Status status) {
+        if (status.isOK()) {
+            emplaceValue();
+        } else {
+            setError(std::move(status));
         }
     }
 
@@ -896,7 +905,7 @@ public:
                         if (!input->status.isOK())
                             return output->setError(std::move(input->status));
 
-                        output->setFromStatusWith(statusCall(func, std::move(*input->data)));
+                        output->setFrom(statusCall(func, std::move(*input->data)));
                     });
                 });
         } else {
@@ -953,11 +962,10 @@ public:
                     return makeContinuation<Result>([func = std::forward<Func>(func)](
                         SharedState<T> * input, SharedState<Result> * output) mutable noexcept {
                         if (!input->status.isOK())
-                            return output->setFromStatusWith(
+                            return output->setFrom(
                                 statusCall(func, Wrapper(std::move(input->status))));
 
-                        output->setFromStatusWith(
-                            statusCall(func, Wrapper(std::move(*input->data))));
+                        output->setFrom(statusCall(func, Wrapper(std::move(*input->data))));
                     });
                 });
         } else {
@@ -1030,7 +1038,7 @@ public:
                         if (input->status.isOK())
                             return output->emplaceValue(std::move(*input->data));
 
-                        output->setFromStatusWith(statusCall(func, std::move(input->status)));
+                        output->setFrom(statusCall(func, std::move(input->status)));
                     });
                 });
         } else {
diff --git a/src/mongo/util/future_test_promise_int.cpp b/src/mongo/util/future_test_promise_int.cpp
index a74fc1e52e4..38bc194688c 100644
--- a/src/mongo/util/future_test_promise_int.cpp
+++ b/src/mongo/util/future_test_promise_int.cpp
@@ -34,13 +34,19 @@
 #include "mongo/stdx/thread.h"
 #include "mongo/unittest/death_test.h"
 #include "mongo/unittest/unittest.h"
+#include "mongo/util/concepts.h"
 
 #include "mongo/util/future_test_utils.h"
 
 namespace mongo {
 namespace {
 
-TEST(Promise, Success_setFrom) {
+static_assert(!canSetFrom<Promise<int>, int>, "Use Promise<T>::emplaceValue(...) instead");
+static_assert(!canSetFrom<Promise<int>, Status>, "Use Promise<T>::setError(...) instead");
+static_assert(canSetFrom<Promise<int>, StatusWith<int>>);
+static_assert(canSetFrom<Promise<int>, Future<int>>);
+
+TEST(Promise, Success_setFrom_future) {
     FUTURE_SUCCESS_TEST<kNoExecutorFuture_needsPromiseSetFrom>(
         [] { return 1; },
         [](/*Future<int>*/ auto&& fut) {
@@ -50,7 +56,13 @@ TEST(Promise, Success_setFrom) {
         });
 }
 
-TEST(Promise, Fail_setFrom) {
+TEST(Promise, Success_setFrom_statusWith) {
+    auto pf = makePromiseFuture<int>();
+    pf.promise.setFrom(StatusWith<int>(3));
+    ASSERT_EQ(std::move(pf.future).getNoThrow(), 3);
+}
+
+TEST(Promise, Fail_setFrom_future) {
     FUTURE_FAIL_TEST<int, kNoExecutorFuture_needsPromiseSetFrom>([](/*Future<int>*/ auto&& fut) {
         auto pf = makePromiseFuture<int>();
         pf.promise.setFrom(std::move(fut));
@@ -58,6 +70,12 @@ TEST(Promise, Fail_setFrom) {
     });
 }
 
+TEST(Promise, Fail_setFrom_statusWith_error) {
+    auto pf = makePromiseFuture<int>();
+    pf.promise.setFrom(StatusWith<int>(failStatus()));
+    ASSERT_THROWS_failStatus(std::move(pf.future).get());
+}
+
 TEST(Promise, Success_setWith_value) {
     auto pf = makePromiseFuture<int>();
     pf.promise.setWith([&] { return 1; });
diff --git a/src/mongo/util/future_test_promise_void.cpp b/src/mongo/util/future_test_promise_void.cpp
index 3ef9731e7a7..c57f5db1b78 100644
--- a/src/mongo/util/future_test_promise_void.cpp
+++ b/src/mongo/util/future_test_promise_void.cpp
@@ -40,7 +40,11 @@
 namespace mongo {
 namespace {
 
-TEST(Promise_void, Success_setFrom) {
+static_assert(!canSetFrom<Promise<void>, void>, "Use Promise<T>::emplaceValue() instead");
+static_assert(canSetFrom<Promise<void>, Status>);
+static_assert(canSetFrom<Promise<void>, Future<void>>);
+
+TEST(Promise_void, Success_setFrom_future) {
     FUTURE_SUCCESS_TEST<kNoExecutorFuture_needsPromiseSetFrom>(
         [] {},
         [](/*Future<void>*/ auto&& fut) {
@@ -51,7 +55,13 @@ TEST(Promise_void, Success_setFrom) {
         });
 }
 
-TEST(Promise_void, Fail_setFrom) {
+TEST(Promise_void, Success_setFrom_status) {
+    auto pf = makePromiseFuture<void>();
+    pf.promise.setFrom(Status::OK());
+    ASSERT_OK(std::move(pf.future).getNoThrow());
+}
+
+TEST(Promise_void, Fail_setFrom_future) {
     FUTURE_FAIL_TEST<void, kNoExecutorFuture_needsPromiseSetFrom>([](/*Future<void>*/ auto&& fut) {
         auto pf = makePromiseFuture<void>();
         pf.promise.setFrom(std::move(fut));
@@ -59,6 +69,12 @@ TEST(Promise_void, Fail_setFrom) {
     });
 }
 
+TEST(Promise_void, Fail_setFrom_status) {
+    auto pf = makePromiseFuture<void>();
+    pf.promise.setFrom(failStatus());
+    ASSERT_THROWS_failStatus(std::move(pf.future).get());
+}
+
 TEST(Promise_void, Success_setWith_value) {
     auto pf = makePromiseFuture<void>();
     pf.promise.setWith([&] {});
diff --git a/src/mongo/util/future_test_utils.h b/src/mongo/util/future_test_utils.h
index 374f00f7a7f..54154306fc4 100644
--- a/src/mongo/util/future_test_utils.h
+++ b/src/mongo/util/future_test_utils.h
@@ -34,6 +34,7 @@
 #include "mongo/stdx/thread.h"
 #include "mongo/unittest/death_test.h"
 #include "mongo/unittest/unittest.h"
+#include "mongo/util/concepts.h"
 #include "mongo/util/executor_test_util.h"
 
 #if !defined(__has_feature)
@@ -207,4 +208,22 @@ void FUTURE_FAIL_TEST(const TestFunc& test) {
         test(Future<CompletionType>::makeReady(failStatus()).thenRunOn(exec));
     }
 }
+
+/**
+ * True if PromiseT::setFrom(ArgT) is valid.
+ */
+template <typename PromiseT, typename ArgT, typename = void>
+inline constexpr bool canSetFrom = false;
+
+template <typename PromiseT>
+inline constexpr bool canSetFrom<PromiseT,  //
+                                 void,      //
+                                 decltype(std::declval<PromiseT&>().setFrom())> = true;
+
+template <typename PromiseT, typename ArgT>
+inline constexpr bool
+    canSetFrom<PromiseT,  //
+               ArgT,      //
+               decltype(std::declval<PromiseT&>().setFrom(std::declval<ArgT>()))> = true;
+
 }  // namespace mongo
diff --git a/src/mongo/util/read_through_cache.h b/src/mongo/util/read_through_cache.h
index 24c2c410451..6fb9b63de14 100644
--- a/src/mongo/util/read_through_cache.h
+++ b/src/mongo/util/read_through_cache.h
@@ -541,9 +541,9 @@ private:
             promisesToSet.pop_back();
 
             if (promisesToSet.empty())
-                p->setFromStatusWith(std::move(result));
+                p->setFrom(std::move(result));
             else
-                p->setFromStatusWith(result);
+                p->setFrom(result);
         }
 
         return mustDoAnotherLoop
diff --git a/src/mongo/util/read_through_cache_test.cpp b/src/mongo/util/read_through_cache_test.cpp
index 855b75cae34..8e7de5c4d2f 100644
--- a/src/mongo/util/read_through_cache_test.cpp
+++ b/src/mongo/util/read_through_cache_test.cpp
@@ -434,7 +434,7 @@ TEST_F(ReadThroughCacheAsyncTest, FailedInProgressLookupForNotCausallyConsistent
     ASSERT(inProgress.valid(WithLock::withoutLock()));
     auto promisesToSet = inProgress.getAllPromisesOnError(WithLock::withoutLock());
     ASSERT_EQ(1U, promisesToSet.size());
-    promisesToSet.front()->setFromStatusWith(asyncLookupResult.getStatus());
+    promisesToSet.front()->setError(asyncLookupResult.getStatus());
 
     ASSERT(future.isReady());
     ASSERT_THROWS_CODE(future.get(), DBException, ErrorCodes::InternalError);