[PATCH] coroutines: Accept 'extern "C"' coroutines.

[PATCH] coroutines: Accept 'extern "C"' coroutines.

[Iain Sandoe]

This came up in discussion and was relatively easy to 'fix' although it remains
to be seen whether it was really intended.  Tested on x86_64-darwin21. 
OK for trunk?
Iain

--- >8 ---

As of now the standard does not appear to forbid this, and clang accepts.
The use-cases are somewhat unclear (question placed with the designers) but
implemented here for sake of compatibility.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

gcc/cp/ChangeLog:

	* coroutines.cc (coro_build_actor_or_destroy_function): Accept
	coroutines that are 'extern "C"' and build unmangled names for
	the actor and destroy functions.

gcc/testsuite/ChangeLog:

	* g++.dg/coroutines/torture/extern-c-coroutine.C: New test.
---
 gcc/cp/coroutines.cc                          | 12 +++
 .../coroutines/torture/extern-c-coroutine.C   | 89 +++++++++++++++++++
 2 files changed, 101 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/extern-c-coroutine.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 3f23317a315..c1a0d6c2283 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -32,6 +32,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree.h"
 #include "gcc-rich-location.h"
 #include "hash-map.h"
+#include "cgraph.h"
 
 static bool coro_promise_type_found_p (tree, location_t);
 
@@ -4060,6 +4061,17 @@ coro_build_actor_or_destroy_function (tree orig, tree fn_type,
       else
 	info->destroy_decl = fn;
     }
+  /* If the function is extern "C" the mangler will not be called.  */
+  if (DECL_EXTERN_C_P (orig))
+    {
+      /* clone_function_name () picks up the name from DECL_ASSEMBLER_NAME
+	 which is not yet set here.  */
+      SET_DECL_ASSEMBLER_NAME (fn, DECL_NAME (orig));
+      if (actor_p)
+	SET_DECL_ASSEMBLER_NAME (fn, clone_function_name (fn, "actor"));
+      else
+	SET_DECL_ASSEMBLER_NAME (fn, clone_function_name (fn, "destroy"));
+    }
   return fn;
 }
 
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/extern-c-coroutine.C b/gcc/testsuite/g++.dg/coroutines/torture/extern-c-coroutine.C
new file mode 100644
index 00000000000..c178a80ee4b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/extern-c-coroutine.C
@@ -0,0 +1,89 @@
+#include <coroutine>
+#include <cstdio>
+
+#ifndef OUTPUT
+#  define PRINT(X)
+#  define PRINTF(X,...)
+#else
+#  define PRINT(X) puts(X)
+#  define PRINTF printf
+#endif
+
+struct future {
+  struct promise_type;
+  using handle_type = std::coroutine_handle<future::promise_type>;
+  handle_type handle;
+  future () : handle(0) {}
+  future (handle_type _handle)
+    : handle(_handle) {
+        PRINT("Created future object from handle");
+  }
+  future (const future &) = delete; // no copying
+  future (future &&s) : handle(s.handle) {
+	s.handle = nullptr;
+	PRINT("future mv ctor ");
+  }
+  future &operator = (future &&s) {
+	handle = s.handle;
+	s.handle = nullptr;
+	PRINT("future op=  ");
+	return *this;
+  }
+  ~future() {
+        PRINT("Destroyed future");
+        if ( handle )
+          handle.destroy();
+  }
+
+  struct promise_type {
+    void return_value (int v) {
+      PRINTF ("return_value (%d)\n", v);
+      vv = v;
+    }
+
+    std::suspend_always initial_suspend() noexcept { return {}; }
+    std::suspend_always final_suspend() noexcept { return {}; }
+    void unhandled_exception() {}
+    auto get_return_object() {return handle_type::from_promise (*this);}
+    
+    int get_value () { return vv; }
+  private:
+    int vv;
+  };
+  bool await_ready() { return false; }
+  void await_suspend(std::coroutine_handle<>) {}
+  void await_resume() {}
+};
+
+extern "C" future
+test () {
+  co_return 22;
+}
+
+extern "C" future
+f () noexcept
+{
+  PRINT ("future: about to return");
+  co_return 42;
+}
+
+int main ()
+{
+  PRINT ("main: create future");
+  future x = f ();
+  PRINT ("main: got future - resuming");
+  if (x.handle.done())
+    __builtin_abort ();
+  x.handle.resume();
+  PRINT ("main: after resume");
+  int y = x.handle.promise().get_value();
+  if ( y != 42 )
+    __builtin_abort ();
+  if (!x.handle.done())
+    {
+      PRINT ("main: apparently not done...");
+      __builtin_abort ();
+    }
+  PRINT ("main: returning");
+  return 0;
+}
-- 
2.37.1 (Apple Git-137.1)

