aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIain Sandoe <iain@sandoe.co.uk>2022-11-30 17:05:56 +0000
committerIain Sandoe <iain@sandoe.co.uk>2022-12-04 10:39:36 +0000
commit58a7b1e354530d8dfe7d8fb859c8b8b5a9140f1f (patch)
tree1d5aa2a02785dc85b25480d17af01e2af6a0055b
parent8c45e67ac673bbddaaf9770a1a6944b382174938 (diff)
downloadgcc-58a7b1e354530d8dfe7d8fb859c8b8b5a9140f1f.zip
gcc-58a7b1e354530d8dfe7d8fb859c8b8b5a9140f1f.tar.gz
gcc-58a7b1e354530d8dfe7d8fb859c8b8b5a9140f1f.tar.bz2
coroutines: Do not promote temporaries that will be elided.
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.
-rw-r--r--gcc/cp/coroutines.cc1
-rw-r--r--gcc/testsuite/g++.dg/coroutines/pr100611.C94
-rw-r--r--gcc/testsuite/g++.dg/coroutines/pr101367.C72
-rw-r--r--gcc/testsuite/g++.dg/coroutines/pr101976.C78
-rw-r--r--gcc/testsuite/g++.dg/coroutines/pr99576_1.C124
-rw-r--r--gcc/testsuite/g++.dg/coroutines/pr99576_2.C72
6 files changed, 441 insertions, 0 deletions
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 01a3e83..3f23317 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 0000000..14edf48
--- /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 0000000..0a9e5be
--- /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 0000000..1854ba0
--- /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 0000000..612f0cd
--- /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 0000000..b7371d6
--- /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;
+}