From 9feb96948070162c977587450bf887a03d19144b Mon Sep 17 00:00:00 2001
From: Chris Cotter <ccotter14@bloomberg.net>
Date: Sat, 24 Jun 2023 00:22:48 -0400
Subject: [PATCH 1/7] Update finally to support value channel references

This aligns with the general idea that senders should be able to send
references and values, so the adaptor algorithms should preserve the
types in value channels when there is no specific requirement to modify
the value channel type.

In making this change, the CreateTest.AwaitTest test failed to compile.
This appears to be the create operation accepting universal reference
`Ts&&...` arguments which are then forwarded to the underlying receiver's
`set_value`. However, in this test, the type of `result` is `int&`,
but the create sender only sends `int`. Before this change (and before
the schedule affine task), something else ended up decaying the
parameter's type further downstream. With the scheduler affine task
and its reliance on `finally`, the `finally` receiver's `set_value`
strictly enforces that it's only called with the expected type, which
caused the compilation error with my changes. Fundamentally, the create
sender was incorrectly sending `int&` despite advertising it only
sends `int`. I updated the create algorithm to faithfully send exactly
what it advertises to fix the problem.

I left finally's existing behavior of decay_t-ing error types since I
ran into other issues trying to get that to work, and it seems less
useful/likely (though, I don't see why it shouldn't be done later).

Note that per discussion in #541, this is a source and behavior
breaking change.
---
 include/unifex/create.hpp  |  18 +++---
 include/unifex/finally.hpp |  11 ++--
 test/create_test.cpp       |  51 ++++++++++++----
 test/finally_test.cpp      | 116 +++++++++++++++++++++++++++++++++++--
 4 files changed, 167 insertions(+), 29 deletions(-)

