c++, v2: Implement C++29 P3950R1 - return_value & return_void Are Not Mutually Exclusive paper as DR [PR125830]

Message ID asCR3th04tGkiIbR@tucnak
State New
Headers
Series c++, v2: Implement C++29 P3950R1 - return_value & return_void Are Not Mutually Exclusive paper as DR [PR125830] |

Commit Message

Jakub Jelinek Oct. 3, 2026, 5:25 a.m. UTC
  On Fri, Oct 02, 2026 at 11:20:45PM +0200, Jason Merrill wrote:
> Everything in that function should change tf_warning_or_error to complain.

Ok, here is an updated patch which adds it to the remaining call -
build_op_call.

> > So far lightly tested (on coroutines/* and affected tests), ok for trunk
> > if it passes full bootstrap/regtest?  And what to do about the above
> > mentioned bugs in the code being removed?  Shall I address those first
> > and then send this patch to remove it afterwards (and just backport that
> > first patch and not the second)?
> 
> Do you think it's important to fix those bugs on release branches?  They
> sound like minor diagnostic quality issues.

It is an ICE after error, e.g. g++ 15.2.1 emits on:
#if __has_include(<coroutine>)
#include <coroutine>
#else
#include <experimental/coroutine>
namespace std { using namespace experimental; }
#endif

struct dummy_coroutine {};

namespace std {

template<>
class coroutine_traits<::dummy_coroutine> {
public:
    struct promise_type {
        void return_value(int x) {  }
        void return_void() {}
        void return_void(int) {}
        std::suspend_never initial_suspend() noexcept { return {}; }
        std::suspend_never final_suspend() noexcept { return {}; }
        dummy_coroutine get_return_object() { return {}; }
        void unhandled_exception() {}
    };
};

}

int x;

dummy_coroutine
foo() {
  if (x == 1)
    co_return 17;
  else if (x == 2)
    co_return;
  else if (x == 3)
    co_return;
}

g++ -S -std=c++20 /tmp/pr97438.C -o /tmp/pr97438.s
/tmp/pr97438.C: In function ‘dummy_coroutine foo()’:
/tmp/pr97438.C:31:1: error: the coroutine promise type ‘std::__n4861::coroutine_traits<dummy_coroutine>::promise_type’ declares both ‘return_value’ and ‘return_void’
   31 | foo() {
      | ^~~
/tmp/pr97438.C:-61845579:33: note: ‘return_void’ declared here
/tmp/pr97438.C:16:14: note: ‘return_value’ declared here
   16 |         void return_value(int x) {  }
      |              ^~~~~~~~~~~~
/tmp/pr97438.C:35: confused by earlier errors, bailing out
Preprocessed source stored into /tmp/cc2bwCtQ.out file, please attach this to your bugreport.

Anyway, following patch has been successfully bootstrapped/regtested on
x86_64-linux and i686-linux.

2026-10-03  Jakub Jelinek  <jakub@redhat.com>

	P3950R1
	PR c++/125830
	* c-cppbuiltin.cc (c_cpp_builtins): Bump __cpp_impl_coroutine
	from 201902L to 202606L.

	* coroutines.cc: Implement C++29 P3950R1 - return_value & return_void
	Are Not Mutually Exclusive paper as DR.
	(struct coroutine_info): Remove coro_co_return_error_emitted member.
	(coro_promise_type_found_p): Remove diagnostics about both return_void
	and return_value successful lookup.
	(lookup_promise_method): Add complain argument, pass it down to
	lookup_member.
	(coro_build_promise_expression): Add complain argument, pass it down
	to lookup_promise_method, build_new_method_call and
	build_class_member_access_expr.
	(get_coroutine_return_void_expr): Pass tf_none to
	coro_build_promise_expression.

	* g++.dg/DRs/dr2581-2.C (__cpp_impl_coroutine): Redefine to 202606L
	rather than 201902L.
	* g++.dg/coroutines/pr105301.C: Change dg-error to dg-bogus.
	* g++.dg/coroutines/coro-pre-proc.C: Expect different value of
	__cpp_impl_coroutine.
	* g++.dg/coroutines/pr97438.C: Change dg-error to dg-bogus.
	* g++.dg/coroutines/p3950r1.C: New test.


	Jakub
  

Comments

Jason Merrill Oct. 3, 2026, 7:24 a.m. UTC | #1
On 10/3/26 7:25 AM, Jakub Jelinek wrote:
> On Fri, Oct 02, 2026 at 11:20:45PM +0200, Jason Merrill wrote:
>> Everything in that function should change tf_warning_or_error to complain.
> 
> Ok, here is an updated patch which adds it to the remaining call -
> build_op_call.

OK, thanks.

>>> So far lightly tested (on coroutines/* and affected tests), ok for trunk
>>> if it passes full bootstrap/regtest?  And what to do about the above
>>> mentioned bugs in the code being removed?  Shall I address those first
>>> and then send this patch to remove it afterwards (and just backport that
>>> first patch and not the second)?
>>
>> Do you think it's important to fix those bugs on release branches?  They
>> sound like minor diagnostic quality issues.
> 
> It is an ICE after error, e.g. g++ 15.2.1 emits on:

Yeah, ice-after-error fixes don't seem important to backport.

Jason
  

Patch

--- a/gcc/c-family/c-cppbuiltin.cc	2026-09-10 16:56:41.686638893 +0200
+++ b/gcc/c-family/c-cppbuiltin.cc	2026-10-01 12:42:04.068512619 +0200
@@ -1152,7 +1152,7 @@  c_cpp_builtins (cpp_reader *pfile)
       else if (cxx_dialect >= cxx20)
 	cpp_warn (pfile, "__cpp_modules");
       if (flag_coroutines)
-	cpp_define (pfile, "__cpp_impl_coroutine=201902L"); /* n4861, DIS */
+	cpp_define (pfile, "__cpp_impl_coroutine=202606L"); /* n4861, DIS */
       else if (cxx_dialect >= cxx20)
 	cpp_warn (pfile, "__cpp_impl_coroutine");
       if (flag_tm)
--- a/gcc/cp/coroutines.cc	2026-09-25 17:52:15.184532722 +0200
+++ g/gcc/cp/coroutines.cc	2026-10-02 23:36:36.084757544 +0200
@@ -298,7 +298,6 @@  struct GTY((for_user)) coroutine_info
   /* Flags to avoid repeated errors for per-function issues.  */
   bool coro_ret_type_error_emitted;
   bool coro_promise_error_emitted;
-  bool coro_co_return_error_emitted;
 };
 
 struct coroutine_info_hasher : ggc_ptr_hash<coroutine_info>
