diff mbox series

c++: make BUILTIN_SOURCE_LOCATION follow DECL_RAMP_FN

Message ID 20240729122211.779156-1-arsen@aarsen.me
State New
Headers show
Series c++: make BUILTIN_SOURCE_LOCATION follow DECL_RAMP_FN | expand

Commit Message

Arsen Arsenović July 29, 2024, 12:20 p.m. UTC 
This fixes the value of current_function in compiler generated coroutine
code.

PR c++/110855 - std::source_location doesn't work with C++20 coroutine

gcc/cp/ChangeLog:

	PR c++/110855
	* cp-gimplify.cc (fold_builtin_source_location): Use the name of
	the DECL_RAMP_FN of the current function if present.

gcc/testsuite/ChangeLog:

	PR c++/110855
	* g++.dg/coroutines/pr110855.C: New test.
---
Tested on x86_64-pc-linux-gnu.

OK for trunk?

TIA, have a lovely day.

 gcc/cp/cp-gimplify.cc                      |  9 +++-
 gcc/testsuite/g++.dg/coroutines/pr110855.C | 61 ++++++++++++++++++++++
 2 files changed, 69 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110855.C

Comments

Jason Merrill July 29, 2024, 7:37 p.m. UTC | #1 
I don't know what all-caps BUILTIN_SOURCE_LOCATION refers to 
specifically (it doesn't match CP_BUILT_IN_SOURCE_LOCATION, for 
instance); let's just refer to source_location.

On 7/29/24 8:20 AM, Arsen Arsenović wrote:
> This fixes the value of current_function in compiler generated coroutine
> code.
> 
> PR c++/110855 - std::source_location doesn't work with C++20 coroutine
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/110855
> 	* cp-gimplify.cc (fold_builtin_source_location): Use the name of
> 	the DECL_RAMP_FN of the current function if present.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/110855
> 	* g++.dg/coroutines/pr110855.C: New test.
> ---
> Tested on x86_64-pc-linux-gnu.
> 
> OK for trunk?
> 
> TIA, have a lovely day.

Again, best to put stuff like this that doesn't go in the commit message 
first, followed by scissors (-- 8< --), then the commit message.

The patch is OK.

>   gcc/cp/cp-gimplify.cc                      |  9 +++-
>   gcc/testsuite/g++.dg/coroutines/pr110855.C | 61 ++++++++++++++++++++++
>   2 files changed, 69 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110855.C
> 
> diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
> index e6629dea5fdc..651751312fbe 100644
> --- a/gcc/cp/cp-gimplify.cc
> +++ b/gcc/cp/cp-gimplify.cc
> @@ -3929,7 +3929,14 @@ fold_builtin_source_location (const_tree t)
>   	      const char *name = "";
>   
>   	      if (current_function_decl)
> -		name = cxx_printable_name (current_function_decl, 2);
> +		{
> +		  /* If this is a coroutine, we should get the name of the user
> +		     function rather than the actor we generate.  */
> +		  if (tree ramp = DECL_RAMP_FN (current_function_decl))
> +		    name = cxx_printable_name (ramp, 2);
> +		  else
> +		    name = cxx_printable_name (current_function_decl, 2);
> +		}
>   
>   	      val = build_string_literal (name);
>   	    }
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr110855.C b/gcc/testsuite/g++.dg/coroutines/pr110855.C
> new file mode 100644
> index 000000000000..6b5c0147ec83
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr110855.C
> @@ -0,0 +1,61 @@
> +// { dg-do run }
> +// { dg-output {^} }
> +// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
> +// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
> +// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
> +// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
> +// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
> +// { dg-output {$} }
> +// https://gcc.gnu.org/PR110855
> +#include <coroutine>
> +#include <source_location>
> +
> +struct ReturnObject {
> +  struct promise_type {
> +    auto
> +    initial_suspend(const std::source_location location =
> +                    std::source_location::current()) {
> +      __builtin_puts (location.function_name ());
> +      return std::suspend_never{};
> +    }
> +    auto
> +    final_suspend(const std::source_location location =
> +                  std::source_location::current()) noexcept {
> +      __builtin_puts (location.function_name ());
> +      return std::suspend_never{};
> +    }
> +    auto
> +    get_return_object(const std::source_location location =
> +                      std::source_location::current()) {
> +      __builtin_puts (location.function_name ());
> +      return ReturnObject{std::coroutine_handle<promise_type>::from_promise(*this)};
> +    }
> +    auto
> +    unhandled_exception() { }
> +    auto return_void(const std::source_location location =
> +                     std::source_location::current()) {
> +      __builtin_puts (location.function_name ());
> +    }
> +  };
> +  std::coroutine_handle<> handle;
> +};
> +
> +struct awaitable : std::suspend_never
> +{
> +  void await_resume(const std::source_location location =
> +                     std::source_location::current())
> +  {
> +      __builtin_puts (location.function_name ());
> +  }
> +};
> +
> +ReturnObject
> +bar(int, char, bool) {
> +  co_await awaitable{};
> +  co_return;
> +}
> +
> +int
> +main() {
> +  bar(1, 'a', false);
> +}
Arsen Arsenović July 29, 2024, 11:02 p.m. UTC | #2 
Jason Merrill <jason@redhat.com> writes:

> I don't know what all-caps BUILTIN_SOURCE_LOCATION refers to specifically (it
> doesn't match CP_BUILT_IN_SOURCE_LOCATION, for instance); let's just refer to
> source_location.

ACK - will reword.

> On 7/29/24 8:20 AM, Arsen Arsenović wrote:
>> This fixes the value of current_function in compiler generated coroutine
>> code.
>> PR c++/110855 - std::source_location doesn't work with C++20 coroutine
>> gcc/cp/ChangeLog:
>> 	PR c++/110855
>> 	* cp-gimplify.cc (fold_builtin_source_location): Use the name of
>> 	the DECL_RAMP_FN of the current function if present.
>> gcc/testsuite/ChangeLog:
>> 	PR c++/110855
>> 	* g++.dg/coroutines/pr110855.C: New test.
>> ---
>> Tested on x86_64-pc-linux-gnu.
>> OK for trunk?
>> TIA, have a lovely day.
>
> Again, best to put stuff like this that doesn't go in the commit message first,
> followed by scissors (-- 8< --), then the commit message.

Ah, I see what you mean now.  Sorry, will use that feature in the
future.

> The patch is OK.

Thanks.

>>   gcc/cp/cp-gimplify.cc                      |  9 +++-
>>   gcc/testsuite/g++.dg/coroutines/pr110855.C | 61 ++++++++++++++++++++++
>>   2 files changed, 69 insertions(+), 1 deletion(-)
>>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110855.C
>> diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
>> index e6629dea5fdc..651751312fbe 100644
>> --- a/gcc/cp/cp-gimplify.cc
>> +++ b/gcc/cp/cp-gimplify.cc
>> @@ -3929,7 +3929,14 @@ fold_builtin_source_location (const_tree t)
>>   	      const char *name = "";
>>     	      if (current_function_decl)
>> -		name = cxx_printable_name (current_function_decl, 2);
>> +		{
>> +		  /* If this is a coroutine, we should get the name of the user
>> +		     function rather than the actor we generate.  */
>> +		  if (tree ramp = DECL_RAMP_FN (current_function_decl))
>> +		    name = cxx_printable_name (ramp, 2);
>> +		  else
>> +		    name = cxx_printable_name (current_function_decl, 2);
>> +		}
>>     	      val = build_string_literal (name);
>>   	    }
>> diff --git a/gcc/testsuite/g++.dg/coroutines/pr110855.C b/gcc/testsuite/g++.dg/coroutines/pr110855.C
>> new file mode 100644
>> index 000000000000..6b5c0147ec83
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/coroutines/pr110855.C
>> @@ -0,0 +1,61 @@
>> +// { dg-do run }
>> +// { dg-output {^} }
>> +// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
>> +// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
>> +// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
>> +// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
>> +// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
>> +// { dg-output {$} }
>> +// https://gcc.gnu.org/PR110855
>> +#include <coroutine>
>> +#include <source_location>
>> +
>> +struct ReturnObject {
>> +  struct promise_type {
>> +    auto
>> +    initial_suspend(const std::source_location location =
>> +                    std::source_location::current()) {
>> +      __builtin_puts (location.function_name ());
>> +      return std::suspend_never{};
>> +    }
>> +    auto
>> +    final_suspend(const std::source_location location =
>> +                  std::source_location::current()) noexcept {
>> +      __builtin_puts (location.function_name ());
>> +      return std::suspend_never{};
>> +    }
>> +    auto
>> +    get_return_object(const std::source_location location =
>> +                      std::source_location::current()) {
>> +      __builtin_puts (location.function_name ());
>> +      return ReturnObject{std::coroutine_handle<promise_type>::from_promise(*this)};
>> +    }
>> +    auto
>> +    unhandled_exception() { }
>> +    auto return_void(const std::source_location location =
>> +                     std::source_location::current()) {
>> +      __builtin_puts (location.function_name ());
>> +    }
>> +  };
>> +  std::coroutine_handle<> handle;
>> +};
>> +
>> +struct awaitable : std::suspend_never
>> +{
>> +  void await_resume(const std::source_location location =
>> +                     std::source_location::current())
>> +  {
>> +      __builtin_puts (location.function_name ());
>> +  }
>> +};
>> +
>> +ReturnObject
>> +bar(int, char, bool) {
>> +  co_await awaitable{};
>> +  co_return;
>> +}
>> +
>> +int
>> +main() {
>> +  bar(1, 'a', false);
>> +}
diff mbox series