Re: [PATCH v2] coroutines: Accept 'extern "C"' coroutines.

[Iain Sandoe]

Hi.

It seems that everyone agrees that extern C coroutines should be permitted,
although I have yet to see a useful testcase.

This patch has been revised to append the suffices for such functions in
mangle.cc rather than as part of the outlined function decl production.

tested on x86_64-darwin21.
OK for trunk?
Iain


— 8< —

'extern "C"' coroutines are permitted by the standard and expected to work
(although constructing useful cases could be challenging). In order to
permit this we need to arrange for the outlined helper functions to be
named properly, even when no mangling is required.  To do this, we append
the actor and destroy suffixes in all cases.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

gcc/cp/ChangeLog:

	* mangle.cc (write_mangled_name): Append the helper function
	suffixes here...
	(write_encoding): ... rather than here.

gcc/testsuite/ChangeLog:

	* g++.dg/coroutines/torture/extern-c-coroutine.C: New test.
---
 gcc/cp/mangle.cc                              | 23 ++---
 .../coroutines/torture/extern-c-coroutine.C   | 89 +++++++++++++++++++
 2 files changed, 101 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/extern-c-coroutine.C

diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
index 074cf27ec7a..5789adcf680 100644
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -805,6 +805,18 @@ write_mangled_name (const tree decl, bool top_level)
     write_string (".pre");
   else if (DECL_IS_POST_FN_P (decl))
     write_string (".post");