@@ -825,37 +824,6 @@  coro_promise_type_found_p (tree fndecl,
 	  return false;
 	}
 
-      /* Test for errors in the promise type that can be determined now.  */
-      tree has_ret_void = lookup_member (coro_info->promise_type,
-					 coro_return_void_identifier,
-					 /*protect=*/1, /*want_type=*/0,
-					 tf_none);
-      tree has_ret_val = lookup_member (coro_info->promise_type,
-					coro_return_value_identifier,
-					/*protect=*/1, /*want_type=*/0,
-					tf_none);
-      if (has_ret_void && has_ret_val)
-	{
-	  auto_diagnostic_group d;
-	  location_t ploc = DECL_SOURCE_LOCATION (fndecl);
-	  if (!coro_info->coro_co_return_error_emitted)
-	    error_at (ploc, "the coroutine promise type %qT declares both"
-		      " %<return_value%> and %<return_void%>",
-		      coro_info->promise_type);
-	  inform (DECL_SOURCE_LOCATION (BASELINK_FUNCTIONS (has_ret_void)),
-		  "%<return_void%> declared here");
-	  has_ret_val = BASELINK_FUNCTIONS (has_ret_val);
-	  const char *message = "%<return_value%> declared here";
-	  if (TREE_CODE (has_ret_val) == OVERLOAD)
-	    {
-	      has_ret_val = OVL_FIRST (has_ret_val);
-	      message = "%<return_value%> first declared here";
-	    }
-	  inform (DECL_SOURCE_LOCATION (has_ret_val), message);
-	  coro_info->coro_co_return_error_emitted = true;
-	  return false;
-	}
-
       /* Try to find the handle type for the promise.  */
       tree handle_type
 	= instantiate_coro_handle_for_promise_type (loc, coro_info->promise_type);
