Promises: Fix composition problems of loops of seq (#27337)

* Promises: Fix composition problems of loops of seq

* Automated change: Fix sanity tests

Co-authored-by: ctiller <ctiller@users.noreply.github.com>

build_autogenerated.yaml

@@ -6590,10 +6590,14 @@ targets:
   build: test
   language: c++
   headers:
+  - src/core/lib/gprpp/construct_destruct.h
+  - src/core/lib/promise/detail/basic_seq.h
   - src/core/lib/promise/detail/promise_factory.h
   - src/core/lib/promise/detail/promise_like.h
+  - src/core/lib/promise/detail/switch.h
   - src/core/lib/promise/loop.h
   - src/core/lib/promise/poll.h
+  - src/core/lib/promise/seq.h
   src:
   - test/core/promise/loop_test.cc
   deps:

src/core/lib/promise/detail/basic_seq.h

@@ -68,12 +68,14 @@ struct SeqState {
   // Move constructor - assumes we're in the initial state (move prior) as it's
   // illegal to move a promise after polling it.
   SeqState(SeqState&& other) noexcept
-      : prior(std::move(other.prior)),
-        next_factory(std::move(other.next_factory)) {}
+      : next_factory(std::move(other.next_factory)) {
+    new (&prior) PriorState(std::move(other.prior));
+  }
   // Copy constructor - assumes we're in the initial state (move prior) as it's
   // illegal to move a promise after polling it.
-  SeqState(const SeqState& other)
-      : prior(other.prior), next_factory(other.next_factory) {}
+  SeqState(const SeqState& other) : next_factory(other.next_factory) {
+    new (&prior) PriorState(std::move(other.prior));
+  }
   // Empty destructor - we instead destruct the innards in BasicSeq manually
   // depending on state.
   ~SeqState() {}
@@ -115,7 +117,7 @@ struct SeqState<Traits, 0, Fs...> {
         next_factory(other.next_factory) {}
   // Empty destructor - we instead destruct the innards in BasicSeq manually
   // depending on state.
-  ~SeqState(){};
+  ~SeqState() {}
   // Evaluate the current promise, next promise factory types for this state.
   // Our callable is the first element of Fs, wrapped in PromiseLike to handle
   // some common edge cases. The next factory is the second element.

src/core/lib/promise/loop.h

@@ -54,6 +54,13 @@ class Loop {
   explicit Loop(F f) : factory_(std::move(f)), promise_(factory_.Repeated()) {}
   ~Loop() { promise_.~Promise(); }
 
+  Loop(Loop&& loop) noexcept
+      : factory_(std::move(loop.factory_)),
+        promise_(std::move(loop.promise_)) {}
+
+  Loop(const Loop& loop) = delete;
+  Loop& operator=(const Loop& loop) = delete;
+
   Poll<Result> operator()() {
     while (true) {
       // Poll the inner promise.

test/core/promise/BUILD

@@ -121,6 +121,7 @@ grpc_cc_test(
     uses_polling = False,
     deps = [
         "//:loop",
+        "//:seq",
         "//test/core/util:grpc_suppressions",
     ],
 )

test/core/promise/loop_test.cc

@@ -16,6 +16,8 @@
 
 #include <gtest/gtest.h>
 
+#include "src/core/lib/promise/seq.h"
+
 namespace grpc_core {
 
 TEST(LoopTest, CountToFive) {
@@ -40,6 +42,12 @@ TEST(LoopTest, FactoryCountToFive) {
   EXPECT_EQ(i, 5);
 }
 
+TEST(LoopTest, LoopOfSeq) {
+  auto x =
+      Loop(Seq([]() { return 42; }, [](int i) -> LoopCtl<int> { return i; }))();
+  EXPECT_EQ(x, Poll<int>(42));
+}
+
 }  // namespace grpc_core
 
 int main(int argc, char** argv) {