SERVER-41372 Constrain callback-taking methods on Futures

This reverts commit d60d7d2985f8a75ed08a7836cb5e460415f2e26d.

2 files changed, 106 insertions, 47 deletions

diff --git a/src/mongo/util/future.h b/src/mongo/util/future.h
index cd1a24066c4..5bef6ff3209 100644
--- a/src/mongo/util/future.h
+++ b/src/mongo/util/future.h
@@ -144,7 +144,7 @@ public:
         return SemiFuture(Impl::makeReady(std::move(val)));
     }
 
-    template <typename U = T, typename = std::enable_if_t<std::is_void_v<U>>>
+    REQUIRES_FOR_NON_TEMPLATE(std::is_void_v<T>)
     static SemiFuture<void> makeReady() {
         return SemiFuture(Impl::makeReady());
     }
@@ -281,8 +281,8 @@ private:
 };
 
 // Deduction Guides
-template <typename T,
-          typename = std::enable_if_t<!isStatusOrStatusWith<T> && !future_details::isFutureLike<T>>>
+TEMPLATE(typename T)
+REQUIRES(!isStatusOrStatusWith<T> && !future_details::isFutureLike<T>)
 SemiFuture(T)->SemiFuture<T>;
 template <typename T>
 SemiFuture(StatusWith<T>)->SemiFuture<T>;
@@ -327,7 +327,7 @@ public:
     static Future<T> makeReady(StatusWith<T_unless_void> val) {
         return Future(Impl::makeReady(std::move(val)));
     }
-    template <typename U = T, typename = std::enable_if_t<std::is_void_v<U>>>
+    REQUIRES_FOR_NON_TEMPLATE(std::is_void_v<T>)
     static Future<void> makeReady() {
         return Future(Impl::makeReady());
     }
@@ -343,8 +343,9 @@ public:
      * For now, the callback must not fail, since there is nowhere to propagate the error to.
      * TODO decide how to handle func throwing.
      */
