[PATCH] coroutines: Do not promote temporaries that will be elided.

[Iain Sandoe <iains.gcc@gmail.com>]

Thanks to Adrian for working on this and producing the revised test-cases.

This has been tested on x86_64-darwin21 with our testsuite, cppcoro and
a patched version of folly (that enables the experimental coroutines tests)
without regresssion (actually a handful of progressions on folly, plus the
fixes to our suite).

I am not sure how best to backport this; I think we want it at least on 12
and preferably earlier, but the TARGET_EXPR_ELIDING_P flag is not available
there, perhaps we can construct some local function in coroutines.cc that
emulates it? (or maybe the TARGET_EXPR_ELIDING_P and associated fixes is
suitable for backport?)

thanks
Iain

-- *< --

We usually need to 'promote' (i.e. save to the coroutine frame) any temporary
variable that is in a target expression that must persist across an await
expression.  However, if the TE is just used as a direct initializer for
another object it will be elided - and we should not promote it since that
would lead to a DTOR call for something that is never constructed.

Since we now have a mechanism to tell if TEs will be elided, use that.

Although the PRs referenced initially appear to be different issues, they all
stem from this.

Co-Authored-By: Adrian Perl <adrian.perl@web.de>
Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

	PR c++/100611
	PR c++/101367
	PR c++/101976
	PR c++/99576

gcc/cp/ChangeLog:

	* coroutines.cc (find_interesting_subtree): Do not promote temporaries
	that are only used as direct initializers for some other object.

gcc/testsuite/ChangeLog:

	* g++.dg/coroutines/pr100611.C: New test.
	* g++.dg/coroutines/pr101367.C: New test.
	* g++.dg/coroutines/pr101976.C: New test.
	* g++.dg/coroutines/pr99576_1.C: New test.
	* g++.dg/coroutines/pr99576_2.C: New test.