@@ -1028,12 +996,13 @@  get_coroutine_from_address (tree decl)
 
 static tree
 lookup_promise_method (tree fndecl, tree member_id, location_t loc,
-		       bool musthave)
+		       bool musthave,
+		       tsubst_flags_t complain = tf_warning_or_error)
 {
   tree promise = get_coroutine_promise_type (fndecl);
   tree pm_memb
     = lookup_member (promise, member_id,
-		     /*protect=*/1, /*want_type=*/0, tf_warning_or_error);
+		     /*protect=*/1, /*want_type=*/0, complain);
   if (musthave && pm_memb == NULL_TREE)
     {
       error_at (loc, "no member named %qE in %qT", member_id, promise);
@@ -1051,9 +1020,10 @@  lookup_promise_method (tree fndecl, tree
 static tree
 coro_build_promise_expression (tree fn, tree promise_obj, tree member_id,
 			       location_t loc, vec<tree, va_gc> **args,
-			       bool musthave)
+			       bool musthave,
+			       tsubst_flags_t complain = tf_warning_or_error)
 {
-  tree meth = lookup_promise_method (fn, member_id, loc, musthave);
+  tree meth = lookup_promise_method (fn, member_id, loc, musthave, complain);
   if (meth == error_mark_node)
     return error_mark_node;
 
@@ -1067,17 +1037,17 @@  coro_build_promise_expression (tree fn,
   tree expr;
   if (BASELINK_P (meth))
     expr = build_new_method_call (promise, meth, args, NULL_TREE,
-				  LOOKUP_NORMAL, NULL, tf_warning_or_error);
+				  LOOKUP_NORMAL, NULL, complain);
   else
     {
       expr = build_class_member_access_expr (promise, meth, NULL_TREE,
-					     true, tf_warning_or_error);
+					     true, complain);
       vec<tree, va_gc> *real_args;
       if (!args)
 	real_args = make_tree_vector ();
       else
 	real_args = *args;
-      expr = build_op_call (expr, &real_args, tf_warning_or_error);
+      expr = build_op_call (expr, &real_args, complain);
     }
   return expr;
 }
@@ -1094,7 +1064,7 @@  get_coroutine_return_void_expr (tree dec
 	info->return_void
 	  = coro_build_promise_expression (current_function_decl, NULL,
 					   coro_return_void_identifier,
-					   loc, NULL, musthave);
+					   loc, NULL, musthave, tf_none);
       /* Don't return an error if it's an optional call.  */
       if (!musthave && info->return_void == error_mark_node)
 	return NULL_TREE;
--- a/gcc/testsuite/g++.dg/DRs/dr2581-2.C	2026-08-06 11:48:09.058480111 +0200
+++ b/gcc/testsuite/g++.dg/DRs/dr2581-2.C	2026-10-01 12:44:13.876768629 +0200
@@ -61,7 +61,7 @@ 
 #define __cpp_hex_float 201603L			// { dg-error "'__cpp_hex_float' redefined" "" { target c++20 } }
 #define __cpp_if_consteval 202106L		// { dg-error "'__cpp_if_consteval' redefined" "" { target c++23 } }
 #define __cpp_if_constexpr 201606L		// { dg-error "'__cpp_if_constexpr' redefined" "" { target c++20 } }
-#define __cpp_impl_coroutine 201902L		// { dg-error "'__cpp_impl_coroutine' redefined" "" { target c++20 } }
+#define __cpp_impl_coroutine 202606L		// { dg-error "'__cpp_impl_coroutine' redefined" "" { target c++20 } }
 #define __cpp_impl_destroying_delete 201806L	// { dg-error "'__cpp_impl_destroying_delete' redefined" "" { target c++20 } }
 #define __cpp_impl_three_way_comparison 201907L	// { dg-error "'__cpp_impl_three_way_comparison' redefined" "" { target c++20 } }
 #define __cpp_impl_reflection 202603L		// { dg-error "'__cpp_impl_reflection' redefined" "" { target c++26 } }
