c++: Fix ICE locating 'this' for (not matching) template member function [PR115364]

We currently ICE when emitting the error message for this invalid code:

=== cut here ===
struct foo {
  template<int> void not_const() {}
};
void fn(const foo& obj) {
  obj.not_const<5>();
}
=== cut here ===

The problem is that get_fndecl_argument_location assumes that it has a
FUNCTION_DECL in its hands to find the location of the bad argument. It might
however have a TEMPLATE_DECL if there's a single candidate that cannot be
instantiated, like here.

This patch simply defaults to using the FNDECL's location in this case, which
fixes this PR.

Successfully tested on x86_64-pc-linux-gnu.

	PR c++/115364

gcc/cp/ChangeLog:

	* call.cc (get_fndecl_argument_location): Use FNDECL's location for
	TEMPLATE_DECLs.

gcc/testsuite/ChangeLog:

	* g++.dg/overload/template7.C: New test.

---
 gcc/cp/call.cc                            | 4 ++++
 gcc/testsuite/g++.dg/overload/template7.C | 9 +++++++++
 2 files changed, 13 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/overload/template7.C

On Fri, 28 Jun 2024, Simon Martin wrote:

> We currently ICE when emitting the error message for this invalid code:
> 
> === cut here ===
> struct foo {
>   template<int> void not_const() {}
> };
> void fn(const foo& obj) {
>   obj.not_const<5>();
> }
> === cut here ===
> 
> The problem is that get_fndecl_argument_location assumes that it has a
> FUNCTION_DECL in its hands to find the location of the bad argument. It might
> however have a TEMPLATE_DECL if there's a single candidate that cannot be
> instantiated, like here.
> 
> This patch simply defaults to using the FNDECL's location in this case, which
> fixes this PR.
> 
> Successfully tested on x86_64-pc-linux-gnu.
> 
> 	PR c++/115364
> 
> gcc/cp/ChangeLog:
> 
> 	* call.cc (get_fndecl_argument_location): Use FNDECL's location for
> 	TEMPLATE_DECLs.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/overload/template7.C: New test.
> 
> ---
>  gcc/cp/call.cc                            | 4 ++++
>  gcc/testsuite/g++.dg/overload/template7.C | 9 +++++++++
>  2 files changed, 13 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/overload/template7.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 7bbc1fb0c78..d5ff2311e63 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -8347,6 +8347,10 @@ get_fndecl_argument_location (tree fndecl, int argnum)
>    if (DECL_ARTIFICIAL (fndecl))
>      return DECL_SOURCE_LOCATION (fndecl);
>  
> +  /* Use FNDECL's location for TEMPLATE_DECLs.  */
> +  if (TREE_CODE (fndecl) == TEMPLATE_DECL)
> +    return DECL_SOURCE_LOCATION (fndecl);
> +

For TEMPLATE_DECL fndecl, it'd be more natural to return the
corresponding argument location of its DECL_TEMPLATE_RESULT (which
should be a FUNCTION_DECL).  The STRIP_TEMPLATE macro would be
convenient to use here.


It seems this doesn't fix the regression completely however because
in GCC 11 the code was rejected with a "permerror" (which can be
downgraded to a warning with -fpermissive):

  115364.C: In function ‘void fn(const foo&)’:
  115364.C:5:43: error: passing ‘const foo’ as ‘this’ argument discards qualifiers [-fpermissive]
      5 | void fn(const foo& obj) { obj.not_const<5>(); }
        |                           ~~~~~~~~~~~~~~~~^~
  115364.C:3:24: note:   in call to ‘void foo::not_const() [with int <anonymous> = 5]’
      3 |     template<int> void not_const() {}
        |                        ^~~~~~~~~

and we now reject with an ordinary error:

  115364.C: In function ‘void fn(const foo&)’:
  115364.C:5:27: error: cannot convert ‘const foo*’ to ‘foo*’
      5 | void fn(const foo& obj) { obj.not_const<5>(); }
        |                           ^~~
        |                           |
        |                           const foo*
  115364.C:3:24: note:   initializing argument 'this' of ‘template<int <anonymous> > void foo::not_const()’
      3 |     template<int> void not_const() {}
        |                        ^~~~~~~~~

To restore the error into a permerror, we need to figure out why we're
unexpectedly hitting this code path with a TEMPLATE_DECL, and why it's
necessary that the member function needs to take no arguments.  It turns
out I looked into this and submitted a patch for PR106760 (of which this
PR115364 is a dup) last year:
https://gcc.gnu.org/pipermail/gcc-patches/2023-June/620514.html

The patch was approved, but I lost track of it and never pushed it :/
I'm going to go ahead and push that fix shortly, sorry for not doing so
earlier.  Thanks for looking into this issue!

>    int i;
>    tree param;
>  
> diff --git a/gcc/testsuite/g++.dg/overload/template7.C b/gcc/testsuite/g++.dg/overload/template7.C
> new file mode 100644
> index 00000000000..67191c4ff62
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/overload/template7.C
> @@ -0,0 +1,9 @@
> +// PR c++/115364
> +// { dg-do compile }
> +
> +struct foo {
> +  template<int> void not_const() {} // { dg-note "initializing" }
> +};
> +void fn(const foo& obj) {
> +  obj.not_const<5>(); // { dg-error "cannot convert" }
> +}
> -- 
> 2.44.0
> 
> 
> 
>

