[gcc(refs/users/giulianob/heads/autopar_rebase2)] coroutines: Implicitly movable objects should use move CTORs for co_return.
Giuliano Belinassi
giulianob@gcc.gnu.org
Mon Aug 17 22:28:53 GMT 2020
https://gcc.gnu.org/g:0cb227752d38a274c312138c3038fee200ef284b
commit 0cb227752d38a274c312138c3038fee200ef284b
Author: Iain Sandoe <iain@sandoe.co.uk>
Date: Sat May 16 19:23:19 2020 +0100
coroutines: Implicitly movable objects should use move CTORs for co_return.
This is a case where the standard contains conflicting information.
after discussion between implementators, the accepted intent is of
[class.copy.elision]. This amends the handling of co_return statements
to follow that.
gcc/cp/ChangeLog:
2020-05-16 Iain Sandoe <iain@sandoe.co.uk>
* coroutines.cc (finish_co_return_stmt): Implement rules
from [class.copy.elision] /3.
gcc/testsuite/ChangeLog:
2020-05-16 Iain Sandoe <iain@sandoe.co.uk>
* g++.dg/coroutines/co-return-syntax-10-movable.C: New test.
Diff:
---
gcc/cp/ChangeLog | 5 +
gcc/cp/coroutines.cc | 109 ++++++++++++++-------
gcc/testsuite/ChangeLog | 4 +
.../coroutines/co-return-syntax-10-movable.C | 67 +++++++++++++
4 files changed, 150 insertions(+), 35 deletions(-)
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 4b9fc52b6e3..7ac074a19b6 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,8 @@
+2020-05-16 Iain Sandoe <iain@sandoe.co.uk>
+
+ * coroutines.cc (finish_co_return_stmt): Implement rules
+ from [class.copy.elision] /3.
+
2020-05-16 Patrick Palka <ppalka@redhat.com>
PR c++/57943
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 730e6fef82a..facfafaaa86 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -969,16 +969,23 @@ finish_co_yield_expr (location_t kw, tree expr)
return op;
}
-/* Check that it's valid to have a co_return keyword here.
+/* Check and build a co_return statememt.
+ First that it's valid to have a co_return keyword here.
If it is, then check and build the p.return_{void(),value(expr)}.
- These are built against the promise proxy, but saved for expand time. */
+ These are built against a proxy for the promise, which will be filled
+ in with the actual frame version when the function is transformed. */
tree
finish_co_return_stmt (location_t kw, tree expr)
{
- if (expr == error_mark_node)
+ if (expr)
+ STRIP_ANY_LOCATION_WRAPPER (expr);
+
+ if (error_operand_p (expr))
return error_mark_node;
+ /* If it fails the following test, the function is not permitted to be a
+ coroutine, so the co_return statement is erroneous. */
if (!coro_common_keyword_context_valid_p (current_function_decl, kw,
"co_return"))
return error_mark_node;
@@ -987,32 +994,32 @@ finish_co_return_stmt (location_t kw, tree expr)
already. */
DECL_COROUTINE_P (current_function_decl) = 1;
- if (processing_template_decl)
- {
- current_function_returns_value = 1;
+ /* This function will appear to have no return statement, even if it
+ is declared to return non-void (most likely). This is correct - we
+ synthesize the return for the ramp in the compiler. So suppress any
+ extraneous warnings during substitution. */
+ TREE_NO_WARNING (current_function_decl) = true;
- if (check_for_bare_parameter_packs (expr))
- return error_mark_node;
+ if (processing_template_decl
+ && check_for_bare_parameter_packs (expr))
+ return error_mark_node;
- tree functype = TREE_TYPE (current_function_decl);
- /* If we don't know the promise type, we can't proceed, return the
- expression as it is. */
- if (dependent_type_p (functype) || type_dependent_expression_p (expr))
- {
- expr
- = build2_loc (kw, CO_RETURN_EXPR, void_type_node, expr, NULL_TREE);
- expr = maybe_cleanup_point_expr_void (expr);
- expr = add_stmt (expr);
- return expr;
- }
+ /* If we don't know the promise type, we can't proceed, build the
+ co_return with the expression unchanged. */
+ tree functype = TREE_TYPE (current_function_decl);
+ if (dependent_type_p (functype) || type_dependent_expression_p (expr))
+ {
+ /* co_return expressions are always void type, regardless of the
+ expression type. */
+ expr = build2_loc (kw, CO_RETURN_EXPR, void_type_node,
+ expr, NULL_TREE);
+ expr = maybe_cleanup_point_expr_void (expr);
+ return add_stmt (expr);
}
if (!coro_promise_type_found_p (current_function_decl, kw))
return error_mark_node;
- if (error_operand_p (expr))
- return error_mark_node;
-
/* Suppress -Wreturn-type for co_return, we need to check indirectly
whether the promise type has a suitable return_void/return_value. */
TREE_NO_WARNING (current_function_decl) = true;
@@ -1020,16 +1027,29 @@ finish_co_return_stmt (location_t kw, tree expr)
if (!processing_template_decl && warn_sequence_point)
verify_sequence_points (expr);
+ if (expr)
+ {
+ /* If we had an id-expression obfuscated by force_paren_expr, we need
+ to undo it so we can try to treat it as an rvalue below. */
+ expr = maybe_undo_parenthesized_ref (expr);
+
+ if (processing_template_decl)
+ expr = build_non_dependent_expr (expr);
+
+ if (error_operand_p (expr))
+ return error_mark_node;
+ }
+
/* If the promise object doesn't have the correct return call then
there's a mis-match between the co_return <expr> and this. */
- tree co_ret_call = NULL_TREE;
+ tree co_ret_call = error_mark_node;
if (expr == NULL_TREE || VOID_TYPE_P (TREE_TYPE (expr)))
{
tree crv_meth
= lookup_promise_method (current_function_decl,
coro_return_void_identifier, kw,
/*musthave=*/true);
- if (!crv_meth || crv_meth == error_mark_node)
+ if (crv_meth == error_mark_node)
return error_mark_node;
co_ret_call = build_new_method_call (
@@ -1042,13 +1062,37 @@ finish_co_return_stmt (location_t kw, tree expr)
= lookup_promise_method (current_function_decl,
coro_return_value_identifier, kw,
/*musthave=*/true);
- if (!crv_meth || crv_meth == error_mark_node)
+ if (crv_meth == error_mark_node)
return error_mark_node;
- vec<tree, va_gc> *args = make_tree_vector_single (expr);
- co_ret_call = build_new_method_call (
- get_coroutine_promise_proxy (current_function_decl), crv_meth, &args,
- NULL_TREE, LOOKUP_NORMAL, NULL, tf_warning_or_error);
+ /* [class.copy.elision] / 3.
+ An implicitly movable entity is a variable of automatic storage
+ duration that is either a non-volatile object or an rvalue reference
+ to a non-volatile object type. For such objects in the context of
+ the co_return, the overload resolution should be carried out first
+ treating the object as an rvalue, if that fails, then we fall back
+ to regular overload resolution. */
+
+ if (treat_lvalue_as_rvalue_p (expr, /*parm_ok*/true)
+ && CLASS_TYPE_P (TREE_TYPE (expr))
+ && !TYPE_VOLATILE (TREE_TYPE (expr)))
+ {
+ vec<tree, va_gc> *args = make_tree_vector_single (move (expr));
+ /* It's OK if this fails... */
+ co_ret_call = build_new_method_call
+ (get_coroutine_promise_proxy (current_function_decl), crv_meth,
+ &args, NULL_TREE, LOOKUP_NORMAL|LOOKUP_PREFER_RVALUE,
+ NULL, tf_none);
+ }
+
+ if (co_ret_call == error_mark_node)
+ {
+ vec<tree, va_gc> *args = make_tree_vector_single (expr);
+ /* ... but this must succeed if we didn't get the move variant. */
+ co_ret_call = build_new_method_call
+ (get_coroutine_promise_proxy (current_function_decl), crv_meth,
+ &args, NULL_TREE, LOOKUP_NORMAL, NULL, tf_warning_or_error);
+ }
}
/* Makes no sense for a co-routine really. */
@@ -1057,13 +1101,8 @@ finish_co_return_stmt (location_t kw, tree expr)
"function declared %<noreturn%> has a"
" %<co_return%> statement");
- if (!co_ret_call || co_ret_call == error_mark_node)
- return error_mark_node;
-
expr = build2_loc (kw, CO_RETURN_EXPR, void_type_node, expr, co_ret_call);
- expr = maybe_cleanup_point_expr_void (expr);
- expr = add_stmt (expr);
- return expr;
+ return finish_expr_stmt (expr);
}
/* We need to validate the arguments to __builtin_coro_promise, since the
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index a7f777118f2..9d757c47e19 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2020-05-16 Iain Sandoe <iain@sandoe.co.uk>
+
+ * g++.dg/coroutines/co-return-syntax-10-movable.C: New test.
+
2020-05-16 Patrick Palka <ppalka@redhat.com>
PR c++/57943
diff --git a/gcc/testsuite/g++.dg/coroutines/co-return-syntax-10-movable.C b/gcc/testsuite/g++.dg/coroutines/co-return-syntax-10-movable.C
new file mode 100644
index 00000000000..e2c47a9ec1b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/co-return-syntax-10-movable.C
@@ -0,0 +1,67 @@
+// Check that we obey the extra rules for implicitly movable co_return
+// objects [class.copy.elision]/3.
+
+#include "coro.h"
+
+#include <utility>
+
+template <typename T>
+struct coro1 {
+ struct promise_type;
+ using handle_type = coro::coroutine_handle<coro1::promise_type>;
+ handle_type handle;
+ coro1 () : handle(0) {}
+ coro1 (handle_type _handle)
+ : handle(_handle) { }
+ coro1 (const coro1 &) = delete; // no copying
+ coro1 (coro1 &&s) : handle(s.handle) { s.handle = nullptr; }
+ coro1 &operator = (coro1 &&s) {
+ handle = s.handle;
+ s.handle = nullptr;
+ return *this;
+ }
+ ~coro1() {
+ if ( handle )
+ handle.destroy();
+ }
+
+ struct promise_type {
+ T value;
+ promise_type() {}
+ ~promise_type() {}
+
+ auto get_return_object () { return handle_type::from_promise (*this);}
+ coro::suspend_always initial_suspend () const { return {}; }
+ coro::suspend_always final_suspend () const { return {}; }
+
+ void return_value(T&& v) noexcept { value = std::move(v); }
+
+ T get_value (void) { return value; }
+ void unhandled_exception() { }
+ };
+};
+
+struct MoveOnlyType
+{
+ int value_;
+
+ explicit MoveOnlyType() noexcept : value_(0) {}
+ explicit MoveOnlyType(int value) noexcept : value_(value) {}
+
+ MoveOnlyType(MoveOnlyType&& other) noexcept
+ : value_(std::exchange(other.value_, -1)) {}
+
+ MoveOnlyType& operator=(MoveOnlyType&& other) noexcept {
+ value_ = std::exchange(other.value_, -1);
+ return *this;
+ }
+
+ ~MoveOnlyType() { value_ = -2; }
+};
+
+coro1<MoveOnlyType>
+my_coro ()
+{
+ MoveOnlyType x{10};
+ co_return x;
+}
More information about the Gcc-cvs
mailing list