--- a/gcc/testsuite/g++.dg/coroutines/pr105301.C	2026-03-27 10:17:15.380310407 +0100
+++ b/gcc/testsuite/g++.dg/coroutines/pr105301.C	2026-10-01 12:40:29.792779226 +0200
@@ -46,7 +46,7 @@  struct bad_promise_6 {
 };
 
 coro<bad_promise_6>
-bad_implicit_return() // { dg-error {.aka 'bad_promise_6'. declares both 'return_value' and 'return_void'} }
+bad_implicit_return() // { dg-bogus {.aka 'bad_promise_6'. declares both 'return_value' and 'return_void'} }
 {
   co_await a;
 }
--- a/gcc/testsuite/g++.dg/coroutines/coro-pre-proc.C	2026-03-27 10:17:15.379310423 +0100
+++ b/gcc/testsuite/g++.dg/coroutines/coro-pre-proc.C	2026-10-01 12:44:29.900553347 +0200
@@ -4,6 +4,6 @@ 
 #error "coroutines should engaged."
 #endif
 
-#if __cpp_impl_coroutine != 201902L
+#if __cpp_impl_coroutine != 202606L
 #error "coroutine version out of sync."
 #endif
--- a/gcc/testsuite/g++.dg/coroutines/pr97438.C	2026-03-27 10:17:15.384310342 +0100
+++ b/gcc/testsuite/g++.dg/coroutines/pr97438.C	2026-10-01 12:46:40.857793918 +0200
@@ -25,6 +25,6 @@  public:
 }
 
 dummy_coroutine
-foo() { // { dg-error {the coroutine promise type 'std::__n4861::coroutine_traits<dummy_coroutine>::promise_type' declares both 'return_value' and 'return_void'} }
+foo() { // { dg-bogus {the coroutine promise type 'std::__n4861::coroutine_traits<dummy_coroutine>::promise_type' declares both 'return_value' and 'return_void'} }
     co_return 17;
 }
--- a/gcc/testsuite/g++.dg/coroutines/p3950r1.C	2026-10-01 12:52:43.323958890 +0200
+++ b/gcc/testsuite/g++.dg/coroutines/p3950r1.C	2026-10-01 13:06:52.331832286 +0200
@@ -0,0 +1,74 @@ 
+// C++29 P3950R1 - return_value & return_void Are Not Mutually Exclusive
+
+#include <coroutine>
+#include <type_traits>
+
+struct dummy_coroutine1 {};
+template <typename T>
+struct dummy_coroutine2 {};
+
+namespace std {
+
+template<>
+class coroutine_traits<::dummy_coroutine1> {
+public:
+  struct promise_type {
+    void return_value(int x) {}
+    void return_value(double x) {}
+    void return_void() {}
+    std::suspend_never initial_suspend() noexcept { return {}; }
+    std::suspend_never final_suspend() noexcept { return {}; }
+    dummy_coroutine1 get_return_object() { return {}; }
+    void unhandled_exception() {}
+  };
+};
+
+template<typename T>
+class coroutine_traits<::dummy_coroutine2<T>> {
+public:
+  struct promise_type {
+    template<typename U = T>
+    void return_value(U x) requires std::is_same_v<U, T> {}
+    void return_void() requires std::is_same_v<void, T> {}
+    std::suspend_never initial_suspend() noexcept { return {}; }
+    std::suspend_never final_suspend() noexcept { return {}; }
+    dummy_coroutine2<T> get_return_object() { return {}; }
+    void unhandled_exception() {}
+  };
+};
+
+}
+
+int x;
+
+dummy_coroutine1
+foo ()
+{
+  if (x == 1)
+    co_return 17;
+  else if (x == 2)
+    co_return 17.0;
+  else if (x == 3)
+    co_return;
+}
+
+dummy_coroutine2<void>
+bar ()
+{
+  if (x == 4)
+    co_return;
+}
+
+dummy_coroutine2<int>
+baz ()
+{
+  if (x == 5)
+    co_return 42;
+}
+
+dummy_coroutine2<double>
+qux ()
+{
+  if (x == 6)
+    co_return 42.0;
+}