Patch

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index e6629dea5fdc..651751312fbe 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -3929,7 +3929,14 @@  fold_builtin_source_location (const_tree t)
 	      const char *name = "";
 
 	      if (current_function_decl)
-		name = cxx_printable_name (current_function_decl, 2);
+		{
+		  /* If this is a coroutine, we should get the name of the user
+		     function rather than the actor we generate.  */
+		  if (tree ramp = DECL_RAMP_FN (current_function_decl))
+		    name = cxx_printable_name (ramp, 2);
+		  else
+		    name = cxx_printable_name (current_function_decl, 2);
+		}
 
 	      val = build_string_literal (name);
 	    }
diff --git a/gcc/testsuite/g++.dg/coroutines/pr110855.C b/gcc/testsuite/g++.dg/coroutines/pr110855.C
new file mode 100644
index 000000000000..6b5c0147ec83
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr110855.C
@@ -0,0 +1,61 @@ 
+// { dg-do run }
+// { dg-output {^} }
+// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
+// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
+// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
+// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
+// { dg-output {ReturnObject bar\(int, char, bool\)(\n|\r\n|\r)} }
+// { dg-output {$} }
+// https://gcc.gnu.org/PR110855
+#include <coroutine>
+#include <source_location>
+
+struct ReturnObject {
+  struct promise_type {
+    auto
+    initial_suspend(const std::source_location location =
+                    std::source_location::current()) {
+      __builtin_puts (location.function_name ());
+      return std::suspend_never{};
+    }
+    auto
+    final_suspend(const std::source_location location =
+                  std::source_location::current()) noexcept {
+      __builtin_puts (location.function_name ());
+      return std::suspend_never{};
+    }
+    auto
+    get_return_object(const std::source_location location =
+                      std::source_location::current()) {
+      __builtin_puts (location.function_name ());
+      return ReturnObject{std::coroutine_handle<promise_type>::from_promise(*this)};
+    }
+    auto
+    unhandled_exception() { }
+    auto return_void(const std::source_location location =
+                     std::source_location::current()) {
+      __builtin_puts (location.function_name ());
+    }
+  };
+  std::coroutine_handle<> handle;
+};
+
+struct awaitable : std::suspend_never
+{
+  void await_resume(const std::source_location location =
+                     std::source_location::current())
+  {
+      __builtin_puts (location.function_name ());
+  }
+};
+
+ReturnObject
+bar(int, char, bool) {
+  co_await awaitable{};
+  co_return;
+}
+
+int
+main() {
+  bar(1, 'a', false);
+}