On 29 Jun 2024, at 0:00, Patrick Palka wrote:

> On Fri, 28 Jun 2024, Simon Martin wrote:
>
>> We currently ICE when emitting the error message for this invalid 
>> code:
>>
>> === cut here ===
>> struct foo {
>>   template<int> void not_const() {}
>> };
>> void fn(const foo& obj) {
>>   obj.not_const<5>();
>> }
>> === cut here ===
>>
>> The problem is that get_fndecl_argument_location assumes that it has 
>> a
>> FUNCTION_DECL in its hands to find the location of the bad argument. 
>> It might
>> however have a TEMPLATE_DECL if there's a single candidate that 
>> cannot be
>> instantiated, like here.
>>
>> This patch simply defaults to using the FNDECL's location in this 
>> case, which
>> fixes this PR.
>>
>> Successfully tested on x86_64-pc-linux-gnu.
>>
>> 	PR c++/115364
>>
>> gcc/cp/ChangeLog:
>>
>> 	* call.cc (get_fndecl_argument_location): Use FNDECL's location for
>> 	TEMPLATE_DECLs.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* g++.dg/overload/template7.C: New test.
>>
>> ---
>>  gcc/cp/call.cc                            | 4 ++++
>>  gcc/testsuite/g++.dg/overload/template7.C | 9 +++++++++
>>  2 files changed, 13 insertions(+)
>>  create mode 100644 gcc/testsuite/g++.dg/overload/template7.C
>>
>> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
>> index 7bbc1fb0c78..d5ff2311e63 100644
>> --- a/gcc/cp/call.cc
>> +++ b/gcc/cp/call.cc
>> @@ -8347,6 +8347,10 @@ get_fndecl_argument_location (tree fndecl, int 
>> argnum)
>>    if (DECL_ARTIFICIAL (fndecl))
>>      return DECL_SOURCE_LOCATION (fndecl);
>>
>> +  /* Use FNDECL's location for TEMPLATE_DECLs.  */
>> +  if (TREE_CODE (fndecl) == TEMPLATE_DECL)
>> +    return DECL_SOURCE_LOCATION (fndecl);
>> +
>
> For TEMPLATE_DECL fndecl, it'd be more natural to return the
> corresponding argument location of its DECL_TEMPLATE_RESULT (which
> should be a FUNCTION_DECL).  The STRIP_TEMPLATE macro would be
> convenient to use here.
>
>
> It seems this doesn't fix the regression completely however because
> in GCC 11 the code was rejected with a "permerror" (which can be
> downgraded to a warning with -fpermissive):
>
>   115364.C: In function ‘void fn(const foo&)’:
>   115364.C:5:43: error: passing ‘const foo’ as ‘this’ argument 
> discards qualifiers [-fpermissive]
>       5 | void fn(const foo& obj) { obj.not_const<5>(); }
>         |                           ~~~~~~~~~~~~~~~~^~
>   115364.C:3:24: note:   in call to ‘void foo::not_const() [with int 
> <anonymous> = 5]’
>       3 |     template<int> void not_const() {}
>         |                        ^~~~~~~~~
>
> and we now reject with an ordinary error:
>
>   115364.C: In function ‘void fn(const foo&)’:
>   115364.C:5:27: error: cannot convert ‘const foo*’ to ‘foo*’
>       5 | void fn(const foo& obj) { obj.not_const<5>(); }
>         |                           ^~~
>         |                           |
>         |                           const foo*
>   115364.C:3:24: note:   initializing argument 'this' of 
> ‘template<int <anonymous> > void foo::not_const()’
>       3 |     template<int> void not_const() {}
>         |                        ^~~~~~~~~
>
> To restore the error into a permerror, we need to figure out why we're
> unexpectedly hitting this code path with a TEMPLATE_DECL, and why it's
> necessary that the member function needs to take no arguments.  It 
> turns
> out I looked into this and submitted a patch for PR106760 (of which 
> this
> PR115364 is a dup) last year:
> https://gcc.gnu.org/pipermail/gcc-patches/2023-June/620514.html
>
> The patch was approved, but I lost track of it and never pushed it :/
> I'm going to go ahead and push that fix shortly, sorry for not doing 
> so
> earlier.  Thanks for looking into this issue!
Sounds good, thanks!

>
>>    int i;
>>    tree param;
>>
>> diff --git a/gcc/testsuite/g++.dg/overload/template7.C 
>> b/gcc/testsuite/g++.dg/overload/template7.C
>> new file mode 100644
>> index 00000000000..67191c4ff62
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/overload/template7.C
>> @@ -0,0 +1,9 @@
>> +// PR c++/115364
>> +// { dg-do compile }
>> +
>> +struct foo {
>> +  template<int> void not_const() {} // { dg-note "initializing" }
>> +};
>> +void fn(const foo& obj) {
>> +  obj.not_const<5>(); // { dg-error "cannot convert" }
>> +}
>> -- 
>> 2.44.0
>>
>>
>>
>>