diff --git a/include/unifex/create.hpp b/include/unifex/create.hpp
index 86113713b..a655c0c5d 100644
--- a/include/unifex/create.hpp
+++ b/include/unifex/create.hpp
@@ -24,7 +24,7 @@
 namespace unifex {
 namespace _create {
 
-template <typename Receiver, typename Fn, typename Context>
+template <typename Receiver, typename Fn, typename Context, typename... ValueTypes>
 struct _op {
   struct type {
     explicit type(Receiver rec, Fn fn, Context ctx)
@@ -36,7 +36,7 @@ struct _op {
     template (typename... Ts)
       (requires receiver_of<Receiver, Ts...>)
     void set_value(Ts&&... ts) noexcept(is_nothrow_receiver_of_v<Receiver, Ts...>) {
-      unifex::set_value((Receiver&&) rec_, (Ts&&) ts...);
+      unifex::set_value((Receiver&&) rec_, (ValueTypes) ts...);
     }
 
     template (typename Error)
@@ -79,10 +79,10 @@ struct _op {
   };
 };
 
-template <typename Receiver, typename Fn, typename Context>
-using _operation = typename _op<Receiver, Fn, Context>::type;
+template <typename Receiver, typename Fn, typename Context, typename... ValueTypes>
+using _operation = typename _op<Receiver, Fn, Context, ValueTypes...>::type;
 
-template <typename Fn, typename Context>
+template <typename Fn, typename Context, typename... ValueTypes>
 struct _snd_base {
   struct type {
     template <template<typename...> class Variant>
@@ -101,14 +101,14 @@ struct _snd_base {
       (requires derived_from<remove_cvref_t<Self>, type> AND
         constructible_from<Fn, member_t<Self, Fn>> AND
         constructible_from<Context, member_t<Self, Context>>)
-    friend _operation<remove_cvref_t<Receiver>, Fn, Context>
+    friend _operation<remove_cvref_t<Receiver>, Fn, Context, ValueTypes...>
     tag_invoke(tag_t<connect>, Self&& self, Receiver&& rec)
         noexcept(std::is_nothrow_constructible_v<
-          _operation<Receiver, Fn, Context>,
+          _operation<Receiver, Fn, Context, ValueTypes...>,
           Receiver,
           member_t<Self, Fn>,
           member_t<Self, Context>>) {
-      return _operation<remove_cvref_t<Receiver>, Fn, Context>{
+      return _operation<remove_cvref_t<Receiver>, Fn, Context, ValueTypes...>{
         (Receiver&&) rec,
         ((Self&&) self).fn_,
         ((Self&&) self).ctx_};
@@ -121,7 +121,7 @@ struct _snd_base {
 
 template <typename Fn, typename Context, typename... ValueTypes>
 struct _snd {
-  struct type : _snd_base<Fn, Context>::type {
+  struct type : _snd_base<Fn, Context, ValueTypes...>::type {
     template <template<typename...> class Variant, template <typename...> class Tuple>
     using value_types = Variant<Tuple<ValueTypes...>>;
   };
diff --git a/include/unifex/finally.hpp b/include/unifex/finally.hpp
index c3c012cf5..fee455a80 100644
--- a/include/unifex/finally.hpp
+++ b/include/unifex/finally.hpp
@@ -71,7 +71,7 @@ namespace unifex
         SourceSender,
         CompletionSender,
         Receiver,
-        std::decay_t<Values>...>::type;
+        Values...>::type;
 
     template <
         typename SourceSender,
@@ -384,7 +384,7 @@ namespace unifex
         auto* const op = op_;
 
         UNIFEX_TRY {
-          unifex::activate_union_member<std::tuple<std::decay_t<Values>...>>(
+          unifex::activate_union_member<std::tuple<Values...>>(
             op->value_, static_cast<Values&&>(values)...);
         } UNIFEX_CATCH (...) {
           std::move(*this).set_error(std::current_exception());
@@ -411,8 +411,7 @@ namespace unifex
                   });
           unifex::start(completionOp);
         } UNIFEX_CATCH (...) {
-          using decayed_tuple_t = std::tuple<std::decay_t<Values>...>;
-          unifex::deactivate_union_member<decayed_tuple_t>(op->value_);
+          unifex::deactivate_union_member<std::tuple<Values...>>(op->value_);
           unifex::set_error(
               static_cast<Receiver&&>(op->receiver_), std::current_exception());
         }
@@ -593,7 +592,7 @@ namespace unifex
         sender_value_types_t<
             remove_cvref_t<SourceSender>,
             manual_lifetime_union,
-            decayed_tuple<std::tuple>::template apply>
+            std::tuple>
                 value_;
       };
 
@@ -647,7 +646,7 @@ namespace unifex
           template <typename...> class Variant,
           template <typename...> class Tuple>
       using value_types = typename sender_traits<SourceSender>::
-          template value_types<Variant, decayed_tuple<Tuple>::template apply>;
+          template value_types<Variant, Tuple>;
 
       // This can produce any of the error_types of SourceSender, or of
       // CompletionSender or an exception_ptr corresponding to an exception thrown
diff --git a/test/create_test.cpp b/test/create_test.cpp
index b1ae2d259..650d721c1 100644
--- a/test/create_test.cpp
+++ b/test/create_test.cpp
@@ -30,6 +30,9 @@
 using namespace unifex;
 
 namespace {
+
+int global;
+
 struct CreateTest : testing::Test {
   unifex::single_thread_context someThread;
   unifex::async_scope someScope;
@@ -47,6 +50,14 @@ struct CreateTest : testing::Test {
     });
   }
 
+  void anIntRefAPI(void* context, void (*completed)(void* context, int& result)) {
+    // Execute some work asynchronously on some other thread. When its
+    // work is finished, pass the result to the callback.
+    someScope.detached_spawn_call_on(someThread.get_scheduler(), [=]() noexcept {
+      completed(context, global);
+    });
+  }
+
   void aVoidAPI(void* context, void (*completed)(void* context)) {
     // Execute some work asynchronously on some other thread. When its
     // work is finished, pass the result to the callback.
@@ -58,18 +69,38 @@ struct CreateTest : testing::Test {
 } // anonymous namespace
 
 TEST_F(CreateTest, BasicTest) {
-  auto snd = [this](int a, int b) {
-    return create<int>([a, b, this](auto& rec) {
-      static_assert(receiver_of<decltype(rec), int>);
-      anIntAPI(a, b, &rec, [](void* context, int result) {
-        unifex::void_cast<decltype(rec)>(context).set_value(result);
+  {
+    auto snd = [this](int a, int b) {
+      return create<int>([a, b, this](auto& rec) {
+        static_assert(receiver_of<decltype(rec), int>);
+        anIntAPI(a, b, &rec, [](void* context, int result) {
+          unifex::void_cast<decltype(rec)>(context).set_value(result);
+        });
       });
-    });
-  }(1, 2);
+    }(1, 2);
 
-  std::optional<int> res = sync_wait(std::move(snd));
-  ASSERT_TRUE(res.has_value());
-  EXPECT_EQ(*res, 3);
+    std::optional<int> res = sync_wait(std::move(snd));
+    ASSERT_TRUE(res.has_value());
+    EXPECT_EQ(*res, 3);
+  }
+
+  {
+    auto snd = [this]() {
+      return create<int&>([this](auto& rec) {
+        static_assert(receiver_of<decltype(rec), int&>);
+        anIntRefAPI(&rec, [](void* context, int& result) {
+          unifex::void_cast<decltype(rec)>(context).set_value(result);
+        });
+      });
+    }();
+
+    std::optional<std::reference_wrapper<int>> res = sync_wait(std::move(snd));
+    ASSERT_TRUE(res.has_value());
+    global = 0;
+    EXPECT_EQ(*res, 0);
+    global = 10;
+    EXPECT_EQ(*res, 10);
+  }
 }
 
 TEST_F(CreateTest, VoidWithContextTest) {
diff --git a/test/finally_test.cpp b/test/finally_test.cpp
index 338836b8d..b29d1ce1f 100644
--- a/test/finally_test.cpp
+++ b/test/finally_test.cpp
@@ -14,19 +14,24 @@
  * limitations under the License.
  */
 #include <unifex/finally.hpp>
+
 #include <unifex/inplace_stop_token.hpp>
-#include <unifex/sync_wait.hpp>
-#include <unifex/timed_single_thread_context.hpp>
-#include <unifex/scheduler_concepts.hpp>
-#include <unifex/then.hpp>
 #include <unifex/just.hpp>
 #include <unifex/just_done.hpp>
 #include <unifex/just_error.hpp>
+#include <unifex/just_from.hpp>
 #include <unifex/let_done.hpp>
 #include <unifex/let_error.hpp>
+#include <unifex/scheduler_concepts.hpp>
+#include <unifex/sync_wait.hpp>
+#include <unifex/then.hpp>
+#include <unifex/timed_single_thread_context.hpp>
+#include <unifex/upon_error.hpp>
 
 #include <cstdio>
 #include <thread>
+#include <tuple>
+#include <variant>
 
 #include <gtest/gtest.h>
 
@@ -45,6 +50,109 @@ TEST(Finally, Value) {
   EXPECT_EQ(res->second, context.get_thread_id());
 }
 
+TEST(Finally, Ref) {
+  {
+    int a = 0;
+
+    auto sndr = just_from([&a]() -> int& { return a; })
+      | finally(just());
+    using Sndr = decltype(sndr);
+
+    static_assert(std::is_same_v<
+      Sndr::value_types<std::variant, std::tuple>,
+      std::variant<std::tuple<int&>>
+    >);
+    static_assert(std::is_same_v<
+      Sndr::error_types<std::variant>,
+      std::variant<std::exception_ptr>
+    >);
+    static_assert(!Sndr::sends_done);
+
+    auto res = std::move(sndr) | sync_wait();
+
+    ASSERT_FALSE(!res);
+    EXPECT_EQ(*res, 0);
+    a = 10;
+    EXPECT_EQ(*res, 10);
+  }
+
+  {
+    int a = 0;
+
+    auto res = just_from([&a]() -> const int& { return a; })
+      | finally(just())
+      | sync_wait();
+
+    ASSERT_FALSE(!res);
+    EXPECT_EQ(*res, 0);
+    a = 10;
+    EXPECT_EQ(*res, 10);
+  }
+
+  {
+    int a = 0;
+
+    auto res = just_from([&a]() -> int& { return a; })
+      | finally(just())
+      | then([](int& i) -> int& { return i; })
+      | sync_wait();
+
+    ASSERT_FALSE(!res);
+    EXPECT_EQ(*res, 0);
+    a = 10;
+    EXPECT_EQ(*res, 10);
+  }
+}
+
+struct sends_error_ref {
+
+  template <
+    template <typename...> class Variant,
+    template <typename...> class Tuple>
+  using value_types = Variant<Tuple<>>;
+
+  template <template <typename...> class Variant>
+  using error_types = Variant<int>;
+
+  static constexpr bool sends_done = false;
+
+  template <class Receiver>
+  struct operation {
+    friend auto tag_invoke(tag_t<start>, operation& self) noexcept {
+      set_error(std::move(self.receiver), self.val);
+    }
+
+    int& val;
+    Receiver receiver;
+  };
+
+  template <class Receiver>
+  friend auto tag_invoke(tag_t<connect>, sends_error_ref self, Receiver&& receiver) {
+    return operation<Receiver>{self.val, std::forward<Receiver>(receiver)};
+  }
+
+  int& val;
+};
+
+
+TEST(Finally, ErrorRefDecays) {
+  // TODO: Should errors also have references preserved in 'finally?' See the
+  // 'finally' discussion in issue #541.
+  int a = 0;
+  auto sndr = sends_error_ref{a} | finally(just());
+  using Sndr = decltype(sndr);
+
+  static_assert(std::is_same_v<
+    Sndr::value_types<std::variant, std::tuple>,
+    std::variant<std::tuple<>>
+  >);
+  static_assert(std::is_same_v<
+    Sndr::error_types<std::variant>,
+    std::variant<int, std::exception_ptr>
+  >);
+  static_assert(!Sndr::sends_done);
+}
+
 TEST(Finally, Done) {
   timed_single_thread_context context;
 

From 472130ca38f383c2c9e611e0aab333143ee57399 Mon Sep 17 00:00:00 2001
From: Chris Cotter <ccotter14@bloomberg.net>
Date: Sun, 2 Jul 2023 23:52:35 -0400
Subject: [PATCH 2/7] Address some feedback

 - Fix conversion and add tests for create setting values, lvalue
   references and rvalue references.
 - Update the create receiver set_value function to be noexcept.
---
 include/unifex/create.hpp |  34 +++++++-
 test/create_test.cpp      | 172 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 202 insertions(+), 4 deletions(-)

diff --git a/include/unifex/create.hpp b/include/unifex/create.hpp
index a655c0c5d..75e83ac46 100644
--- a/include/unifex/create.hpp
+++ b/include/unifex/create.hpp
@@ -24,6 +24,25 @@
 namespace unifex {
 namespace _create {
 
+template <typename From, typename To, typename = void>
+struct _do_convert {
+  To operator()(From&&) = delete;
+};
+
+template <typename From>
+struct _do_convert<From, From, void> {
+  From&& operator()(From&& obj) noexcept {
+    return static_cast<From&&>(obj);
+  }
+};
+
+template <typename From, typename To>
+struct _do_convert<From, To, std::void_t<std::enable_if_t<convertible_to<From, To>>>> {
+  To operator()(From&& obj) {
+    return static_cast<From&&>(obj);
+  }
+};
+
 template <typename Receiver, typename Fn, typename Context, typename... ValueTypes>
 struct _op {
   struct type {
@@ -34,9 +53,18 @@ struct _op {
       : rec_((Receiver&&) rec), fn_((Fn&&) fn), ctx_((Context&&) ctx) {}
 
     template (typename... Ts)
-      (requires receiver_of<Receiver, Ts...>)
-    void set_value(Ts&&... ts) noexcept(is_nothrow_receiver_of_v<Receiver, Ts...>) {
-      unifex::set_value((Receiver&&) rec_, (ValueTypes) ts...);
+      (requires receiver_of<Receiver, Ts...> AND (convertible_to<Ts, ValueTypes> && ...))
+    void set_value(Ts&&... ts) noexcept {
+      UNIFEX_TRY {
+        // Satisfy the value completion contract with _do_convert, which
+        // ensures the value passed to the receiver conforms to
+        // the value types of the create sender. E.g., if set_value
+        // is called with an lvalue reference but create sends non-reference
+        // values, _do_convert converts the provided Ts.
+        unifex::set_value((Receiver&&) rec_, _do_convert<Ts, ValueTypes>{}(static_cast<Ts&&>(ts))...);
+      } UNIFEX_CATCH(...) {
+        unifex::set_error(std::move(rec_), std::current_exception());
+      }
     }
 
     template (typename Error)
diff --git a/test/create_test.cpp b/test/create_test.cpp
index 650d721c1..087d7080f 100644
--- a/test/create_test.cpp
+++ b/test/create_test.cpp
@@ -15,8 +15,10 @@
  */
 
 #include <unifex/create.hpp>
-#include <unifex/single_thread_context.hpp>
 #include <unifex/async_scope.hpp>
+#include <unifex/finally.hpp>
+#include <unifex/just.hpp>
+#include <unifex/single_thread_context.hpp>
 #include <unifex/sync_wait.hpp>
 
 #include <optional>
@@ -73,6 +75,7 @@ TEST_F(CreateTest, BasicTest) {
     auto snd = [this](int a, int b) {
       return create<int>([a, b, this](auto& rec) {
         static_assert(receiver_of<decltype(rec), int>);
+        static_assert(!receiver_of<decltype(rec), int*>);
         anIntAPI(a, b, &rec, [](void* context, int result) {
           unifex::void_cast<decltype(rec)>(context).set_value(result);
         });
@@ -103,6 +106,173 @@ TEST_F(CreateTest, BasicTest) {
   }
 }
 
+TEST_F(CreateTest, FinallyCreate) {
+    auto snd = [this](int a, int b) {
+      return create<int>([a, b, this](auto& rec) {
+        static_assert(receiver_of<std::decay_t<decltype(rec)>, int>);
+        anIntAPI(a, b, &rec, [](void* context, int result) {
+          unifex::void_cast<decltype(rec)>(context).set_value(result);
+        });
+      });
+    }(1, 2) | finally(just());
+
+    std::optional<int> res = sync_wait(std::move(snd));
+    ASSERT_TRUE(res.has_value());
+    EXPECT_EQ(*res, 3);
+}
+
+TEST_F(CreateTest, DoubleCreateSetsIntValue) {
+  auto snd = [this](int a, int b) {
+    return create<double>([a, b, this](auto& rec) {
+      static_assert(receiver_of<std::decay_t<decltype(rec)>, int>);
+      anIntAPI(a, b, &rec, [](void* context, int result) {
+        unifex::void_cast<decltype(rec)>(context).set_value(result);
+      });
+    });
+  }(1, 2);
+
+  static_assert(std::is_same_v<decltype(sync_wait(std::move(snd))), std::optional<double>>);
+  std::optional<double> res = sync_wait(std::move(snd));
+  ASSERT_TRUE(res.has_value());
+  EXPECT_EQ(*res, 3);
+}
+
+struct TrackingObject {
+  static int moves;
+  static int copies;
+
+  explicit TrackingObject(int val) : val(val) {}
+  TrackingObject(const TrackingObject& other) : val(other.val) {
+    ++copies;
+  }
+  TrackingObject(TrackingObject&& other) : val(other.val) {
+    ++moves;
+    other.was_moved = true;
+  }
+  TrackingObject& operator=(const TrackingObject&) = delete;
+  TrackingObject& operator=(TrackingObject&&) = delete;
+
+  int val;
+  bool was_moved = false;
+};
+int TrackingObject::moves = 0;
+int TrackingObject::copies = 0;
+
+TEST_F(CreateTest, CreateObjectNotCopied) {
+  auto snd = [this](int a, int b) {
+    return create<TrackingObject>([a, b, this](auto& rec) {
+      static_assert(receiver_of<std::decay_t<decltype(rec)>, TrackingObject>);
+      anIntAPI(a, b, &rec, [](void* context, int result) {
+        unifex::void_cast<decltype(rec)>(context).set_value(TrackingObject{result});
+      });
+    });
+  }(1, 2);
+
+  TrackingObject::copies = 0;
+
+  std::optional<TrackingObject> res = sync_wait(std::move(snd));
+  ASSERT_TRUE(res.has_value());
+  EXPECT_EQ(res->val, 3);
+  EXPECT_EQ(TrackingObject::copies, 0);
+}
+
+TEST_F(CreateTest, CreateObjectCopied) {
+  auto snd = [this](int a, int b) {
+    return create<TrackingObject>([a, b, this](auto& rec) {
+      static_assert(receiver_of<std::decay_t<decltype(rec)>, TrackingObject>);
+      anIntAPI(a, b, &rec, [](void* context, int result) {
+        TrackingObject obj{result};
+        unifex::void_cast<decltype(rec)>(context).set_value(obj);
+      });
+    });
+  }(1, 2);
+
+  TrackingObject::copies = 0;
+
+  std::optional<TrackingObject> res = sync_wait(std::move(snd));
+  ASSERT_TRUE(res.has_value());
+  EXPECT_EQ(res->val, 3);
+  EXPECT_EQ(TrackingObject::copies, 1);
+}
+
+TEST_F(CreateTest, CreateWithConditionalMove) {
+  TrackingObject obj{0};
+
+  struct Data {
+    void* context;
+    TrackingObject* obj;
+  };
+  Data data{nullptr, &obj};
+
+  auto snd = [this, &data](int a, int b) {
+    return create<TrackingObject&&>([a, b, &data, this](auto& rec) {
+      static_assert(receiver_of<std::decay_t<decltype(rec)>, TrackingObject&&>);
+      data.context = &rec;
+      anIntAPI(a, b, &data, [](void* context, int result) {
+        Data& data = unifex::void_cast<Data&>(context);
+        data.obj->val = result;
+        unifex::void_cast<decltype(rec)>(data.context).set_value(std::move(*data.obj));
+      });
+    });
+  }(1, 2) | then([](TrackingObject&& obj) {
+      return obj.val;
+  });
+
+  TrackingObject::copies = 0;
+  TrackingObject::moves = 0;
+
+  std::optional<int> res = sync_wait(std::move(snd));
+  ASSERT_TRUE(res.has_value());
+  EXPECT_EQ(*res, 3);
+  EXPECT_EQ(TrackingObject::copies, 0);
+  EXPECT_EQ(TrackingObject::moves, 0);
+  EXPECT_FALSE(obj.was_moved);
+}
+
+TEST_F(CreateTest, CreateWithConversions) {
+  struct A {
+    int val;
+  };
+  struct B {
+    B(A a) : val(a.val) {}
+    B(int val) : val(val) {}
+    operator A() const {
+      return A{val};
+    }
+    int val;
+  };
+
+  {
+    auto snd = [this](int a, int b) {
+      return create<A>([a, b, this](auto& rec) {
+        static_assert(receiver_of<std::decay_t<decltype(rec)>, A>);
+        anIntAPI(a, b, &rec, [](void* context, int result) {
+          unifex::void_cast<decltype(rec)>(context).set_value(B{result});
+        });
+      });
+    }(1, 2);
+
+    std::optional<A> res = sync_wait(std::move(snd));
+    ASSERT_TRUE(res.has_value());
+    EXPECT_EQ(res->val, 3);
+  }
+
+  {
+    auto snd = [this](int a, int b) {
+      return create<B>([a, b, this](auto& rec) {
+        static_assert(receiver_of<std::decay_t<decltype(rec)>, int>);
+        anIntAPI(a, b, &rec, [](void* context, int result) {
+          unifex::void_cast<decltype(rec)>(context).set_value(A{result});
+        });
+      });
+    }(1, 2);
+
+    std::optional<B> res = sync_wait(std::move(snd));
+    ASSERT_TRUE(res.has_value());
+    EXPECT_EQ(res->val, 3);
+  }
+}
+
 TEST_F(CreateTest, VoidWithContextTest) {
   bool called = false;
   auto snd = [&called, this]() {

From 2bb4f836bd4c0fa1fe3499e1cf59c5dfe1cfee16 Mon Sep 17 00:00:00 2001
From: Chris Cotter <ccotter14@bloomberg.net>
Date: Mon, 3 Jul 2023 00:32:11 -0400
Subject: [PATCH 3/7] Use move()

---
 include/unifex/create.hpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/unifex/create.hpp b/include/unifex/create.hpp
index 75e83ac46..6d1f6e380 100644
--- a/include/unifex/create.hpp
+++ b/include/unifex/create.hpp
@@ -61,7 +61,7 @@ struct _op {
         // the value types of the create sender. E.g., if set_value
         // is called with an lvalue reference but create sends non-reference
         // values, _do_convert converts the provided Ts.
-        unifex::set_value((Receiver&&) rec_, _do_convert<Ts, ValueTypes>{}(static_cast<Ts&&>(ts))...);
+        unifex::set_value(std::move(rec_), _do_convert<Ts, ValueTypes>{}(static_cast<Ts&&>(ts))...);
       } UNIFEX_CATCH(...) {
         unifex::set_error(std::move(rec_), std::current_exception());
       }

From 6a722c684dfbc9f9785b42c00f80458cc94d07f7 Mon Sep 17 00:00:00 2001
From: Chris Cotter <ccotter14@bloomberg.net>
Date: Mon, 3 Jul 2023 17:42:39 -0400
Subject: [PATCH 4/7] Fix conversion to always create a new object for
 create<T>

---
 include/unifex/create.hpp |  6 +++---
 test/create_test.cpp      | 22 ++++++++++++++++++++++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/include/unifex/create.hpp b/include/unifex/create.hpp
index 6d1f6e380..ff5cb9124 100644
--- a/include/unifex/create.hpp
+++ b/include/unifex/create.hpp
@@ -58,10 +58,10 @@ struct _op {
       UNIFEX_TRY {
         // Satisfy the value completion contract with _do_convert, which
         // ensures the value passed to the receiver conforms to
-        // the value types of the create sender. E.g., if set_value
+        // the value types of the create sender. For example, if set_value
         // is called with an lvalue reference but create sends non-reference
-        // values, _do_convert converts the provided Ts.
-        unifex::set_value(std::move(rec_), _do_convert<Ts, ValueTypes>{}(static_cast<Ts&&>(ts))...);
+        // values, _do_convert decays the provided Ts.
+        unifex::set_value(std::move(rec_), _do_convert<Ts&&, ValueTypes>{}(static_cast<Ts&&>(ts))...);
       } UNIFEX_CATCH(...) {
         unifex::set_error(std::move(rec_), std::current_exception());
       }
diff --git a/test/create_test.cpp b/test/create_test.cpp
index 087d7080f..78e7779a5 100644
--- a/test/create_test.cpp
+++ b/test/create_test.cpp
@@ -195,6 +195,28 @@ TEST_F(CreateTest, CreateObjectCopied) {
   EXPECT_EQ(TrackingObject::copies, 1);
 }
 
+TEST_F(CreateTest, CreateObjectLeadsToNewObject) {
+  auto snd = [this](int a, int b) {
+    return create<TrackingObject>([a, b, this](auto& rec) {
+      static_assert(receiver_of<std::decay_t<decltype(rec)>, TrackingObject>);
+      anIntAPI(a, b, &rec, [](void* context, int result) {
+        unifex::void_cast<decltype(rec)>(context).set_value(TrackingObject{result});
+      });
+    });
+  }(1, 2) | then([](TrackingObject&& obj) {
+      return obj.val;
+  });
+
+  TrackingObject::copies = 0;
+  TrackingObject::moves = 0;
+
+  std::optional<int> res = sync_wait(std::move(snd));
+  ASSERT_TRUE(res.has_value());
+  EXPECT_EQ(*res, 3);
+  EXPECT_EQ(TrackingObject::copies, 0);
+  EXPECT_GT(TrackingObject::moves, 0);
+}
+
 TEST_F(CreateTest, CreateWithConditionalMove) {
   TrackingObject obj{0};
 

From e2453ccc7ce843a49ec57b32c667b006f7d68d07 Mon Sep 17 00:00:00 2001
From: Chris Cotter <ccotter14@bloomberg.net>
Date: Mon, 3 Jul 2023 22:56:40 -0400
Subject: [PATCH 5/7] Apply suggestions from code review

Co-authored-by: Ian Petersen <ispeters@gmail.com>
---
 include/unifex/create.hpp |  5 +++--
 test/create_test.cpp      |  7 ++-----
 test/finally_test.cpp     | 12 ++++++------
 3 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/include/unifex/create.hpp b/include/unifex/create.hpp
index ff5cb9124..4c30ae00b 100644
--- a/include/unifex/create.hpp
+++ b/include/unifex/create.hpp
@@ -53,7 +53,7 @@ struct _op {
       : rec_((Receiver&&) rec), fn_((Fn&&) fn), ctx_((Context&&) ctx) {}
 
     template (typename... Ts)
-      (requires receiver_of<Receiver, Ts...> AND (convertible_to<Ts, ValueTypes> && ...))
+      (requires (convertible_to<Ts, ValueTypes> && ...))
     void set_value(Ts&&... ts) noexcept {
       UNIFEX_TRY {
         // Satisfy the value completion contract with _do_convert, which
@@ -128,7 +128,8 @@ struct _snd_base {
     template (typename Self, typename Receiver)
       (requires derived_from<remove_cvref_t<Self>, type> AND
         constructible_from<Fn, member_t<Self, Fn>> AND
-        constructible_from<Context, member_t<Self, Context>>)
+        constructible_from<Context, member_t<Self, Context>> AND
+        receiver_of<Receiver, ValueTypes...>)
     friend _operation<remove_cvref_t<Receiver>, Fn, Context, ValueTypes...>
     tag_invoke(tag_t<connect>, Self&& self, Receiver&& rec)
         noexcept(std::is_nothrow_constructible_v<
diff --git a/test/create_test.cpp b/test/create_test.cpp
index 78e7779a5..1f815f1ff 100644
--- a/test/create_test.cpp
+++ b/test/create_test.cpp
@@ -99,10 +99,7 @@ TEST_F(CreateTest, BasicTest) {
 
     std::optional<std::reference_wrapper<int>> res = sync_wait(std::move(snd));
     ASSERT_TRUE(res.has_value());
-    global = 0;
-    EXPECT_EQ(*res, 0);
-    global = 10;
-    EXPECT_EQ(*res, 10);
+    EXPECT_EQ(&res->get(), &global);
   }
 }
 
@@ -214,7 +211,7 @@ TEST_F(CreateTest, CreateObjectLeadsToNewObject) {
   ASSERT_TRUE(res.has_value());
   EXPECT_EQ(*res, 3);
   EXPECT_EQ(TrackingObject::copies, 0);
-  EXPECT_GT(TrackingObject::moves, 0);
+  EXPECT_GE(TrackingObject::moves, 1);
 }
 
 TEST_F(CreateTest, CreateWithConditionalMove) {
diff --git a/test/finally_test.cpp b/test/finally_test.cpp
index b29d1ce1f..3bdc42fca 100644
--- a/test/finally_test.cpp
+++ b/test/finally_test.cpp
@@ -59,14 +59,14 @@ TEST(Finally, Ref) {
     using Sndr = decltype(sndr);
 
     static_assert(std::is_same_v<
-      Sndr::value_types<std::variant, std::tuple>,
+     sender_value_types_t<Sndr, std::variant, std::tuple>,
       std::variant<std::tuple<int&>>
     >);
     static_assert(std::is_same_v<
-      Sndr::error_types<std::variant>,
+      sender_error_types_t<Sndr, std::variant>,
       std::variant<std::exception_ptr>
     >);
-    static_assert(!Sndr::sends_done);
+    static_assert(!sender_traits<Sndr>::sends_done);
 
     auto res = std::move(sndr) | sync_wait();
 
@@ -143,14 +143,14 @@ TEST(Finally, ErrorRefDecays) {
   using Sndr = decltype(sndr);
 
   static_assert(std::is_same_v<
-    Sndr::value_types<std::variant, std::tuple>,
+    sender_value_types_t<Sndr, std::variant, std::tuple>,
     std::variant<std::tuple<>>
   >);
   static_assert(std::is_same_v<
-    Sndr::error_types<std::variant>,
+    sender_error_types_t<Sndr, std::variant>,
     std::variant<int, std::exception_ptr>
   >);
-  static_assert(!Sndr::sends_done);
+  static_assert(!sender_traits<Sndr>::sends_done);
 }
 
 TEST(Finally, Done) {

From ab1811d7ed9b2b7ae7a53019d6ece9491f90db48 Mon Sep 17 00:00:00 2001
From: Chris Cotter <ccotter14@bloomberg.net>
Date: Mon, 3 Jul 2023 22:42:36 -0400
Subject: [PATCH 6/7] Use explicit cast from forwarded Ts... to ValueTypes

---
 include/unifex/create.hpp | 30 +++++-------------------------
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/include/unifex/create.hpp b/include/unifex/create.hpp
index 4c30ae00b..b1bda16bc 100644
--- a/include/unifex/create.hpp
+++ b/include/unifex/create.hpp
@@ -24,25 +24,6 @@
 namespace unifex {
 namespace _create {
 
-template <typename From, typename To, typename = void>
-struct _do_convert {
-  To operator()(From&&) = delete;
-};
-
-template <typename From>
-struct _do_convert<From, From, void> {
-  From&& operator()(From&& obj) noexcept {
-    return static_cast<From&&>(obj);
-  }
-};
-
-template <typename From, typename To>
-struct _do_convert<From, To, std::void_t<std::enable_if_t<convertible_to<From, To>>>> {
-  To operator()(From&& obj) {
-    return static_cast<From&&>(obj);
-  }
-};
-
 template <typename Receiver, typename Fn, typename Context, typename... ValueTypes>
 struct _op {
   struct type {
@@ -56,12 +37,11 @@ struct _op {
       (requires (convertible_to<Ts, ValueTypes> && ...))
     void set_value(Ts&&... ts) noexcept {
       UNIFEX_TRY {
-        // Satisfy the value completion contract with _do_convert, which
-        // ensures the value passed to the receiver conforms to
-        // the value types of the create sender. For example, if set_value
-        // is called with an lvalue reference but create sends non-reference
-        // values, _do_convert decays the provided Ts.
-        unifex::set_value(std::move(rec_), _do_convert<Ts&&, ValueTypes>{}(static_cast<Ts&&>(ts))...);
+        // Satisfy the value completion contract by converting to the
+        // Sender's value_types. For example, if set_value is called with
+        // an lvalue reference but the create Sender sends non-reference
+        // values.
+        unifex::set_value(std::move(rec_), static_cast<ValueTypes>(static_cast<Ts&&>(ts))...);
       } UNIFEX_CATCH(...) {
         unifex::set_error(std::move(rec_), std::current_exception());
       }

From 913eaf62ef06ce28b3acffb072a1974cfadc2ecb Mon Sep 17 00:00:00 2001
From: Chris Cotter <ccotter14@bloomberg.net>
Date: Mon, 3 Jul 2023 23:00:32 -0400
Subject: [PATCH 7/7] Compare pointers

---
 test/finally_test.cpp | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/test/finally_test.cpp b/test/finally_test.cpp
index 3bdc42fca..2977caac1 100644
--- a/test/finally_test.cpp
+++ b/test/finally_test.cpp
@@ -71,9 +71,7 @@ TEST(Finally, Ref) {
     auto res = std::move(sndr) | sync_wait();
 
     ASSERT_FALSE(!res);
-    EXPECT_EQ(*res, 0);
-    a = 10;
-    EXPECT_EQ(*res, 10);
+    EXPECT_EQ(&res->get(), &a);
   }
 
   {
@@ -84,9 +82,7 @@ TEST(Finally, Ref) {
       | sync_wait();
 
     ASSERT_FALSE(!res);
-    EXPECT_EQ(*res, 0);
-    a = 10;
-    EXPECT_EQ(*res, 10);
+    EXPECT_EQ(&res->get(), &a);
   }
 
   {
@@ -98,9 +94,7 @@ TEST(Finally, Ref) {
       | sync_wait();
 
     ASSERT_FALSE(!res);
-    EXPECT_EQ(*res, 0);
-    a = 10;
-    EXPECT_EQ(*res, 10);
+    EXPECT_EQ(&res->get(), &a);
   }
 }