-    template <typename Func>
-        void getAsync(Func&& func) && noexcept {
+    TEMPLATE(typename Func)
+    REQUIRES(future_details::isCallableExactR<void, Func, StatusOrStatusWith<T>>)
+    void getAsync(Func&& func) && noexcept {
         std::move(this->_impl).getAsync(std::forward<Func>(func));
     }
 
@@ -383,8 +384,9 @@ public:
      * The callback takes a T and can return anything (see above for how Statusy and Futurey returns
      * are handled.)
      */
-    template <typename Func>
-        /*see above*/ auto then(Func&& func) && noexcept {
+    TEMPLATE(typename Func)
+    REQUIRES(future_details::isCallable<Func, T>)
+    /*see above*/ auto then(Func&& func) && noexcept {
         return wrap<Func, T>(std::move(this->_impl).then(std::forward<Func>(func)));
     }
 
@@ -395,8 +397,9 @@ public:
      * The callback takes a StatusOrStatusWith<T> and can return anything (see above for how Statusy
      * and Futurey returns are handled.)
      */
-    template <typename Func>
-        /*see above*/ auto onCompletion(Func&& func) && noexcept {
+    TEMPLATE(typename Func)
+    REQUIRES(future_details::isCallable<Func, StatusOrStatusWith<T>>)
+    /*see above*/ auto onCompletion(Func&& func) && noexcept {
         return wrap<Func, Status>(std::move(this->_impl).onCompletion(std::forward<Func>(func)));
     }
 
@@ -413,8 +416,9 @@ public:
      * The callback takes a non-OK Status and returns a possibly-wrapped T (see above for how
      * Statusy and Futurey returns are handled.)
      */
-    template <typename Func>
-        /*see above*/ auto onError(Func&& func) && noexcept {
+    TEMPLATE(typename Func)
+    REQUIRES(future_details::isCallableR<T, Func, Status>)
+    /*see above*/ auto onError(Func&& func) && noexcept {
         return wrap<Func, Status>(std::move(this->_impl).onError(std::forward<Func>(func)));
     }
 
@@ -425,8 +429,9 @@ public:
      * The callback takes a non-OK Status and returns a possibly-wrapped T (see above for how
      * Statusy and Futurey returns are handled.)
      */
-    template <ErrorCodes::Error code, typename Func>
-        /*see above*/ auto onError(Func&& func) && noexcept {
+    TEMPLATE(ErrorCodes::Error code, typename Func)
+    REQUIRES(future_details::isCallableR<T, Func, Status>)
+    /*see above*/ auto onError(Func&& func) && noexcept {
         return wrap<Func, Status>(
             std::move(this->_impl).template onError<code>(std::forward<Func>(func)));
     }
@@ -438,8 +443,9 @@ public:
      * The callback takes a non-OK Status and returns a possibly-wrapped T (see above for how
      * Statusy and Futurey returns are handled.)
      */
-    template <ErrorCategory category, typename Func>
-        /*see above*/ auto onErrorCategory(Func&& func) && noexcept {
+    TEMPLATE(ErrorCategory category, typename Func)
+    REQUIRES(future_details::isCallableR<T, Func, Status>)
+    /*see above*/ auto onErrorCategory(Func&& func) && noexcept {
         return wrap<Func, Status>(
             std::move(this->_impl).template onErrorCategory<category>(std::forward<Func>(func)));
     }
@@ -464,8 +470,9 @@ public:
      *
      * The callback takes a const T& and must return void.
      */
-    template <typename Func>
-        Future<T> tap(Func&& func) && noexcept {
+    TEMPLATE(typename Func)
+    REQUIRES(future_details::isCallableExactR<void, Func, const T>)
+    Future<T> tap(Func&& func) && noexcept {
         return Future<T>(std::move(this->_impl).tap(std::forward<Func>(func)));
     }
 
@@ -476,8 +483,9 @@ public:
      *
      * The callback takes a non-OK Status and must return void.
      */
-    template <typename Func>
-        Future<T> tapError(Func&& func) && noexcept {
+    TEMPLATE(typename Func)
+    REQUIRES(future_details::isCallableExactR<void, Func, const Status>)
+    Future<T> tapError(Func&& func) && noexcept {
         return Future<T>(std::move(this->_impl).tapError(std::forward<Func>(func)));
     }
 
@@ -489,8 +497,9 @@ public:
      *
      * The callback takes a StatusOrStatusWith<T> and must return void.
      */
-    template <typename Func>
-        Future<T> tapAll(Func&& func) && noexcept {
+    TEMPLATE(typename Func)
+    REQUIRES(future_details::isCallableExactR<void, Func, const StatusOrStatusWith<T>>)
+    Future<T> tapAll(Func&& func) && noexcept {
         return Future<T>(std::move(this->_impl).tapAll(std::forward<Func>(func)));
     }
 
@@ -514,8 +523,8 @@ private:
 };
 
 // Deduction Guides
-template <typename T,
-          typename = std::enable_if_t<!isStatusOrStatusWith<T> && !future_details::isFutureLike<T>>>
+TEMPLATE(typename T)
+REQUIRES(!isStatusOrStatusWith<T> && !future_details::isFutureLike<T>)
 Future(T)->Future<T>;
 template <typename T>
 Future(StatusWith<T>)->Future<T>;
@@ -556,7 +565,7 @@ public:
         static_assert(!std::is_void_v<T>);
     }
 
-    template <typename U = T, typename = std::enable_if_t<std::is_void_v<U>>>
+    REQUIRES_FOR_NON_TEMPLATE(std::is_void_v<T>)
     explicit ExecutorFuture(ExecutorPtr exec) : SemiFuture<void>(), _exec(std::move(exec)) {}
 
     /**
@@ -592,8 +601,9 @@ public:
     // it should be doable, but will be fairly complicated.
     //
 
-    template <typename Func>
-        void getAsync(Func&& func) && noexcept {
+    TEMPLATE(typename Func)
+    REQUIRES(future_details::isCallableExactR<void, Func, StatusOrStatusWith<T>>)
+    void getAsync(Func&& func) && noexcept {
         static_assert(std::is_void_v<decltype(func(std::declval<StatusOrStatusWith<T>>()))>,
                       "func passed to getAsync must return void");
 
@@ -611,37 +621,42 @@ public:
         });
     }
 
-    template <typename Func>
-        auto then(Func&& func) && noexcept {
+    TEMPLATE(typename Func)
+    REQUIRES(future_details::isCallable<Func, T>)
+    auto then(Func&& func) && noexcept {
         return mongo::ExecutorFuture(
             std::move(_exec), std::move(this->_impl).then(wrapCB<T>(std::forward<Func>(func))));
     }
 
-    template <typename Func>
-        auto onCompletion(Func&& func) && noexcept {
+    TEMPLATE(typename Func)
+    REQUIRES(future_details::isCallable<Func, StatusOrStatusWith<T>>)
+    auto onCompletion(Func&& func) && noexcept {
         return mongo::ExecutorFuture(
             std::move(_exec),
             std::move(this->_impl)
                 .onCompletion(wrapCB<StatusOrStatusWith<T>>(std::forward<Func>(func))));
     }
 
-    template <typename Func>
-        ExecutorFuture<T> onError(Func&& func) && noexcept {
+    TEMPLATE(typename Func)
+    REQUIRES(future_details::isCallableR<T, Func, Status>)
+    ExecutorFuture<T> onError(Func&& func) && noexcept {
         return mongo::ExecutorFuture(
             std::move(_exec),
             std::move(this->_impl).onError(wrapCB<Status>(std::forward<Func>(func))));
     }
 
-    template <ErrorCodes::Error code, typename Func>
-        ExecutorFuture<T> onError(Func&& func) && noexcept {
+    TEMPLATE(ErrorCodes::Error code, typename Func)
+    REQUIRES(future_details::isCallableR<T, Func, Status>)
+    ExecutorFuture<T> onError(Func&& func) && noexcept {
         return mongo::ExecutorFuture(
             std::move(_exec),
             std::move(this->_impl)
                 .template onError<code>(wrapCB<Status>(std::forward<Func>(func))));
     }
 
-    template <ErrorCategory category, typename Func>
-        ExecutorFuture<T> onErrorCategory(Func&& func) && noexcept {
+    TEMPLATE(ErrorCategory category, typename Func)
+    REQUIRES(future_details::isCallableR<T, Func, Status>)
+    ExecutorFuture<T> onErrorCategory(Func&& func) && noexcept {
         return mongo::ExecutorFuture(
             std::move(_exec),
             std::move(this->_impl)
@@ -692,8 +707,8 @@ private:
 };
 
 // Deduction Guides
-template <typename T,
-          typename = std::enable_if_t<!isStatusOrStatusWith<T> && !future_details::isFutureLike<T>>>
+TEMPLATE(typename T)
+REQUIRES(!isStatusOrStatusWith<T> && !future_details::isFutureLike<T>)
 ExecutorFuture(ExecutorPtr, T)->ExecutorFuture<T>;
 template <typename T>
 ExecutorFuture(ExecutorPtr, future_details::FutureImpl<T>)->ExecutorFuture<T>;
@@ -766,7 +781,8 @@ public:
      * because this method will correctly propagate errors thrown from makeResult(), rather than
      * ErrorCodes::BrokenPromise.
      */
-    template <typename Func>
+    TEMPLATE(typename Func)
+    REQUIRES(future_details::isCallableR<T, Func, void>)
     void setWith(Func&& func) noexcept {
         setFrom(Future<void>::makeReady().then(std::forward<Func>(func)));
     }
@@ -782,7 +798,8 @@ public:
         });
     }
 
-    template <typename... Args>
+    TEMPLATE(typename... Args)
+    REQUIRES(std::is_constructible_v<T, Args...> || (std::is_void_v<T> && sizeof...(Args) == 0))
     void emplaceValue(Args&&... args) noexcept {
         setImpl([&](boost::intrusive_ptr<SharedStateT>&& sharedState) {
             sharedState->emplaceValue(std::forward<Args>(args)...);
@@ -963,8 +980,8 @@ private:
 };
 
 // Deduction Guides
-template <typename T,
-          typename = std::enable_if_t<!isStatusOrStatusWith<T> && !future_details::isFutureLike<T>>>
+TEMPLATE(typename T)
+REQUIRES(!isStatusOrStatusWith<T> && !future_details::isFutureLike<T>)
 SharedSemiFuture(T)->SharedSemiFuture<T>;
 template <typename T>
 SharedSemiFuture(StatusWith<T>)->SharedSemiFuture<T>;
@@ -1014,7 +1031,8 @@ public:
         return SharedSemiFuture<T>(_sharedState);
     }
 
-    template <typename Func>
+    TEMPLATE(typename Func)
+    REQUIRES(future_details::isCallableR<T, Func, void>)
     void setWith(Func&& func) noexcept {
         invariant(!std::exchange(_haveCompleted, true));
         setFrom(Future<void>::makeReady().then(std::forward<Func>(func)));
@@ -1025,7 +1043,8 @@ public:
         std::move(future).propagateResultTo(_sharedState.get());
     }
 
-    template <typename... Args>
+    TEMPLATE(typename... Args)
+    REQUIRES(std::is_constructible_v<T, Args...> || (std::is_void_v<T> && sizeof...(Args) == 0))
     void emplaceValue(Args&&... args) noexcept {
         invariant(!std::exchange(_haveCompleted, true));
         _sharedState->emplaceValue(std::forward<Args>(args)...);
@@ -1058,7 +1077,8 @@ private:
  * Promise::setWith, and has the same reasons to prefer it over Future<T>::makeReady(). Also, it
  * deduces the T, so it is easier to use.
  */
-template <typename Func>
+TEMPLATE(typename Func)
+REQUIRES(future_details::isCallable<Func, void>)
 auto makeReadyFutureWith(Func&& func) {
     return Future<void>::makeReady().then(std::forward<Func>(func));
 }
diff --git a/src/mongo/util/future_impl.h b/src/mongo/util/future_impl.h
index bb9a467710d..19adb0aa928 100644
--- a/src/mongo/util/future_impl.h
+++ b/src/mongo/util/future_impl.h
@@ -50,7 +50,6 @@
 #include "mongo/util/scopeguard.h"
 
 namespace mongo {
-
 template <typename T>
 class Promise;
 
@@ -70,6 +69,7 @@ template <typename T>
 class SharedSemiFuture;
 
 namespace future_details {
+
 template <typename T>
 class FutureImpl;
 template <>
@@ -188,6 +188,45 @@ using VoidToFakeVoid = std::conditional_t<std::is_void_v<T>, FakeVoid, T>;
 template <typename T>
 using FakeVoidToVoid = std::conditional_t<std::is_same_v<T, FakeVoid>, void, T>;
 
+struct InvalidCallSentinal;  // Nothing actually returns this.
+template <typename Func, typename Arg, typename = void>
+struct FriendlyInvokeResultImpl {
+    using type = InvalidCallSentinal;
+};
+template <typename Func, typename Arg>
+struct FriendlyInvokeResultImpl<
+    Func,
+    Arg,
+    std::enable_if_t<std::is_invocable_v<Func, std::enable_if_t<!std::is_void_v<Arg>, Arg>>>> {
+    using type = std::invoke_result_t<Func, Arg>;
+};
+template <typename Func>
+struct FriendlyInvokeResultImpl<Func, void, std::enable_if_t<std::is_invocable_v<Func>>> {
+    using type = std::invoke_result_t<Func>;
+};
+template <typename Func>
+struct FriendlyInvokeResultImpl<Func, const void, std::enable_if_t<std::is_invocable_v<Func>>> {
+    using type = std::invoke_result_t<Func>;
+};
+
+template <typename Func, typename Arg>
+using FriendlyInvokeResult = typename FriendlyInvokeResultImpl<Func, Arg>::type;
+
+// Like is_invocable_v<Func, Args>, but handles Args == void correctly.
+template <typename Func, typename Arg>
+inline constexpr bool isCallable =
+    !std::is_same_v<FriendlyInvokeResult<Func, Arg>, InvalidCallSentinal>;
+
+// Like is_invocable_r_v<Func, Args>, but handles Args == void correctly and unwraps the return.
+template <typename Ret, typename Func, typename Arg>
+inline constexpr bool isCallableR =
+    (isCallable<Func, Arg> && std::is_same_v<UnwrappedType<FriendlyInvokeResult<Func, Arg>>, Ret>);
+
+// Like isCallableR, but doesn't unwrap the result type.
+template <typename Ret, typename Func, typename Arg>
+inline constexpr bool isCallableExactR = (isCallable<Func, Arg> &&
+                                          std::is_same_v<FriendlyInvokeResult<Func, Arg>, Ret>);
+
 /**
  * call() normalizes arguments to hide the FakeVoid shenanigans from users of Futures.
  * In the future it may also expand tuples to argument lists.