public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] coroutines: Implicitly movable objects should use move CTORs  for co_return.
@ 2020-05-13 10:59 Iain Sandoe
  2020-05-13 13:17 ` Nathan Sidwell
  0 siblings, 1 reply; 6+ messages in thread
From: Iain Sandoe @ 2020-05-13 10:59 UTC (permalink / raw)
  To: gcc-patches; +Cc: Nathan Sidwell

.. and now to the right list…

I came across a build failure in a folly experimental test case where,
at first, it appeared that GCC was DTRT … however, further
investigation concluded that this was a case of differing interpretations
between implementations.

It’s kinda unhelpful that the discussion appears in class elision
but it motivates the selection of the correct overload even in cases
where there’s no elision.

The (conflicting info) issue is being taken to WG21 / Core.

tested on x86_64-darwin so far,
OK master after regstrap on Linux?
for 10.2 after some bake time on master?
thanks
Iain

----

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-13  Iain Sandoe  <iain@sandoe.co.uk>

	* coroutines.cc (finish_co_return_stmt): Implement rules
	from [class.copy.elision] /3.

gcc/testsuite/ChangeLog:

2020-05-13  Iain Sandoe  <iain@sandoe.co.uk>

	* g++.dg/coroutines/co-return-syntax-10-movable.C: New test.
---
 gcc/cp/coroutines.cc                          | 56 +++++++++++-----
 .../coroutines/co-return-syntax-10-movable.C  | 67 +++++++++++++++++++
 2 files changed, 107 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/co-return-syntax-10-movable.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 730e6fef82a..423c37e1c9c 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -999,11 +999,10 @@ finish_co_return_stmt (location_t kw, tree expr)
 	 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 = build2_loc (kw, CO_RETURN_EXPR, unknown_type_node,
+			     expr, NULL_TREE);
 	  expr = maybe_cleanup_point_expr_void (expr);
-	  expr = add_stmt (expr);
-	  return expr;
+	  return add_stmt (expr);
 	}
     }
 
@@ -1022,7 +1021,7 @@ finish_co_return_stmt (location_t kw, tree expr)
 
   /* 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
@@ -1045,25 +1044,50 @@ finish_co_return_stmt (location_t kw, tree expr)
       if (!crv_meth || 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.  */
+      tree obj = STRIP_NOPS (expr);
+      if (TREE_TYPE (obj)
+	  && TYPE_REF_P (TREE_TYPE (obj))
+	  && TYPE_REF_IS_RVALUE (TREE_TYPE (obj)))
+	obj = TREE_OPERAND (obj, 0);
+
+      if (TREE_CODE (obj) == PARM_DECL
+	  || (VAR_P (obj)
+	      && decl_storage_duration (obj) == dk_auto
+	      && !TYPE_VOLATILE (TREE_TYPE (obj))))
+	{
+	  vec<tree, va_gc> *args = make_tree_vector_single (rvalue (expr));
+	  co_ret_call = build_new_method_call
+	    (get_coroutine_promise_proxy (current_function_decl), crv_meth,
+	     &args, NULL_TREE, LOOKUP_NORMAL, NULL, tf_none);
+	}
+
+      if (co_ret_call == 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);
+	  release_tree_vector (args);
+	}
     }
 
+  expr = build2_loc (kw, CO_RETURN_EXPR, void_type_node, expr, co_ret_call);
+  expr = maybe_cleanup_point_expr_void (expr);
+
   /* Makes no sense for a co-routine really. */
   if (TREE_THIS_VOLATILE (current_function_decl))
     warning_at (kw, 0,
 		"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 add_stmt (expr);
 }
 
 /* We need to validate the arguments to __builtin_coro_promise, since the
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..8c18e13bce2
--- /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);}
+  std::suspend_always initial_suspend () const { return {}; }
+  std::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;
+}
-- 
2.24.1


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-14 15:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-13 10:59 [PATCH] coroutines: Implicitly movable objects should use move CTORs for co_return Iain Sandoe
2020-05-13 13:17 ` Nathan Sidwell
2020-05-13 13:26   ` Iain Sandoe
2020-05-13 15:10     ` Nathan Sidwell
2020-05-14 13:04       ` [PATCH v2] " Iain Sandoe
2020-05-14 15:13         ` Nathan Sidwell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).