make all non-forking rvalue awaitables movable

include/tmc/atomic_condvar.hpp

@@ -13,6 +13,7 @@
 #include "tmc/detail/waiter_list.hpp"
 
 #include <atomic>
+#include <cassert>
 #include <coroutine>
 #include <cstddef>
 #include <mutex>
@@ -24,22 +25,24 @@ class waiter_count_accessor;
 
 namespace tmc {
 template <typename T> class atomic_condvar;
+template <typename T> class aw_atomic_condvar;
 template <typename T> class aw_atomic_condvar_co_notify;
 
-/// The awaitable type returned by `atomic_condvar.await()`
+/// The awaiter type produced by co_awaiting aw_atomic_condvar. It is
+/// constructed in place in the awaiting coroutine's frame, where it lives
+/// across the suspension.
 template <typename T>
-class [[nodiscard(
-  "You must co_await aw_atomic_condvar for it to have any effect."
-)]] aw_atomic_condvar : tmc::detail::AwaitTagNoGroupAsIs {
+class aw_atomic_condvar_impl {
   T expected;
   atomic_condvar<T>& parent;
   tmc::detail::waiter_list_waiter waiter;
 
+  friend class aw_atomic_condvar<T>;
   friend class atomic_condvar<T>;
   friend class aw_atomic_condvar_co_notify<T>;
 
-  inline aw_atomic_condvar(
-    atomic_condvar<T>& Parent TMC_LIFETIMEBOUND, T Expected
+  inline aw_atomic_condvar_impl(
+    atomic_condvar<T>& Parent, T Expected
   ) noexcept
       : expected(Expected), parent(Parent) {}
 
@@ -70,9 +73,40 @@ class [[nodiscard(
   inline void await_resume() noexcept {}
 
   // Cannot be moved or copied due to holding intrusive list pointer
+  aw_atomic_condvar_impl(aw_atomic_condvar_impl const&) = delete;
+  aw_atomic_condvar_impl& operator=(aw_atomic_condvar_impl const&) = delete;
+  aw_atomic_condvar_impl(aw_atomic_condvar_impl&&) = delete;
+  aw_atomic_condvar_impl& operator=(aw_atomic_condvar_impl&&) = delete;
+};
+
+/// The awaitable type returned by `atomic_condvar.await()`
+template <typename T>
+class [[nodiscard(
+  "You must co_await aw_atomic_condvar for it to have any effect."
+)]] aw_atomic_condvar : tmc::detail::AwaitTagNoGroupCoAwait {
+  atomic_condvar<T>* parent;
+  T expected;
+
+  friend class atomic_condvar<T>;
+
+  inline aw_atomic_condvar(
+    atomic_condvar<T>& Parent TMC_LIFETIMEBOUND, T Expected
+  ) noexcept
+      : parent(&Parent), expected(Expected) {}
+
+public:
+  inline aw_atomic_condvar_impl<T> operator co_await() && noexcept {
+    assert(parent != nullptr && "aw_atomic_condvar may only be awaited once");
+    return aw_atomic_condvar_impl<T>(*parent, expected);
+  }
+
+  // Movable but not copyable
   aw_atomic_condvar(aw_atomic_condvar const&) = delete;
   aw_atomic_condvar& operator=(aw_atomic_condvar const&) = delete;
-  aw_atomic_condvar(aw_atomic_condvar&&) = delete;
+  inline aw_atomic_condvar(aw_atomic_condvar&& Other) noexcept
+      : parent(Other.parent), expected(Other.expected) {
+    Other.parent = nullptr;
+  }
   aw_atomic_condvar& operator=(aw_atomic_condvar&&) = delete;
 };
 
@@ -97,7 +131,7 @@ class [[nodiscard(
 
   inline std::coroutine_handle<>
   await_suspend(std::coroutine_handle<> Outer) noexcept {
-    std::vector<aw_atomic_condvar<T>*> wakeList =
+    std::vector<aw_atomic_condvar_impl<T>*> wakeList =
       parent.get_n_waiters(notify_count);
     size_t sz = wakeList.size();
     if (sz == 0) {
@@ -112,11 +146,12 @@ class [[nodiscard(
 
   inline void await_resume() noexcept {}
 
-  // Copy/move constructors *could* be implemented, but why?
+  // Movable so that it can be captured by value into a wrapper task when
+  // passed to spawn() / fork(), but not copyable.
   aw_atomic_condvar_co_notify(aw_atomic_condvar_co_notify const&) = delete;
   aw_atomic_condvar_co_notify&
   operator=(aw_atomic_condvar_co_notify const&) = delete;
-  aw_atomic_condvar_co_notify(atomic_condvar<T>&&) = delete;
+  aw_atomic_condvar_co_notify(aw_atomic_condvar_co_notify&&) = default;
   aw_atomic_condvar_co_notify&
   operator=(aw_atomic_condvar_co_notify&&) = delete;
 };
@@ -125,10 +160,10 @@ class [[nodiscard(
 /// `std::atomic<T>::wait()` and `std::atomic<T>::notify_*()`.
 template <typename T> class atomic_condvar {
   std::mutex waiters_lock;
-  std::vector<aw_atomic_condvar<T>*> waiters;
+  std::vector<aw_atomic_condvar_impl<T>*> waiters;
   std::atomic<T> value;
 
-  friend class aw_atomic_condvar<T>;
+  friend class aw_atomic_condvar_impl<T>;
   friend class aw_atomic_condvar_co_notify<T>;
   friend class ::tmc::tests::waiter_count_accessor;
 
@@ -141,8 +176,8 @@ template <typename T> class atomic_condvar {
 
   // Returns the most recently added waiter that matches the expected
   // condition.
-  inline aw_atomic_condvar<T>* get_one_waiter() {
-    aw_atomic_condvar<T>* toWake = nullptr;
+  inline aw_atomic_condvar_impl<T>* get_one_waiter() {
+    aw_atomic_condvar_impl<T>* toWake = nullptr;
     {
       std::scoped_lock<std::mutex> l{waiters_lock};
       auto v = value.load(std::memory_order_seq_cst);
@@ -161,8 +196,8 @@ template <typename T> class atomic_condvar {
 
   // Returns up to N waiters that match the expected condition. The most
   // recently added waiters are at the front of the returned list.
-  inline std::vector<aw_atomic_condvar<T>*> get_n_waiters(size_t N) {
-    std::vector<aw_atomic_condvar<T>*> wakeList;
+  inline std::vector<aw_atomic_condvar_impl<T>*> get_n_waiters(size_t N) {
+    std::vector<aw_atomic_condvar_impl<T>*> wakeList;
     {
       std::scoped_lock<std::mutex> l{waiters_lock};
       auto v = value.load(std::memory_order_seq_cst);
@@ -223,7 +258,7 @@ template <typename T> class atomic_condvar {
   /// Does not symmetric transfer; the awaiter will be posted to its executor.
   /// Awaiters are woken in LIFO order.
   inline void notify_one() {
-    aw_atomic_condvar<T>* toWake = get_one_waiter();
+    aw_atomic_condvar_impl<T>* toWake = get_one_waiter();
     if (toWake != nullptr) {
       toWake->waiter.resume();
     }
@@ -237,7 +272,7 @@ template <typename T> class atomic_condvar {
     if (NotifyCount == 0) {
       return;
     }
-    std::vector<aw_atomic_condvar<T>*> wakeList = get_n_waiters(NotifyCount);
+    std::vector<aw_atomic_condvar_impl<T>*> wakeList = get_n_waiters(NotifyCount);
     for (size_t i = 0; i < wakeList.size(); ++i) {
       wakeList[i]->waiter.resume();
     }
@@ -246,7 +281,7 @@ template <typename T> class atomic_condvar {
   /// Wakes all awaiters that meet the criteria (expected != current value).
   /// Does not symmetric transfer; awaiters will be posted to their executors.
   inline void notify_all() {
-    std::vector<aw_atomic_condvar<T>*> wakeList = get_n_waiters(TMC_ALL_ONES);
+    std::vector<aw_atomic_condvar_impl<T>*> wakeList = get_n_waiters(TMC_ALL_ONES);
     for (size_t i = 0; i < wakeList.size(); ++i) {
       wakeList[i]->waiter.resume();
     }
@@ -263,7 +298,7 @@ template <typename T> class atomic_condvar {
     std::scoped_lock<std::mutex> l{waiters_lock};
     // No need to unlock before resuming here - it's not valid for resumers to
     // access the destroyed mutex anyway.
-    std::vector<aw_atomic_condvar<T>*> wakeList;
+    std::vector<aw_atomic_condvar_impl<T>*> wakeList;
     for (size_t i = 0; i < waiters.size(); ++i) {
       waiters[i]->waiter.resume();
     }

include/tmc/auto_reset_event.hpp

@@ -38,11 +38,12 @@ class [[nodiscard(
 
   inline void await_resume() noexcept {}
 
-  // Copy/move constructors *could* be implemented, but why?
+  // Movable so that it can be captured by value into a wrapper task when
+  // passed to spawn() / fork(), but not copyable.
   aw_auto_reset_event_co_set(aw_auto_reset_event_co_set const&) = delete;
   aw_auto_reset_event_co_set&
   operator=(aw_auto_reset_event_co_set const&) = delete;
-  aw_auto_reset_event_co_set(aw_auto_reset_event_co_set&&) = delete;
+  aw_auto_reset_event_co_set(aw_auto_reset_event_co_set&&) = default;
   aw_auto_reset_event_co_set& operator=(aw_auto_reset_event_co_set&&) = delete;
 };
 

include/tmc/detail/mutex.ipp

@@ -22,13 +22,13 @@ mutex_scope::~mutex_scope() {
   }
 }
 
-bool aw_mutex_lock_scope::await_ready() noexcept {
+bool aw_mutex_lock_scope_impl::await_ready() noexcept {
   return tmc::detail::try_acquire(
     parent.load(std::memory_order_relaxed)->value
   );
 }
 
-void aw_mutex_lock_scope::await_suspend(
+void aw_mutex_lock_scope_impl::await_suspend(
   std::coroutine_handle<> Outer
 ) noexcept {
   // This may be resumed immediately after we call add_waiter(). Access to

include/tmc/detail/semaphore.ipp

@@ -21,13 +21,13 @@ semaphore_scope::~semaphore_scope() {
   }
 }
 
-bool aw_semaphore_acquire_scope::await_ready() noexcept {
+bool aw_semaphore_acquire_scope_impl::await_ready() noexcept {
   return tmc::detail::try_acquire(
     parent.load(std::memory_order_relaxed)->value
   );
 }
 
-void aw_semaphore_acquire_scope::await_suspend(
+void aw_semaphore_acquire_scope_impl::await_suspend(
   std::coroutine_handle<> Outer
 ) noexcept {
   // This may be resumed immediately after we call add_waiter(). Access to

include/tmc/detail/task_wrapper.hpp

@@ -119,7 +119,16 @@ template <typename Result> struct task_wrapper_promise {
     std::is_nothrow_move_constructible_v<RV> &&
     std::is_nothrow_move_assignable_v<RV>
   ) {
-    *customizer.result_ptr = static_cast<RV&&>(Value);
+    if constexpr (requires {
+                    *customizer.result_ptr = static_cast<RV&&>(Value);
+                  }) {
+      *customizer.result_ptr = static_cast<RV&&>(Value);
+    } else {
+      // Results that are move-constructible but not move-assignable (such as
+      // mutex_scope) are stored in a std::optional; construct the value in
+      // place instead.
+      customizer.result_ptr->emplace(static_cast<RV&&>(Value));
+    }
   }
 
 #ifdef TMC_DEBUG_TASK_ALLOC_COUNT

include/tmc/manual_reset_event.hpp

@@ -65,10 +65,11 @@ class [[nodiscard(
   /// Returns the number of awaiters that were woken.
   inline size_t await_resume() noexcept { return wakeCount; }
 
-  // Copy/move constructors *could* be implemented, but why?
+  // Movable so that it can be captured by value into a wrapper task when
+  // passed to spawn() / fork(), but not copyable.
   aw_manual_reset_event_co_set(aw_manual_reset_event_co_set const&) = delete;
   aw_manual_reset_event_co_set& operator=(aw_manual_reset_event_co_set const&) = delete;
-  aw_manual_reset_event_co_set(aw_manual_reset_event_co_set&&) = delete;
+  aw_manual_reset_event_co_set(aw_manual_reset_event_co_set&&) = default;
   aw_manual_reset_event_co_set& operator=(aw_manual_reset_event_co_set&&) = delete;
 };
 

include/tmc/mutex.hpp

@@ -27,7 +27,7 @@ class [[nodiscard("The mutex will be unlocked when this goes out of scope.")]]
 mutex_scope {
   mutex* parent;
 
-  friend class aw_mutex_lock_scope;
+  friend class aw_mutex_lock_scope_impl;
 
   inline mutex_scope(mutex* Parent TMC_LIFETIMEBOUND) noexcept : parent(Parent) {}
 
@@ -45,17 +45,16 @@ mutex_scope {
   TMC_DECL ~mutex_scope();
 };
 
-/// Same as aw_acquire but returns a nodiscard mutex_scope that unlocks the
-/// mutex on destruction.
-class [[nodiscard(
-  "You must co_await aw_mutex_lock_scope for it to have any effect."
-)]] aw_mutex_lock_scope : tmc::detail::AwaitTagNoGroupAsIs {
+/// The awaiter type produced by co_awaiting aw_mutex_lock_scope. It is
+/// constructed in place in the awaiting coroutine's frame, where it lives
+/// across the suspension.
+class aw_mutex_lock_scope_impl {
   tmc::detail::waiter_list_node me;
   std::atomic<mutex*> parent;
 
-  friend class mutex;
+  friend class aw_mutex_lock_scope;
 
-  inline aw_mutex_lock_scope(mutex& Parent) noexcept : parent(&Parent) {}
+  inline aw_mutex_lock_scope_impl(mutex& Parent) noexcept : parent(&Parent) {}
 
 public:
   TMC_DECL bool await_ready() noexcept;
@@ -67,9 +66,37 @@ class [[nodiscard(
   }
 
   // Cannot be moved or copied due to holding intrusive list pointer
+  aw_mutex_lock_scope_impl(aw_mutex_lock_scope_impl const&) = delete;
+  aw_mutex_lock_scope_impl& operator=(aw_mutex_lock_scope_impl const&) = delete;
+  aw_mutex_lock_scope_impl(aw_mutex_lock_scope_impl&&) = delete;
+  aw_mutex_lock_scope_impl& operator=(aw_mutex_lock_scope_impl&&) = delete;
+};
+
+/// Same as aw_acquire but returns a nodiscard mutex_scope that unlocks the
+/// mutex on destruction.
+class [[nodiscard(
+  "You must co_await aw_mutex_lock_scope for it to have any effect."
+)]] aw_mutex_lock_scope : tmc::detail::AwaitTagNoGroupCoAwait {
+  mutex* parent;
+
+  friend class mutex;
+
+  inline aw_mutex_lock_scope(mutex& Parent TMC_LIFETIMEBOUND) noexcept
+      : parent(&Parent) {}
+
+public:
+  inline aw_mutex_lock_scope_impl operator co_await() && noexcept {
+    assert(parent != nullptr && "aw_mutex_lock_scope may only be awaited once");
+    return aw_mutex_lock_scope_impl(*parent);
+  }
+
+  // Movable but not copyable
   aw_mutex_lock_scope(aw_mutex_lock_scope const&) = delete;
   aw_mutex_lock_scope& operator=(aw_mutex_lock_scope const&) = delete;
-  aw_mutex_lock_scope(aw_mutex_lock_scope&&) = delete;
+  inline aw_mutex_lock_scope(aw_mutex_lock_scope&& Other) noexcept
+      : parent(Other.parent) {
+    Other.parent = nullptr;
+  }
   aw_mutex_lock_scope& operator=(aw_mutex_lock_scope&&) = delete;
 };
 
@@ -90,10 +117,11 @@ class [[nodiscard(
 
   inline void await_resume() noexcept {}
 
-  // Copy/move constructors *could* be implemented, but why?
+  // Movable so that it can be captured by value into a wrapper task when
+  // passed to spawn() / fork(), but not copyable.
   aw_mutex_co_unlock(aw_mutex_co_unlock const&) = delete;
   aw_mutex_co_unlock& operator=(aw_mutex_co_unlock const&) = delete;
-  aw_mutex_co_unlock(aw_mutex_co_unlock&&) = delete;
+  aw_mutex_co_unlock(aw_mutex_co_unlock&&) = default;
   aw_mutex_co_unlock& operator=(aw_mutex_co_unlock&&) = delete;
 };
 
@@ -135,17 +163,19 @@ class [[nodiscard(
 
   [[maybe_unused]] inline void await_resume() noexcept {}
 
+  // Movable so that it can be captured by value into a wrapper task when
+  // passed to spawn() / fork(), but not copyable.
   aw_mutex_co_unlock_return(aw_mutex_co_unlock_return const&) = delete;
   aw_mutex_co_unlock_return&
   operator=(aw_mutex_co_unlock_return const&) = delete;
-  aw_mutex_co_unlock_return(aw_mutex_co_unlock_return&&) = delete;
+  aw_mutex_co_unlock_return(aw_mutex_co_unlock_return&&) = default;
   aw_mutex_co_unlock_return& operator=(aw_mutex_co_unlock_return&&) = delete;
 };
 
 /// An async version of std::mutex.
 class mutex : protected tmc::detail::waiter_data_base {
   friend class aw_acquire;
-  friend class aw_mutex_lock_scope;
+  friend class aw_mutex_lock_scope_impl;
   friend class aw_mutex_co_unlock;
   template <typename Result> friend class aw_mutex_co_unlock_return;
   friend class ::tmc::tests::waiter_count_accessor;
@@ -257,7 +287,7 @@ class mutex : protected tmc::detail::waiter_data_base {
   /// ownership to this task. Not re-entrant.
   /// Returns an object that will unlock the mutex (and resume an awaiter) when
   /// it goes out of scope.
-  inline aw_mutex_lock_scope lock_scope() noexcept {
+  inline aw_mutex_lock_scope lock_scope() noexcept TMC_LIFETIMEBOUND {
     return aw_mutex_lock_scope(*this);
   }