From 3ae5352bf06a95ca9bf2b46ac4c53a556b5948f7 Mon Sep 17 00:00:00 2001
From: Eric Niebler <eniebler@nvidia.com>
Date: Sun, 19 May 2024 21:07:32 -0700
Subject: [PATCH] fix completion signatures of `exec::finally`, fixes #1334

---
 include/exec/finally.hpp                   | 42 +++++++++++-----------
 include/stdexec/__detail/__diagnostics.hpp |  2 ++
 test/exec/test_finally.cpp                 | 16 +++++++++
 3 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/include/exec/finally.hpp b/include/exec/finally.hpp
index fb24d7944..1427c95db 100644
--- a/include/exec/finally.hpp
+++ b/include/exec/finally.hpp
@@ -25,9 +25,6 @@ namespace exec {
   namespace __final {
     using namespace stdexec;
 
-    template <class _Arg, class... _Args>
-    using __nonempty_decayed_tuple = __decayed_tuple<_Arg, _Args...>;
-
     template <class _Sigs>
     using __value_types_ =
       __gather_signal<set_value_t, _Sigs, __mbind_front_q<__decayed_tuple, set_value_t>, __q<__types>>;
@@ -62,11 +59,15 @@ namespace exec {
     using __as_rvalues = completion_signatures<set_value_t(__decay_t<_Args>...)>;
 
     template <class _InitialSender, class _FinalSender, class _Env>
-    using __completion_signatures_t = make_completion_signatures<
+    using __completion_signatures_t = __try_make_completion_signatures<
       _InitialSender,
       _Env,
-      completion_signatures<set_error_t(std::exception_ptr)>,
-      __as_rvalues>;
+      __try_make_completion_signatures<
+        _FinalSender,
+        _Env,
+        completion_signatures<set_error_t(std::exception_ptr)>,
+        __mconst<completion_signatures<>>>, // swallow the final sender's value completions
+      __q<__as_rvalues>>;
 
     template <class _Receiver>
     struct __applier {
@@ -148,7 +149,7 @@ namespace exec {
     template <class _InitialSenderId, class _FinalSenderId, class _ReceiverId>
     struct __operation_state {
       using _InitialSender = __cvref_t<_InitialSenderId>;
-      using _FinalSender = __cvref_t<_FinalSenderId>;
+      using _FinalSender = stdexec::__t<_FinalSenderId>;
       using _Receiver = stdexec::__t<_ReceiverId>;
       using __signatures = completion_signatures_of_t<_InitialSender, env_of_t<_Receiver>>;
       using __base_t = __final_operation_base<__result_variant<__signatures>, _ReceiverId>;
@@ -161,7 +162,7 @@ namespace exec {
     template <class _InitialSenderId, class _FinalSenderId, class _ReceiverId>
     struct __initial_receiver {
       using _Receiver = __cvref_t<_ReceiverId>;
-      using _FinalSender = __cvref_t<_FinalSenderId>;
+      using _FinalSender = stdexec::__t<_FinalSenderId>;
 
       using __base_op_t =
         stdexec::__t<__operation_state<_InitialSenderId, _FinalSenderId, _ReceiverId>>;
@@ -230,19 +231,19 @@ namespace exec {
       using __id = __operation_state;
 
       template <class... _Args>
-        requires std::is_constructible_v<__result_variant<__signatures>, __decayed_tuple<_Args...>>
+        requires constructible_from<__result_variant<__signatures>, __decayed_tuple<_Args...>>
       void __store_result_and_start_next_op(_Args&&... __args) {
         this->__result_.__construct(
           std::in_place_type<__decayed_tuple<_Args...>>, static_cast<_Args&&>(__args)...);
         STDEXEC_ASSERT(__op_.index() == 0);
-        _FinalSender __final = static_cast<_FinalSender&&>(std::get_if<0>(&__op_)->__sndr_);
+        _FinalSender& __final = std::get_if<0>(&__op_)->__sndr_;
         __final_op_t& __final_op = __op_.template emplace<1>(__conv{[&] {
           return stdexec::connect(static_cast<_FinalSender&&>(__final), __final_receiver_t{this});
         }});
         stdexec::start(__final_op);
       }
 
-      __t(_InitialSender&& __initial, _FinalSender&& __final, _Receiver __receiver)
+      __t(_InitialSender&& __initial, _FinalSender __final, _Receiver __receiver)
         : __base_t{{static_cast<_Receiver&&>(__receiver)}}
         , __op_(std::in_place_index<0>, __conv{[&] {
                   return __initial_op_t{
@@ -264,10 +265,8 @@ namespace exec {
       using _FinalSender = stdexec::__t<_FinalSenderId>;
 
       template <class _Self, class _Receiver>
-      using __op_t = stdexec::__t<__operation_state<
-        __cvref_id<_Self, _InitialSender>,
-        __cvref_id<_Self, _FinalSender>,
-        __id<_Receiver>>>;
+      using __op_t = stdexec::__t<
+        __operation_state<__cvref_id<_Self, _InitialSender>, __id<_FinalSender>, __id<_Receiver>>>;
 
       class __t {
         _InitialSender __initial_sndr_;
@@ -293,13 +292,16 @@ namespace exec {
         }
 
         template <__decays_to<__t> _Self, class _Env>
-        static auto
-          get_completion_signatures(_Self&&, _Env&&) noexcept -> __completion_signatures_t<
-            __copy_cvref_t<_Self, _InitialSender>,
-            __copy_cvref_t<_Self, _FinalSender>,
-            _Env> {
+        static auto get_completion_signatures(_Self&&, _Env&&) noexcept
+          -> __completion_signatures_t<__copy_cvref_t<_Self, _InitialSender>, _FinalSender, _Env> {
           return {};
         }
+
+        template <__decays_to<__t> _Self, class _Env>
+          requires(!__decay_copyable<__copy_cvref_t<_Self, _FinalSender>>)
+        static auto get_completion_signatures(_Self&&, _Env&&) noexcept {
+          return _ERROR_<_SENDER_TYPE_IS_NOT_COPYABLE_, _WITH_SENDER_<_FinalSender>>{};
+        }
       };
     };
 
diff --git a/include/stdexec/__detail/__diagnostics.hpp b/include/stdexec/__detail/__diagnostics.hpp
index 0fb2860f6..d0718a34f 100644
--- a/include/stdexec/__detail/__diagnostics.hpp
+++ b/include/stdexec/__detail/__diagnostics.hpp
@@ -75,6 +75,8 @@ namespace stdexec {
   template <class _Tag>
   struct _WITH_QUERY_;
 
+  struct _SENDER_TYPE_IS_NOT_COPYABLE_;
+
   inline constexpr __mstring __not_callable_diag =
     "The specified function is not callable with the arguments provided."_mstr;
 
diff --git a/test/exec/test_finally.cpp b/test/exec/test_finally.cpp
index 7b2e68ba9..9a7c8628b 100644
--- a/test/exec/test_finally.cpp
+++ b/test/exec/test_finally.cpp
@@ -69,4 +69,20 @@ namespace {
     CHECK_THROWS_AS(sync_wait(s), int);
     CHECK(called);
   }
+
+  TEST_CASE("finally includes the error types of the final action", "[adaptors][finally]") {
+    auto s = exec::finally(just(), just_error(42));
+    STATIC_REQUIRE(
+      set_equivalent<
+        completion_signatures_of_t<decltype(s), empty_env>,
+        completion_signatures<set_value_t(), set_error_t(std::exception_ptr), set_error_t(int)>>);
+  }
+
+  TEST_CASE("finally includes the stopped signal of the final action", "[adaptors][finally]") {
+    auto s = exec::finally(just(), just_stopped());
+    STATIC_REQUIRE(
+      set_equivalent<
+        completion_signatures_of_t<decltype(s), empty_env>,
+        completion_signatures<set_value_t(), set_error_t(std::exception_ptr), set_stopped_t()>>);
+  }
 } // namespace