+
+  /* If this is a coroutine helper, then append an appropriate string to
+     identify which.  */
+  if (tree ramp = DECL_RAMP_FN (decl))
+    {
+      if (DECL_ACTOR_FN (ramp) == decl)
+	write_string (JOIN_STR "actor");
+      else if (DECL_DESTROY_FN (ramp) == decl)
+	write_string (JOIN_STR "destroy");
+      else
+	gcc_unreachable ();
+    }
 }
 
 /* Returns true if the return type of DECL is part of its signature, and
@@ -863,17 +875,6 @@ write_encoding (const tree decl)
 				mangle_return_type_p (decl),
 				d);
 
-      /* If this is a coroutine helper, then append an appropriate string to
-	 identify which.  */
-      if (tree ramp = DECL_RAMP_FN (decl))
-	{
-	  if (DECL_ACTOR_FN (ramp) == decl)
-	    write_string (JOIN_STR "actor");
-	  else if (DECL_DESTROY_FN (ramp) == decl)
-	    write_string (JOIN_STR "destroy");
-	  else
-	    gcc_unreachable ();
-	}
     }
 }
 
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/extern-c-coroutine.C b/gcc/testsuite/g++.dg/coroutines/torture/extern-c-coroutine.C
new file mode 100644
index 00000000000..c178a80ee4b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/extern-c-coroutine.C
@@ -0,0 +1,89 @@
+#include <coroutine>
+#include <cstdio>
+
+#ifndef OUTPUT
+#  define PRINT(X)
+#  define PRINTF(X,...)
+#else
+#  define PRINT(X) puts(X)
+#  define PRINTF printf
+#endif
+
+struct future {
+  struct promise_type;
+  using handle_type = std::coroutine_handle<future::promise_type>;
+  handle_type handle;
+  future () : handle(0) {}
+  future (handle_type _handle)
+    : handle(_handle) {
+        PRINT("Created future object from handle");
+  }
+  future (const future &) = delete; // no copying
+  future (future &&s) : handle(s.handle) {
+	s.handle = nullptr;
+	PRINT("future mv ctor ");
+  }
+  future &operator = (future &&s) {
+	handle = s.handle;
+	s.handle = nullptr;
+	PRINT("future op=  ");
+	return *this;
+  }
+  ~future() {
+        PRINT("Destroyed future");
+        if ( handle )
+          handle.destroy();
+  }
+
+  struct promise_type {
+    void return_value (int v) {
+      PRINTF ("return_value (%d)\n", v);
+      vv = v;
+    }
+
+    std::suspend_always initial_suspend() noexcept { return {}; }
+    std::suspend_always final_suspend() noexcept { return {}; }
+    void unhandled_exception() {}
+    auto get_return_object() {return handle_type::from_promise (*this);}
+    
+    int get_value () { return vv; }
+  private:
+    int vv;
+  };
+  bool await_ready() { return false; }
+  void await_suspend(std::coroutine_handle<>) {}
+  void await_resume() {}
+};
+
+extern "C" future
+test () {
+  co_return 22;
+}
+
+extern "C" future
+f () noexcept
+{
+  PRINT ("future: about to return");
+  co_return 42;
+}
+
+int main ()
+{
+  PRINT ("main: create future");
+  future x = f ();
+  PRINT ("main: got future - resuming");
+  if (x.handle.done())
+    __builtin_abort ();
+  x.handle.resume();
+  PRINT ("main: after resume");
+  int y = x.handle.promise().get_value();
+  if ( y != 42 )
+    __builtin_abort ();
+  if (!x.handle.done())
+    {
+      PRINT ("main: apparently not done...");
+      __builtin_abort ();
+    }
+  PRINT ("main: returning");
+  return 0;
+}
-- 
2.37.1 (Apple Git-137.1)

Re: [PATCH v2] coroutines: Accept 'extern "C"' coroutines.

[Jason Merrill]

On 12/17/22 08:40, Iain Sandoe wrote:
> Hi.
> 
> It seems that everyone agrees that extern C coroutines should be permitted,
> although I have yet to see a useful testcase.
> 
> This patch has been revised to append the suffices for such functions in
> mangle.cc rather than as part of the outlined function decl production.
> 
> tested on x86_64-darwin21.
> OK for trunk?
> Iain

OK, thanks.