---
 gcc/cp/coroutines.cc                        |   1 +
 gcc/testsuite/g++.dg/coroutines/pr100611.C  |  94 +++++++++++++++
 gcc/testsuite/g++.dg/coroutines/pr101367.C  |  72 ++++++++++++
 gcc/testsuite/g++.dg/coroutines/pr101976.C  |  78 ++++++++++++
 gcc/testsuite/g++.dg/coroutines/pr99576_1.C | 124 ++++++++++++++++++++
 gcc/testsuite/g++.dg/coroutines/pr99576_2.C |  72 ++++++++++++
 6 files changed, 441 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr100611.C
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr101367.C
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr101976.C
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr99576_1.C
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr99576_2.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 01a3e831ee5..3f23317a315 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -2685,6 +2685,7 @@ find_interesting_subtree (tree *expr_p, int *dosub, void *d)
 	}
     }
   else if (tmp_target_expr_p (expr)
+	   && !TARGET_EXPR_ELIDING_P (expr)
 	   && !p->temps_used->contains (expr))
     {
       p->entry = expr_p;
diff --git a/gcc/testsuite/g++.dg/coroutines/pr100611.C b/gcc/testsuite/g++.dg/coroutines/pr100611.C
new file mode 100644
index 00000000000..14edf4870a1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr100611.C
@@ -0,0 +1,94 @@
+// { dg-do run }
+/*
+  Test that instances created in capture clauses within co_await statements do not
+  get 'promoted'. This would lead to the members destructor getting called more
+  than once.
+
+  Correct output should look like:
+  Foo(23) 0xf042d8
+  Foo(const& 23) 0xf042ec
+  ~Foo(23) 0xf042ec
+  After co_await
+  ~Foo(23) 0xf042d8
+*/
+#include <coroutine>
+#include <iostream>
+
+static unsigned int struct_Foo_destructor_counter = 0;
+static bool lambda_was_executed = false;
+
+class Task {
+public:
+  struct promise_type {
+    Task get_return_object() {
+      return {std::coroutine_handle<promise_type>::from_promise(*this)};
+    }
+
+    std::suspend_never initial_suspend() { return {}; }
+    std::suspend_always final_suspend() noexcept { return {}; }
+    void unhandled_exception() {}
+    void return_void() {}
+  };
+
+  ~Task() {
+    if (handle_) {
+      handle_.destroy();
+    }
+  }
+
+  bool await_ready() { return false; }
+  bool await_suspend(std::coroutine_handle<>) { return false; }
+  bool await_resume() { return false; }
+
+private:
+  Task(std::coroutine_handle<promise_type> handle) : handle_(handle) {}
+
+  std::coroutine_handle<promise_type> handle_;
+};
+
+class Foo {
+public:
+  Foo(int id) : id_(id) {
+    std::cout << "Foo(" << id_ << ") " << (void*)this << std::endl;
+  }
+
+  Foo(Foo const& other) : id_(other.id_) {
+    std::cout << "Foo(const& " << id_ << ") " << (void*)this << std::endl;
+  }
+
+  Foo(Foo&& other) : id_(other.id_) {
+    std::cout << "Foo(&& " << id_ << ") " << (void*)this << std::endl;
+  }
+
+  ~Foo() {
+    std::cout << "~Foo(" << id_ << ") " << (void*)this << std::endl;
+    struct_Foo_destructor_counter++;
+
+    if (struct_Foo_destructor_counter > 2){
+      std::cout << "Foo was destroyed more than two times!\n";
+      __builtin_abort();
+    }
+    }
+
+private:
+  int id_;
+};
+
+Task test() {
+  Foo foo(23);
+
+  co_await [foo]() -> Task { // A copy of foo is captured. This copy must not get 'promoted'.
+    co_return;
+  }();
+
+  std::cout << "After co_await\n";
+  if (struct_Foo_destructor_counter == 0){
+    std::cout << "The captured copy of foo was not destroyed after the co_await statement!\n";
+    __builtin_abort();
+  }
+}
+
+int main() {
+  test();
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr101367.C b/gcc/testsuite/g++.dg/coroutines/pr101367.C
new file mode 100644
index 00000000000..0a9e5bee7d1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr101367.C
@@ -0,0 +1,72 @@
+// { dg-do run }
+
+#include <coroutine>
+using namespace std;
+#include <cstdio>
+#include <utility>
+#include <string>
+
+struct resource {
+    template<typename Func>
+    resource(Func fn) { fn(); /*std::printf("resource()\n"); */}
+    ~resource() { /*std::printf("~resource()\n"); */}
+    resource(resource&&) = delete;
+};
+
+template<typename T>
+struct generator {
+    struct promise_type {
+        generator get_return_object() {
+            return generator{coroutine_handle<promise_type>::from_promise(*this)};
+        }
+
+        void return_void() {}
+        void unhandled_exception() {}
+        suspend_always initial_suspend() { return {}; }
+        suspend_always final_suspend() noexcept { return {}; }
+
+        struct awaitable {
+            resource& r;
+
+            awaitable(resource&& r) : r(r) {}
+
+            bool await_ready() noexcept { return false; }
+
+            void await_suspend(coroutine_handle<> h) noexcept {
+                //std::printf("awaitable::await_suspend()\n");
+            }
+
+            void await_resume() noexcept {
+                //std::printf("awaitable::await_resume()\n");
+            }
+        };
+
+        awaitable yield_value(resource&& r) {
+            return awaitable{std::move(r)};
+        }
+    };
+
+    generator(coroutine_handle<promise_type> coro) : coro(coro)
+    {}
+
+    generator(generator&& g) noexcept : coro(std::exchange(g.coro, {}))
+    {}
+
+    ~generator() {
+        if (coro) { coro.destroy(); }
+    }
+
+    coroutine_handle<promise_type> coro;
+};
+
+generator<int> f() {
+    std::string s;
+    // if `s` isn't captured things work ok
+    co_yield resource{[s]{}};
+}
+
+int main() {
+    generator x = f();
+    x.coro.resume();
+    x.coro.resume();
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr101976.C b/gcc/testsuite/g++.dg/coroutines/pr101976.C
new file mode 100644
index 00000000000..1854ba001bb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr101976.C
@@ -0,0 +1,78 @@
+// { dg-do run }
+
+/*
+  Test that members of temporary instances in co_await statements do not get
+  'promoted'. This would lead to the members destructor getting called more
+  than once.
+
+  Correct output should look like:
+  Before co_await
+  nontrivial_move() 0x6ec2e1
+  nontrivial_move(nontrivial_move&&) 0x6ed320
+  In subtask
+  ~nontrivial_move() 0x6ed320
+  ~nontrivial_move() 0x6ec2e1
+  After co_await
+*/
+#include <coroutine>
+#include <iostream>
+
+static unsigned int struct_nontrivial_move_destructor_counter = 0;
+
+struct task {
+  struct promise_type {
+    task get_return_object() {
+      return {std::coroutine_handle<promise_type>::from_promise(*this)};
+    }
+    std::suspend_never initial_suspend() { return {}; }
+    std::suspend_never final_suspend() noexcept { return {}; }
+    void unhandled_exception() {}
+    void return_void() {}
+  };
+
+  bool await_ready() { return true; }
+  void await_suspend(std::coroutine_handle<>) {}
+  void await_resume() {}
+
+  std::coroutine_handle<promise_type> m_handle;
+};
+
+struct nontrivial_move {
+  nontrivial_move() {
+    std::cout << "nontrivial_move() " << (void *)this << std::endl;
+  }
+  nontrivial_move(nontrivial_move&&) {
+    std::cout << "nontrivial_move(nontrivial_move&&) " << (void *)this
+              << std::endl;
+  }
+  ~nontrivial_move() {
+    std::cout << "~nontrivial_move() " << (void *)this << std::endl;
+    struct_nontrivial_move_destructor_counter++;
+    if (struct_nontrivial_move_destructor_counter > 2){
+      std::cerr << "The destructor of nontrivial_move was called more than two times!\n";
+      __builtin_abort();
+    }
+  }
+
+  char buf[128]{}; // Example why the move could be non trivial
+};
+
+struct wrapper {
+  nontrivial_move member;
+};
+
+task subtask(wrapper /* unused */) {
+  std::cout << "In subtask\n";
+  co_return;
+}
+
+task main_task() {
+  std::cout << "Before co_await\n";
+  co_await subtask({}); // wrapper must get 'promoted', but not its member
+  std::cout << "After co_await\n";
+}
+
+int main() {
+  main_task();
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr99576_1.C b/gcc/testsuite/g++.dg/coroutines/pr99576_1.C
new file mode 100644
index 00000000000..612f0cda2b1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr99576_1.C
@@ -0,0 +1,124 @@
+// { dg-do run }
+/*
+  Test that instances created in capture clauses within co_await statements do not get
+  'promoted'. This would lead to their members destructors getting called more
+  than once.
+
+  Correct output should look like:
+  START TASK
+  Foo() 0x4f9320
+  IN LAMBDA
+  ~Foo() 0x4f9320
+  TASK RETURN
+*/
+#include <coroutine>
+#include <exception>
+#include <iostream>
+#include <utility>
+
+static unsigned int struct_Foo_destructor_counter = 0;
+static bool lambda_was_executed = false;
+
+class Task {
+public:
+  struct promise_type {
+    struct final_awaitable {
+      bool await_ready() noexcept { return false; }
+      auto await_suspend(std::coroutine_handle<promise_type> coro) noexcept {
+        return coro.promise().continuation;
+      }
+      void await_resume() noexcept {}
+    };
+    Task get_return_object() {
+      return Task(std::coroutine_handle<promise_type>::from_promise(*this));
+    }
+    std::suspend_always initial_suspend() { return {}; }
+    final_awaitable final_suspend() noexcept { return {}; }
+    void unhandled_exception() { std::terminate(); }
+    void return_void() {}
+
+    std::coroutine_handle<void> continuation = std::noop_coroutine();
+  };
+
+  Task(Task const&) = delete;
+  Task(Task&& other) noexcept
+      : handle_(std::exchange(other.handle_, nullptr)) {}
+  Task& operator=(Task const&) = delete;
+  Task& operator=(Task&& other) noexcept {
+    handle_ = std::exchange(other.handle_, nullptr);
+    return *this;
+  }
+  ~Task() {
+    if (handle_) {
+      handle_.destroy();
+    }
+  }
+
+  bool await_ready() const { return false; }
+  auto await_suspend(std::coroutine_handle<void> continuation) {
+    handle_.promise().continuation = continuation;
+    return handle_;
+  }
+  void await_resume() {}
+
+private:
+  explicit Task(std::coroutine_handle<promise_type> handle) : handle_(handle) {}
+
+  std::coroutine_handle<promise_type> handle_;
+};
+
+struct RunTask {
+  struct promise_type {
+    RunTask get_return_object() { return {}; }
+    std::suspend_never initial_suspend() { return {}; }
+    std::suspend_never final_suspend() noexcept { return {}; }
+    void return_void() {}
+    void unhandled_exception() { std::terminate(); }
+  };
+};
+
+struct Foo {
+  Foo() {
+    std::cout << "Foo() " << (void *)this << std::endl;
+  }
+
+  ~Foo() {
+    std::cout << "~Foo() " << (void *)this << std::endl;
+    struct_Foo_destructor_counter++;
+
+    if (struct_Foo_destructor_counter > 1 || !lambda_was_executed) {
+      std::cout << "The destructor of Foo was called more than once or too early!\n";
+      __builtin_abort();
+    }
+  }
+
+  Foo(Foo&&) = delete;
+  Foo(Foo const&) = delete;
+  Foo& operator=(Foo&&) = delete;
+  Foo& operator=(Foo const&) = delete;
+};
+
+Task DoAsync() {
+  std::cout << "START TASK\n";
+  co_await [foo = Foo{}]() -> Task { // foo is constructed inplace, no copy/move is performed.
+                                     // foo itself must not get 'promoted'.
+    std::cout << "IN LAMBDA\n";
+    lambda_was_executed = true;
+    co_return;
+  }();
+  // After the co_await statement the temporary lambda and foo
+  // must now have been destroyed
+  if (struct_Foo_destructor_counter == 0){
+    std::cout << "foo was not destroyed after the co_await statement!\n";
+    __builtin_abort();
+  }
+  std::cout << "TASK RETURN\n";
+  co_return;
+}
+
+RunTask Main() { co_await DoAsync(); }
+
+int main() {
+  Main();
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr99576_2.C b/gcc/testsuite/g++.dg/coroutines/pr99576_2.C
new file mode 100644
index 00000000000..b7371d64480
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr99576_2.C
@@ -0,0 +1,72 @@
+// { dg-do run }
+/*
+  Test that members of temporary awaitables in co_await statements do not get
+  'promoted'. This would lead to the members destructor getting called more
+  than once.
+
+  Correct output should look like:
+  A 0x4f82d6
+  ~A 0x4f82d6
+*/
+#include <coroutine>
+#include <iostream>
+
+
+static unsigned int struct_A_destructor_counter = 0;
+
+struct task : std::coroutine_handle<> {
+  struct promise_type;
+};
+
+struct task::promise_type {
+  task get_return_object() {
+    return {std::coroutine_handle<promise_type>::from_promise(*this)};
+  }
+  std::suspend_always initial_suspend() { return {}; }
+  std::suspend_always final_suspend() noexcept { return {}; }
+  void unhandled_exception() { std::terminate(); }
+  void return_void() {}
+};
+
+struct A {
+  void log(const char *str) { std::cout << str << " " << (void *)this << std::endl; }
+
+  A() { log(__func__); }
+
+  ~A() {
+    log(__func__);
+    struct_A_destructor_counter++;
+
+    if (struct_A_destructor_counter > 1) {
+      std::cout << "The destructor of A was called more than once!\n";
+      __builtin_abort();
+    }
+  }
+
+  A(A&&) = delete;
+  A(A const&) = delete;
+  A& operator=(A&&) = delete;
+  A& operator=(A const&) = delete;
+};
+
+struct Awaitable {
+  A a{}; // <- This member must NOT get 'promoted'
+  bool await_ready() { return false; }
+  void await_suspend(std::coroutine_handle<> handle) {}
+  void await_resume() {}
+};
+
+task coroutine() {
+  co_await Awaitable{}; // <- This temporary must get 'promoted'
+}
+
+int main() {
+
+  auto task = coroutine();
+  while (!task.done()) {
+    task();
+  }
+  task.destroy();
+
+  return 0;
+}
-- 
2.37.1 (Apple Git-137.1)