> 
> — 8< —
> 
> 'extern "C"' coroutines are permitted by the standard and expected to work
> (although constructing useful cases could be challenging). In order to
> permit this we need to arrange for the outlined helper functions to be
> named properly, even when no mangling is required.  To do this, we append
> the actor and destroy suffixes in all cases.
> 
> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
> 
> gcc/cp/ChangeLog:
> 
> 	* mangle.cc (write_mangled_name): Append the helper function
> 	suffixes here...
> 	(write_encoding): ... rather than here.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/coroutines/torture/extern-c-coroutine.C: New test.
> ---
>   gcc/cp/mangle.cc                              | 23 ++---
>   .../coroutines/torture/extern-c-coroutine.C   | 89 +++++++++++++++++++
>   2 files changed, 101 insertions(+), 11 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/extern-c-coroutine.C
> 
> diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
> index 074cf27ec7a..5789adcf680 100644
> --- a/gcc/cp/mangle.cc
> +++ b/gcc/cp/mangle.cc
> @@ -805,6 +805,18 @@ write_mangled_name (const tree decl, bool top_level)
>       write_string (".pre");
>     else if (DECL_IS_POST_FN_P (decl))
>       write_string (".post");
> +
> +  /* If this is a coroutine helper, then append an appropriate string to
> +     identify which.  */
> +  if (tree ramp = DECL_RAMP_FN (decl))
> +    {
> +      if (DECL_ACTOR_FN (ramp) == decl)
> +	write_string (JOIN_STR "actor");
> +      else if (DECL_DESTROY_FN (ramp) == decl)
> +	write_string (JOIN_STR "destroy");
> +      else
> +	gcc_unreachable ();
> +    }
>   }
>   
>   /* Returns true if the return type of DECL is part of its signature, and
> @@ -863,17 +875,6 @@ write_encoding (const tree decl)
>   				mangle_return_type_p (decl),
>   				d);
>   
> -      /* If this is a coroutine helper, then append an appropriate string to
> -	 identify which.  */
> -      if (tree ramp = DECL_RAMP_FN (decl))
> -	{
> -	  if (DECL_ACTOR_FN (ramp) == decl)
> -	    write_string (JOIN_STR "actor");
> -	  else if (DECL_DESTROY_FN (ramp) == decl)
> -	    write_string (JOIN_STR "destroy");
> -	  else
> -	    gcc_unreachable ();
> -	}
>       }
>   }
>   
> diff --git a/gcc/testsuite/g++.dg/coroutines/torture/extern-c-coroutine.C b/gcc/testsuite/g++.dg/coroutines/torture/extern-c-coroutine.C
> new file mode 100644
> index 00000000000..c178a80ee4b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/torture/extern-c-coroutine.C
> @@ -0,0 +1,89 @@
> +#include <coroutine>
> +#include <cstdio>
> +
> +#ifndef OUTPUT
> +#  define PRINT(X)
> +#  define PRINTF(X,...)
> +#else
> +#  define PRINT(X) puts(X)
> +#  define PRINTF printf
> +#endif
> +
> +struct future {
> +  struct promise_type;
> +  using handle_type = std::coroutine_handle<future::promise_type>;
> +  handle_type handle;
> +  future () : handle(0) {}
> +  future (handle_type _handle)
> +    : handle(_handle) {
> +        PRINT("Created future object from handle");
> +  }
> +  future (const future &) = delete; // no copying
> +  future (future &&s) : handle(s.handle) {
> +	s.handle = nullptr;
> +	PRINT("future mv ctor ");
> +  }
> +  future &operator = (future &&s) {
> +	handle = s.handle;
> +	s.handle = nullptr;
> +	PRINT("future op=  ");
> +	return *this;
> +  }
> +  ~future() {
> +        PRINT("Destroyed future");
> +        if ( handle )
> +          handle.destroy();
> +  }
> +
> +  struct promise_type {
> +    void return_value (int v) {
> +      PRINTF ("return_value (%d)\n", v);
> +      vv = v;
> +    }
> +
> +    std::suspend_always initial_suspend() noexcept { return {}; }
> +    std::suspend_always final_suspend() noexcept { return {}; }
> +    void unhandled_exception() {}
> +    auto get_return_object() {return handle_type::from_promise (*this);}
> +
> +    int get_value () { return vv; }
> +  private:
> +    int vv;
> +  };
> +  bool await_ready() { return false; }
> +  void await_suspend(std::coroutine_handle<>) {}
> +  void await_resume() {}
> +};
> +
> +extern "C" future
> +test () {
> +  co_return 22;
> +}
> +
> +extern "C" future
> +f () noexcept
> +{
> +  PRINT ("future: about to return");
> +  co_return 42;
> +}
> +
> +int main ()
> +{
> +  PRINT ("main: create future");
> +  future x = f ();
> +  PRINT ("main: got future - resuming");
> +  if (x.handle.done())
> +    __builtin_abort ();
> +  x.handle.resume();
> +  PRINT ("main: after resume");
> +  int y = x.handle.promise().get_value();
> +  if ( y != 42 )
> +    __builtin_abort ();
> +  if (!x.handle.done())
> +    {
> +      PRINT ("main: apparently not done...");
> +      __builtin_abort ();
> +    }
> +  PRINT ("main: returning");
> +  return 0;
> +}