[v2] c++: Reject cdtors and conversion operators with a single * as return type [PR118306]

  Hi,

On 14 Jan 2025, at 23:31, Jason Merrill wrote:

> On 1/14/25 2:13 PM, Simon Martin wrote:
>> On 10 Jan 2025, at 19:10, Andrew Pinski wrote:
>>> On Fri, Jan 10, 2025 at 3:18 AM Simon Martin <simon@nasilyan.com>
>>> wrote:
>>>>
>>>> We currently accept the following invalid code (EDG and MSVC do as
>>>> well)
>>>
>>> clang does too: https://github.com/llvm/llvm-project/issues/121706 .

>>>
>>> Note it might be useful if a testcase with multiply `*` is included
>>
>>> too:
>>> ```
>>> struct A {
>>>    ****A ();
>>> };
>>> ```
>> Thanks, makes sense to add those. Done in the attached updated 
>> revision,
>> successfully tested on x86_64-pc-linux-gnu.
>
>> +/* Check that it's OK to declare a function at ID_LOC with the 
>> indicated TYPE,
>> +   TYPE_QUALS and DECLARATOR.  SFK indicates the kind of special 
>> function (if
>> +   any) that this function is.  OPTYPE is the type given in a 
>> conversion
>>     operator declaration, or the class type for a 
>> constructor/destructor.
>>     Returns the actual return type of the function; that may be 
>> different
>>     than TYPE if an error occurs, or for certain special functions.  
>> */
>> @@ -12361,8 +12362,19 @@ check_special_function_return_type 
>> (special_function_kind sfk,
>>  				    tree type,
>>  				    tree optype,
>>  				    int type_quals,
>> +				    const cp_declarator *declarator,
>> +				    location_t id_loc,
>
> id_loc should be the same as declarator->id_loc?
You’re right.

>>  				    const location_t* locations)
>>  {
>> +  /* If TYPE is unspecified, DECLARATOR, if set, should not 
>> represent a pointer
>> +     or a reference type.  */
>> +  if (type == NULL_TREE
>> +      && declarator
>> +      && (declarator->kind == cdk_pointer
>> +	  || declarator->kind == cdk_reference))
>> +    error_at (id_loc, "expected unqualified-id before %qs token",
>> +	      declarator->kind == cdk_pointer ? "*" : "&");
>
> ...and id_loc isn't the location of the ptr-operator, it's the 
> location of the identifier, so this indicates the wrong column.  I 
> think using declarator->id_loc makes sense, just not pretending it's 
> the location of the *.
Good catch, thanks.

> Let's give diagnostics more like the others later in the function 
> instead of trying to emulate cp_parser_error.
Makes sense. This is what the updated patch does, successfully tested on 
x86_64-pc-linux-gnu. OK for GCC 16?

Simon
From d60501bbbb3234f3229ddd3df0691426e00051ee Mon Sep 17 00:00:00 2001
From: Simon Martin <simon@nasilyan.com>
Date: Wed, 15 Jan 2025 09:54:37 +0100
Subject: [PATCH] c++: Reject cdtors and conversion operators with a single *
 as return type [PR118306]

We currently accept the following invalid code (clang, EDG and MSVC do
as well)

=== cut here ===
struct A {
  *A ();
};
=== cut here ===

The problem is that we end up in grokdeclarator with a cp_declarator of
kind cdk_pointer but no type, and we happily go through (if we have a
reference instead we eventually error out trying to form a reference to
void).

This patch makes sure that grokdeclarator errors out when processing a
constructor or a conversion operator with no return type specified but
also a declarator representing a pointer or a reference type.

Successfully tested on x86_64-pc-linux-gnu. OK for GCC 16?

	PR c++/118306

gcc/cp/ChangeLog:

	* decl.cc (indirect_ref_declarator_p): New.
	(check_special_function_return_type): Take declarator
	as input. Call indirect_ref_declarator_p and reject return type
	specifiers with only a * or &.
	(grokdeclarator): Update call to
	check_special_function_return_type.

gcc/testsuite/ChangeLog:

	* g++.dg/parse/constructor4.C: New test.
	* g++.dg/parse/conv_op2.C: New test.
	* g++.dg/parse/default_to_int.C: New test.

---
 gcc/cp/decl.cc                              | 34 +++++++++++--
 gcc/testsuite/g++.dg/parse/constructor4.C   | 54 +++++++++++++++++++++
 gcc/testsuite/g++.dg/parse/conv_op2.C       | 10 ++++
 gcc/testsuite/g++.dg/parse/default_to_int.C | 37 ++++++++++++++
 4 files changed, 130 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/parse/constructor4.C
 create mode 100644 gcc/testsuite/g++.dg/parse/conv_op2.C
 create mode 100644 gcc/testsuite/g++.dg/parse/default_to_int.C

-- 
2.44.0
  

On 1/15/25 7:24 AM, Simon Martin wrote:
> Hi,
> 
> On 14 Jan 2025, at 23:31, Jason Merrill wrote:
> 
>> On 1/14/25 2:13 PM, Simon Martin wrote:
>>> On 10 Jan 2025, at 19:10, Andrew Pinski wrote:
>>>> On Fri, Jan 10, 2025 at 3:18 AM Simon Martin <simon@nasilyan.com>
>>>> wrote:
>>>>>
>>>>> We currently accept the following invalid code (EDG and MSVC do as
>>>>> well)
>>>>
>>>> clang does too: https://github.com/llvm/llvm-project/issues/121706 .
> 
>>>>
>>>> Note it might be useful if a testcase with multiply `*` is included
>>>
>>>> too:
>>>> ```
>>>> struct A {
>>>>     ****A ();
>>>> };
>>>> ```
>>> Thanks, makes sense to add those. Done in the attached updated
>>> revision,
>>> successfully tested on x86_64-pc-linux-gnu.
>>
>>> +/* Check that it's OK to declare a function at ID_LOC with the
>>> indicated TYPE,
>>> +   TYPE_QUALS and DECLARATOR.  SFK indicates the kind of special
>>> function (if
>>> +   any) that this function is.  OPTYPE is the type given in a
>>> conversion
>>>      operator declaration, or the class type for a
>>> constructor/destructor.
>>>      Returns the actual return type of the function; that may be
>>> different
>>>      than TYPE if an error occurs, or for certain special functions.
>>> */
>>> @@ -12361,8 +12362,19 @@ check_special_function_return_type
>>> (special_function_kind sfk,
>>>   				    tree type,
>>>   				    tree optype,
>>>   				    int type_quals,
>>> +				    const cp_declarator *declarator,
>>> +				    location_t id_loc,
>>
>> id_loc should be the same as declarator->id_loc?
> You’re right.
> 
>>>   				    const location_t* locations)
>>>   {
>>> +  /* If TYPE is unspecified, DECLARATOR, if set, should not
>>> represent a pointer
>>> +     or a reference type.  */
>>> +  if (type == NULL_TREE
>>> +      && declarator
>>> +      && (declarator->kind == cdk_pointer
>>> +	  || declarator->kind == cdk_reference))
>>> +    error_at (id_loc, "expected unqualified-id before %qs token",
>>> +	      declarator->kind == cdk_pointer ? "*" : "&");
>>
>> ...and id_loc isn't the location of the ptr-operator, it's the
>> location of the identifier, so this indicates the wrong column.  I
>> think using declarator->id_loc makes sense, just not pretending it's
>> the location of the *.
> Good catch, thanks.
> 
>> Let's give diagnostics more like the others later in the function
>> instead of trying to emulate cp_parser_error.
> Makes sense. This is what the updated patch does, successfully tested on
> x86_64-pc-linux-gnu. OK for GCC 16?

OK.

Jason
  

On 1/15/25 9:56 AM, Jason Merrill wrote:
> On 1/15/25 7:24 AM, Simon Martin wrote:
>> Hi,
>>
>> On 14 Jan 2025, at 23:31, Jason Merrill wrote:
>>
>>> On 1/14/25 2:13 PM, Simon Martin wrote:
>>>> On 10 Jan 2025, at 19:10, Andrew Pinski wrote:
>>>>> On Fri, Jan 10, 2025 at 3:18 AM Simon Martin <simon@nasilyan.com>
>>>>> wrote:
>>>>>>
>>>>>> We currently accept the following invalid code (EDG and MSVC do as
>>>>>> well)
>>>>>
>>>>> clang does too: https://github.com/llvm/llvm-project/issues/121706 .
>>
>>>>>
>>>>> Note it might be useful if a testcase with multiply `*` is included
>>>>
>>>>> too:
>>>>> ```
>>>>> struct A {
>>>>>     ****A ();
>>>>> };
>>>>> ```
>>>> Thanks, makes sense to add those. Done in the attached updated
>>>> revision,
>>>> successfully tested on x86_64-pc-linux-gnu.
>>>
>>>> +/* Check that it's OK to declare a function at ID_LOC with the
>>>> indicated TYPE,
>>>> +   TYPE_QUALS and DECLARATOR.  SFK indicates the kind of special
>>>> function (if
>>>> +   any) that this function is.  OPTYPE is the type given in a
>>>> conversion
>>>>      operator declaration, or the class type for a
>>>> constructor/destructor.
>>>>      Returns the actual return type of the function; that may be
>>>> different
>>>>      than TYPE if an error occurs, or for certain special functions.
>>>> */
>>>> @@ -12361,8 +12362,19 @@ check_special_function_return_type
>>>> (special_function_kind sfk,
>>>>                       tree type,
>>>>                       tree optype,
>>>>                       int type_quals,
>>>> +                    const cp_declarator *declarator,
>>>> +                    location_t id_loc,
>>>
>>> id_loc should be the same as declarator->id_loc?
>> You’re right.
>>
>>>>                       const location_t* locations)
>>>>   {
>>>> +  /* If TYPE is unspecified, DECLARATOR, if set, should not
>>>> represent a pointer
>>>> +     or a reference type.  */
>>>> +  if (type == NULL_TREE
>>>> +      && declarator
>>>> +      && (declarator->kind == cdk_pointer
>>>> +      || declarator->kind == cdk_reference))
>>>> +    error_at (id_loc, "expected unqualified-id before %qs token",
>>>> +          declarator->kind == cdk_pointer ? "*" : "&");
>>>
>>> ...and id_loc isn't the location of the ptr-operator, it's the
>>> location of the identifier, so this indicates the wrong column.  I
>>> think using declarator->id_loc makes sense, just not pretending it's
>>> the location of the *.
>> Good catch, thanks.
>>
>>> Let's give diagnostics more like the others later in the function
>>> instead of trying to emulate cp_parser_error.
>> Makes sense. This is what the updated patch does, successfully tested on
>> x86_64-pc-linux-gnu. OK for GCC 16?
> 
> OK.

Does this also fix 118304?  If so, let's go ahead and apply it to GCC 15.

Jason
  

On Tue, Feb 04, 2025 at 10:39:44AM -0500, Jason Merrill wrote:
> Does this also fix 118304?  If so, let's go ahead and apply it to GCC 15.

It does not, with this patch we emit an error, but still crash on

  struct A {
    *A() = default;
  };

Marek
  

Hi Jason,

On 4 Feb 2025, at 16:39, Jason Merrill wrote:

> On 1/15/25 9:56 AM, Jason Merrill wrote:
>> On 1/15/25 7:24 AM, Simon Martin wrote:
>>> Hi,
>>>
>>> On 14 Jan 2025, at 23:31, Jason Merrill wrote:
>>>
>>>> On 1/14/25 2:13 PM, Simon Martin wrote:
>>>>> On 10 Jan 2025, at 19:10, Andrew Pinski wrote:
>>>>>> On Fri, Jan 10, 2025 at 3:18 AM Simon Martin 
>>>>>> <simon@nasilyan.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> We currently accept the following invalid code (EDG and MSVC do 
>>>>>>> as
>>>>>>> well)
>>>>>>
>>>>>> clang does too: 
>>>>>> https://github.com/llvm/llvm-project/issues/121706 .
>>>
>>>>>>
>>>>>> Note it might be useful if a testcase with multiply `*` is 
>>>>>> included
>>>>>
>>>>>> too:
>>>>>> ```
>>>>>> struct A {
>>>>>>     ****A ();
>>>>>> };
>>>>>> ```
>>>>> Thanks, makes sense to add those. Done in the attached updated
>>>>> revision,
>>>>> successfully tested on x86_64-pc-linux-gnu.
>>>>
>>>>> +/* Check that it's OK to declare a function at ID_LOC with the
>>>>> indicated TYPE,
>>>>> +   TYPE_QUALS and DECLARATOR.  SFK indicates the kind of 
>>>>> special
>>>>> function (if
>>>>> +   any) that this function is.  OPTYPE is the type given in a
>>>>> conversion
>>>>>      operator declaration, or the class type for a
>>>>> constructor/destructor.
>>>>>      Returns the actual return type of the function; that may 
>>>>> be
>>>>> different
>>>>>      than TYPE if an error occurs, or for certain special 
>>>>> functions.
>>>>> */
>>>>> @@ -12361,8 +12362,19 @@ check_special_function_return_type
>>>>> (special_function_kind sfk,
>>>>>                       tree type,
>>>>>                       tree optype,
>>>>>                       int type_quals,
>>>>> +                    const cp_declarator 
>>>>> *declarator,
>>>>> +                    location_t id_loc,
>>>>
>>>> id_loc should be the same as declarator->id_loc?
>>> You’re right.
>>>
>>>>>                       const location_t* 
>>>>> locations)
>>>>>   {
>>>>> +  /* If TYPE is unspecified, DECLARATOR, if set, should not
>>>>> represent a pointer
>>>>> +     or a reference type.  */
>>>>> +  if (type == NULL_TREE
>>>>> +      && declarator
>>>>> +      && (declarator->kind == cdk_pointer
>>>>> +      || declarator->kind == cdk_reference))
>>>>> +    error_at (id_loc, "expected unqualified-id before %qs 
>>>>> token",
>>>>> +          declarator->kind == cdk_pointer ? "*" : "&");
>>>>
>>>> ...and id_loc isn't the location of the ptr-operator, it's the
>>>> location of the identifier, so this indicates the wrong column.  I
>>>> think using declarator->id_loc makes sense, just not pretending 
>>>> it's
>>>> the location of the *.
>>> Good catch, thanks.
>>>
>>>> Let's give diagnostics more like the others later in the function
>>>> instead of trying to emulate cp_parser_error.
>>> Makes sense. This is what the updated patch does, successfully 
>>> tested on
>>> x86_64-pc-linux-gnu. OK for GCC 16?
>>
>> OK.
>
> Does this also fix 118304?  If so, let's go ahead and apply it to GCC 
> 15.
I have checked just now, and we still ICE for 118304’s testcase with 
that fix.

Simon
  

On 2/4/25 10:56 AM, Simon Martin wrote:
> Hi Jason,
> 
> On 4 Feb 2025, at 16:39, Jason Merrill wrote:
> 
>> On 1/15/25 9:56 AM, Jason Merrill wrote:
>>> On 1/15/25 7:24 AM, Simon Martin wrote:
>>>> Hi,
>>>>
>>>> On 14 Jan 2025, at 23:31, Jason Merrill wrote:
>>>>
>>>>> On 1/14/25 2:13 PM, Simon Martin wrote:
>>>>>> On 10 Jan 2025, at 19:10, Andrew Pinski wrote:
>>>>>>> On Fri, Jan 10, 2025 at 3:18 AM Simon Martin
>>>>>>> <simon@nasilyan.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> We currently accept the following invalid code (EDG and MSVC do
>>>>>>>> as
>>>>>>>> well)
>>>>>>>
>>>>>>> clang does too:
>>>>>>> https://github.com/llvm/llvm-project/issues/121706 .
>>>>
>>>>>>>
>>>>>>> Note it might be useful if a testcase with multiply `*` is
>>>>>>> included
>>>>>>
>>>>>>> too:
>>>>>>> ```
>>>>>>> struct A {
>>>>>>>      ****A ();
>>>>>>> };
>>>>>>> ```
>>>>>> Thanks, makes sense to add those. Done in the attached updated
>>>>>> revision,
>>>>>> successfully tested on x86_64-pc-linux-gnu.
>>>>>
>>>>>> +/* Check that it's OK to declare a function at ID_LOC with the
>>>>>> indicated TYPE,
>>>>>> +   TYPE_QUALS and DECLARATOR.  SFK indicates the kind of
>>>>>> special
>>>>>> function (if
>>>>>> +   any) that this function is.  OPTYPE is the type given in a
>>>>>> conversion
>>>>>>       operator declaration, or the class type for a
>>>>>> constructor/destructor.
>>>>>>       Returns the actual return type of the function; that may
>>>>>> be
>>>>>> different
>>>>>>       than TYPE if an error occurs, or for certain special
>>>>>> functions.
>>>>>> */
>>>>>> @@ -12361,8 +12362,19 @@ check_special_function_return_type
>>>>>> (special_function_kind sfk,
>>>>>>                        tree type,
>>>>>>                        tree optype,
>>>>>>                        int type_quals,
>>>>>> +                    const cp_declarator
>>>>>> *declarator,
>>>>>> +                    location_t id_loc,
>>>>>
>>>>> id_loc should be the same as declarator->id_loc?
>>>> You’re right.
>>>>
>>>>>>                        const location_t*
>>>>>> locations)
>>>>>>    {
>>>>>> +  /* If TYPE is unspecified, DECLARATOR, if set, should not
>>>>>> represent a pointer
>>>>>> +     or a reference type.  */
>>>>>> +  if (type == NULL_TREE
>>>>>> +      && declarator
>>>>>> +      && (declarator->kind == cdk_pointer
>>>>>> +      || declarator->kind == cdk_reference))
>>>>>> +    error_at (id_loc, "expected unqualified-id before %qs
>>>>>> token",
>>>>>> +          declarator->kind == cdk_pointer ? "*" : "&");
>>>>>
>>>>> ...and id_loc isn't the location of the ptr-operator, it's the
>>>>> location of the identifier, so this indicates the wrong column.  I
>>>>> think using declarator->id_loc makes sense, just not pretending
>>>>> it's
>>>>> the location of the *.
>>>> Good catch, thanks.
>>>>
>>>>> Let's give diagnostics more like the others later in the function
>>>>> instead of trying to emulate cp_parser_error.
>>>> Makes sense. This is what the updated patch does, successfully
>>>> tested on
>>>> x86_64-pc-linux-gnu. OK for GCC 16?
>>>
>>> OK.
>>
>> Does this also fix 118304?  If so, let's go ahead and apply it to GCC
>> 15.
> I have checked just now, and we still ICE for 118304’s testcase with
> that fix.

Why doesn't the preeexisting

type = void_type_node;

in check_special_function_return_type fix the return type and avoid the ICE?

Jason
  

On 4 Feb 2025, at 17:17, Jason Merrill wrote:

> On 2/4/25 10:56 AM, Simon Martin wrote:
>> Hi Jason,
>>
>> On 4 Feb 2025, at 16:39, Jason Merrill wrote:
>>
>>> On 1/15/25 9:56 AM, Jason Merrill wrote:
>>>> On 1/15/25 7:24 AM, Simon Martin wrote:
>>>>> Hi,
>>>>>
>>>>> On 14 Jan 2025, at 23:31, Jason Merrill wrote:
>>>>>
>>>>>> On 1/14/25 2:13 PM, Simon Martin wrote:
>>>>>>> On 10 Jan 2025, at 19:10, Andrew Pinski wrote:
>>>>>>>> On Fri, Jan 10, 2025 at 3:18 AM Simon Martin
>>>>>>>> <simon@nasilyan.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> We currently accept the following invalid code (EDG and MSVC 
>>>>>>>>> do
>>>>>>>>> as
>>>>>>>>> well)
>>>>>>>>
>>>>>>>> clang does too:
>>>>>>>> https://github.com/llvm/llvm-project/issues/121706 .
>>>>>
>>>>>>>>
>>>>>>>> Note it might be useful if a testcase with multiply `*` is
>>>>>>>> included
>>>>>>>
>>>>>>>> too:
>>>>>>>> ```
>>>>>>>> struct A {
>>>>>>>>      ****A ();
>>>>>>>> };
>>>>>>>> ```
>>>>>>> Thanks, makes sense to add those. Done in the attached updated
>>>>>>> revision,
>>>>>>> successfully tested on x86_64-pc-linux-gnu.
>>>>>>
>>>>>>> +/* Check that it's OK to declare a function at ID_LOC with the
>>>>>>> indicated TYPE,
>>>>>>> +   TYPE_QUALS and DECLARATOR.  SFK indicates the kind of
>>>>>>> special
>>>>>>> function (if
>>>>>>> +   any) that this function is.  OPTYPE is the type given in 
>>>>>>> a
>>>>>>> conversion
>>>>>>>       operator declaration, or the class type for a
>>>>>>> constructor/destructor.
>>>>>>>       Returns the actual return type of the function; that 
>>>>>>> may
>>>>>>> be
>>>>>>> different
>>>>>>>       than TYPE if an error occurs, or for certain special
>>>>>>> functions.
>>>>>>> */
>>>>>>> @@ -12361,8 +12362,19 @@ check_special_function_return_type
>>>>>>> (special_function_kind sfk,
>>>>>>>                        tree type,
>>>>>>>                        tree optype,
>>>>>>>                        int type_quals,
>>>>>>> +                    const cp_declarator
>>>>>>> *declarator,
>>>>>>> +                    location_t id_loc,
>>>>>>
>>>>>> id_loc should be the same as declarator->id_loc?
>>>>> You’re right.
>>>>>
>>>>>>>                        const location_t*
>>>>>>> locations)
>>>>>>>    {
>>>>>>> +  /* If TYPE is unspecified, DECLARATOR, if set, should not
>>>>>>> represent a pointer
>>>>>>> +     or a reference type.  */
>>>>>>> +  if (type == NULL_TREE
>>>>>>> +      && declarator
>>>>>>> +      && (declarator->kind == cdk_pointer
>>>>>>> +      || declarator->kind == cdk_reference))
>>>>>>> +    error_at (id_loc, "expected unqualified-id before %qs
>>>>>>> token",
>>>>>>> +          declarator->kind == cdk_pointer ? "*" : 
>>>>>>> "&");
>>>>>>
>>>>>> ...and id_loc isn't the location of the ptr-operator, it's the
>>>>>> location of the identifier, so this indicates the wrong column.  
>>>>>> I
>>>>>> think using declarator->id_loc makes sense, just not pretending
>>>>>> it's
>>>>>> the location of the *.
>>>>> Good catch, thanks.
>>>>>
>>>>>> Let's give diagnostics more like the others later in the function
>>>>>> instead of trying to emulate cp_parser_error.
>>>>> Makes sense. This is what the updated patch does, successfully
>>>>> tested on
>>>>> x86_64-pc-linux-gnu. OK for GCC 16?
>>>>
>>>> OK.
>>>
>>> Does this also fix 118304?  If so, let's go ahead and apply it to 
>>> GCC
>>> 15.
>> I have checked just now, and we still ICE for 118304’s testcase 
>> with
>> that fix.
>
> Why doesn't the preeexisting
>
> type = void_type_node;
>
> in check_special_function_return_type fix the return type and avoid 
> the ICE?
We hit the gcc_assert at method.cc:3593, that Marek’s fix bypasses. Do 
you want me to merge the fix for 118306 to trunk, so that Marek can then 
merge his fix without a gcc_assert (seen_error ()) instead of the 
current gcc_assert (true || seen_error ())?

Simon
  

On 2/4/25 11:45 AM, Simon Martin wrote:
> On 4 Feb 2025, at 17:17, Jason Merrill wrote:
> 
>> On 2/4/25 10:56 AM, Simon Martin wrote:
>>> Hi Jason,
>>>
>>> On 4 Feb 2025, at 16:39, Jason Merrill wrote:
>>>
>>>> On 1/15/25 9:56 AM, Jason Merrill wrote:
>>>>> On 1/15/25 7:24 AM, Simon Martin wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 14 Jan 2025, at 23:31, Jason Merrill wrote:
>>>>>>
>>>>>>> On 1/14/25 2:13 PM, Simon Martin wrote:
>>>>>>>> On 10 Jan 2025, at 19:10, Andrew Pinski wrote:
>>>>>>>>> On Fri, Jan 10, 2025 at 3:18 AM Simon Martin
>>>>>>>>> <simon@nasilyan.com>
>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> We currently accept the following invalid code (EDG and MSVC
>>>>>>>>>> do
>>>>>>>>>> as
>>>>>>>>>> well)
>>>>>>>>>
>>>>>>>>> clang does too:
>>>>>>>>> https://github.com/llvm/llvm-project/issues/121706 .
>>>>>>
>>>>>>>>>
>>>>>>>>> Note it might be useful if a testcase with multiply `*` is
>>>>>>>>> included
>>>>>>>>
>>>>>>>>> too:
>>>>>>>>> ```
>>>>>>>>> struct A {
>>>>>>>>>       ****A ();
>>>>>>>>> };
>>>>>>>>> ```
>>>>>>>> Thanks, makes sense to add those. Done in the attached updated
>>>>>>>> revision,
>>>>>>>> successfully tested on x86_64-pc-linux-gnu.
>>>>>>>
>>>>>>>> +/* Check that it's OK to declare a function at ID_LOC with the
>>>>>>>> indicated TYPE,
>>>>>>>> +   TYPE_QUALS and DECLARATOR.  SFK indicates the kind of
>>>>>>>> special
>>>>>>>> function (if
>>>>>>>> +   any) that this function is.  OPTYPE is the type given in
>>>>>>>> a
>>>>>>>> conversion
>>>>>>>>        operator declaration, or the class type for a
>>>>>>>> constructor/destructor.
>>>>>>>>        Returns the actual return type of the function; that
>>>>>>>> may
>>>>>>>> be
>>>>>>>> different
>>>>>>>>        than TYPE if an error occurs, or for certain special
>>>>>>>> functions.
>>>>>>>> */
>>>>>>>> @@ -12361,8 +12362,19 @@ check_special_function_return_type
>>>>>>>> (special_function_kind sfk,
>>>>>>>>                         tree type,
>>>>>>>>                         tree optype,
>>>>>>>>                         int type_quals,
>>>>>>>> +                    const cp_declarator
>>>>>>>> *declarator,
>>>>>>>> +                    location_t id_loc,
>>>>>>>
>>>>>>> id_loc should be the same as declarator->id_loc?
>>>>>> You’re right.
>>>>>>
>>>>>>>>                         const location_t*
>>>>>>>> locations)
>>>>>>>>     {
>>>>>>>> +  /* If TYPE is unspecified, DECLARATOR, if set, should not
>>>>>>>> represent a pointer
>>>>>>>> +     or a reference type.  */
>>>>>>>> +  if (type == NULL_TREE
>>>>>>>> +      && declarator
>>>>>>>> +      && (declarator->kind == cdk_pointer
>>>>>>>> +      || declarator->kind == cdk_reference))
>>>>>>>> +    error_at (id_loc, "expected unqualified-id before %qs
>>>>>>>> token",
>>>>>>>> +          declarator->kind == cdk_pointer ? "*" :
>>>>>>>> "&");
>>>>>>>
>>>>>>> ...and id_loc isn't the location of the ptr-operator, it's the
>>>>>>> location of the identifier, so this indicates the wrong column.
>>>>>>> I
>>>>>>> think using declarator->id_loc makes sense, just not pretending
>>>>>>> it's
>>>>>>> the location of the *.
>>>>>> Good catch, thanks.
>>>>>>
>>>>>>> Let's give diagnostics more like the others later in the function
>>>>>>> instead of trying to emulate cp_parser_error.
>>>>>> Makes sense. This is what the updated patch does, successfully
>>>>>> tested on
>>>>>> x86_64-pc-linux-gnu. OK for GCC 16?
>>>>>
>>>>> OK.
>>>>
>>>> Does this also fix 118304?  If so, let's go ahead and apply it to
>>>> GCC
>>>> 15.
>>> I have checked just now, and we still ICE for 118304’s testcase
>>> with
>>> that fix.
>>
>> Why doesn't the preeexisting
>>
>> type = void_type_node;
>>
>> in check_special_function_return_type fix the return type and avoid
>> the ICE?

> We hit the gcc_assert at method.cc:3593, that Marek’s fix bypasses.

Yes, but why doesn't check_special_function_return_type prevent that?

Jason
  

On 2/4/25 3:03 PM, Jason Merrill wrote:
> On 2/4/25 11:45 AM, Simon Martin wrote:
>> On 4 Feb 2025, at 17:17, Jason Merrill wrote:
>>
>>> On 2/4/25 10:56 AM, Simon Martin wrote:
>>>> Hi Jason,
>>>>
>>>> On 4 Feb 2025, at 16:39, Jason Merrill wrote:
>>>>
>>>>> On 1/15/25 9:56 AM, Jason Merrill wrote:
>>>>>> On 1/15/25 7:24 AM, Simon Martin wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 14 Jan 2025, at 23:31, Jason Merrill wrote:
>>>>>>>
>>>>>>>> On 1/14/25 2:13 PM, Simon Martin wrote:
>>>>>>>>> On 10 Jan 2025, at 19:10, Andrew Pinski wrote:
>>>>>>>>>> On Fri, Jan 10, 2025 at 3:18 AM Simon Martin
>>>>>>>>>> <simon@nasilyan.com>
>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> We currently accept the following invalid code (EDG and MSVC
>>>>>>>>>>> do
>>>>>>>>>>> as
>>>>>>>>>>> well)
>>>>>>>>>>
>>>>>>>>>> clang does too:
>>>>>>>>>> https://github.com/llvm/llvm-project/issues/121706 .
>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Note it might be useful if a testcase with multiply `*` is
>>>>>>>>>> included
>>>>>>>>>
>>>>>>>>>> too:
>>>>>>>>>> ```
>>>>>>>>>> struct A {
>>>>>>>>>>       ****A ();
>>>>>>>>>> };
>>>>>>>>>> ```
>>>>>>>>> Thanks, makes sense to add those. Done in the attached updated
>>>>>>>>> revision,
>>>>>>>>> successfully tested on x86_64-pc-linux-gnu.
>>>>>>>>
>>>>>>>>> +/* Check that it's OK to declare a function at ID_LOC with the
>>>>>>>>> indicated TYPE,
>>>>>>>>> +   TYPE_QUALS and DECLARATOR.  SFK indicates the kind of
>>>>>>>>> special
>>>>>>>>> function (if
>>>>>>>>> +   any) that this function is.  OPTYPE is the type given in
>>>>>>>>> a
>>>>>>>>> conversion
>>>>>>>>>        operator declaration, or the class type for a
>>>>>>>>> constructor/destructor.
>>>>>>>>>        Returns the actual return type of the function; that
>>>>>>>>> may
>>>>>>>>> be
>>>>>>>>> different
>>>>>>>>>        than TYPE if an error occurs, or for certain special
>>>>>>>>> functions.
>>>>>>>>> */
>>>>>>>>> @@ -12361,8 +12362,19 @@ check_special_function_return_type
>>>>>>>>> (special_function_kind sfk,
>>>>>>>>>                         tree type,
>>>>>>>>>                         tree optype,
>>>>>>>>>                         int type_quals,
>>>>>>>>> +                    const cp_declarator
>>>>>>>>> *declarator,
>>>>>>>>> +                    location_t id_loc,
>>>>>>>>
>>>>>>>> id_loc should be the same as declarator->id_loc?
>>>>>>> You’re right.
>>>>>>>
>>>>>>>>>                         const location_t*
>>>>>>>>> locations)
>>>>>>>>>     {
>>>>>>>>> +  /* If TYPE is unspecified, DECLARATOR, if set, should not
>>>>>>>>> represent a pointer
>>>>>>>>> +     or a reference type.  */
>>>>>>>>> +  if (type == NULL_TREE
>>>>>>>>> +      && declarator
>>>>>>>>> +      && (declarator->kind == cdk_pointer
>>>>>>>>> +      || declarator->kind == cdk_reference))
>>>>>>>>> +    error_at (id_loc, "expected unqualified-id before %qs
>>>>>>>>> token",
>>>>>>>>> +          declarator->kind == cdk_pointer ? "*" :
>>>>>>>>> "&");
>>>>>>>>
>>>>>>>> ...and id_loc isn't the location of the ptr-operator, it's the
>>>>>>>> location of the identifier, so this indicates the wrong column.
>>>>>>>> I
>>>>>>>> think using declarator->id_loc makes sense, just not pretending
>>>>>>>> it's
>>>>>>>> the location of the *.
>>>>>>> Good catch, thanks.
>>>>>>>
>>>>>>>> Let's give diagnostics more like the others later in the function
>>>>>>>> instead of trying to emulate cp_parser_error.
>>>>>>> Makes sense. This is what the updated patch does, successfully
>>>>>>> tested on
>>>>>>> x86_64-pc-linux-gnu. OK for GCC 16?
>>>>>>
>>>>>> OK.
>>>>>
>>>>> Does this also fix 118304?  If so, let's go ahead and apply it to
>>>>> GCC
>>>>> 15.
>>>> I have checked just now, and we still ICE for 118304’s testcase
>>>> with
>>>> that fix.
>>>
>>> Why doesn't the preeexisting
>>>
>>> type = void_type_node;
>>>
>>> in check_special_function_return_type fix the return type and avoid
>>> the ICE?
> 
>> We hit the gcc_assert at method.cc:3593, that Marek’s fix bypasses.
> 
> Yes, but why doesn't check_special_function_return_type prevent that?

Ah, because we call it before walking the declarator.  We need to check 
again later, perhaps in grokfndecl, that the type is correct.  Perhaps 
instead of your patch.

Jason
  

Hi Jason,

On 4 Feb 2025, at 21:23, Jason Merrill wrote:

> On 2/4/25 3:03 PM, Jason Merrill wrote:
>> On 2/4/25 11:45 AM, Simon Martin wrote:
>>> On 4 Feb 2025, at 17:17, Jason Merrill wrote:
>>>
>>>> On 2/4/25 10:56 AM, Simon Martin wrote:
>>>>> Hi Jason,
>>>>>
>>>>> On 4 Feb 2025, at 16:39, Jason Merrill wrote:
>>>>>
>>>>>> On 1/15/25 9:56 AM, Jason Merrill wrote:
>>>>>>> On 1/15/25 7:24 AM, Simon Martin wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 14 Jan 2025, at 23:31, Jason Merrill wrote:
>>>>>>>>
>>>>>>>>> On 1/14/25 2:13 PM, Simon Martin wrote:
>>>>>>>>>> On 10 Jan 2025, at 19:10, Andrew Pinski wrote:
>>>>>>>>>>> On Fri, Jan 10, 2025 at 3:18 AM Simon Martin
>>>>>>>>>>> <simon@nasilyan.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> We currently accept the following invalid code (EDG and 
>>>>>>>>>>>> MSVC
>>>>>>>>>>>> do
>>>>>>>>>>>> as
>>>>>>>>>>>> well)
>>>>>>>>>>>
>>>>>>>>>>> clang does too:
>>>>>>>>>>> https://github.com/llvm/llvm-project/issues/121706 .
>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Note it might be useful if a testcase with multiply `*` is
>>>>>>>>>>> included
>>>>>>>>>>
>>>>>>>>>>> too:
>>>>>>>>>>> ```
>>>>>>>>>>> struct A {
>>>>>>>>>>>       ****A ();
>>>>>>>>>>> };
>>>>>>>>>>> ```
>>>>>>>>>> Thanks, makes sense to add those. Done in the attached 
>>>>>>>>>> updated
>>>>>>>>>> revision,
>>>>>>>>>> successfully tested on x86_64-pc-linux-gnu.
>>>>>>>>>
>>>>>>>>>> +/* Check that it's OK to declare a function at ID_LOC with 
>>>>>>>>>> the
>>>>>>>>>> indicated TYPE,
>>>>>>>>>> +   TYPE_QUALS and DECLARATOR.  SFK indicates the kind of
>>>>>>>>>> special
>>>>>>>>>> function (if
>>>>>>>>>> +   any) that this function is.  OPTYPE is the type given 
>>>>>>>>>> in
>>>>>>>>>> a
>>>>>>>>>> conversion
>>>>>>>>>>        operator declaration, or the class type for a
>>>>>>>>>> constructor/destructor.
>>>>>>>>>>        Returns the actual return type of the function; 
>>>>>>>>>> that
>>>>>>>>>> may
>>>>>>>>>> be
>>>>>>>>>> different
>>>>>>>>>>        than TYPE if an error occurs, or for certain 
>>>>>>>>>> special
>>>>>>>>>> functions.
>>>>>>>>>> */
>>>>>>>>>> @@ -12361,8 +12362,19 @@ check_special_function_return_type
>>>>>>>>>> (special_function_kind sfk,
>>>>>>>>>>                         tree type,
>>>>>>>>>>                         tree optype,
>>>>>>>>>>                         int type_quals,
>>>>>>>>>> +                    const cp_declarator
>>>>>>>>>> *declarator,
>>>>>>>>>> +                    location_t id_loc,
>>>>>>>>>
>>>>>>>>> id_loc should be the same as declarator->id_loc?
>>>>>>>> You’re right.
>>>>>>>>
>>>>>>>>>>                         const 
>>>>>>>>>> location_t*
>>>>>>>>>> locations)
>>>>>>>>>>     {
>>>>>>>>>> +  /* If TYPE is unspecified, DECLARATOR, if set, should not
>>>>>>>>>> represent a pointer
>>>>>>>>>> +     or a reference type.  */
>>>>>>>>>> +  if (type == NULL_TREE
>>>>>>>>>> +      && declarator
>>>>>>>>>> +      && (declarator->kind == cdk_pointer
>>>>>>>>>> +      || declarator->kind == cdk_reference))
>>>>>>>>>> +    error_at (id_loc, "expected unqualified-id before %qs
>>>>>>>>>> token",
>>>>>>>>>> +          declarator->kind == cdk_pointer ? "*" :
>>>>>>>>>> "&");
>>>>>>>>>
>>>>>>>>> ...and id_loc isn't the location of the ptr-operator, it's the

>>>>>>>>> location of the identifier, so this indicates the wrong 
>>>>>>>>> column.
>>>>>>>>> I
>>>>>>>>> think using declarator->id_loc makes sense, just not 
>>>>>>>>> pretending
>>>>>>>>> it's
>>>>>>>>> the location of the *.
>>>>>>>> Good catch, thanks.
>>>>>>>>
>>>>>>>>> Let's give diagnostics more like the others later in the 
>>>>>>>>> function
>>>>>>>>> instead of trying to emulate cp_parser_error.
>>>>>>>> Makes sense. This is what the updated patch does, successfully
>>>>>>>> tested on
>>>>>>>> x86_64-pc-linux-gnu. OK for GCC 16?
>>>>>>>
>>>>>>> OK.
>>>>>>
>>>>>> Does this also fix 118304?  If so, let's go ahead and apply it 
>>>>>> to
>>>>>> GCC
>>>>>> 15.
>>>>> I have checked just now, and we still ICE for 118304’s testcase
>>>>> with
>>>>> that fix.
>>>>
>>>> Why doesn't the preeexisting
>>>>
>>>> type = void_type_node;
>>>>
>>>> in check_special_function_return_type fix the return type and avoid

>>>> the ICE?
>>
>>> We hit the gcc_assert at method.cc:3593, that Marek’s fix 

>>> bypasses.
>>
>> Yes, but why doesn't check_special_function_return_type prevent that?
>
> Ah, because we call it before walking the declarator.  We need to 
> check again later, perhaps in grokfndecl, that the type is correct.  
> Perhaps instead of your patch.
One “issue” with adding another check in or close to grokfndecl is 
that DECLARATOR will have “been moved to the ID”, and the fact that 
we had a CDK_POINTER kind is “lost”. We could obviously somehow 
propagate this information, but there might be something easier.

It actually feels like having check_special_function_return_type return 
error_mark_node in case of error makes sense. If I amend my patch to do 
so (see attachment), then we fix both PR108306 and PR108304, and full 

testing is successful on x86_64-pc-linux-gnu (modulo tweaking a couple 
of tests that were expecting what we can consider spurious error 
messages).

Am I missing anything, or should we go ahead with this approach and 
merge this updated patch to GCC15?

Simon
From 192fab4034a4f457b8a9e93016e6b68a30671c6c Mon Sep 17 00:00:00 2001
From: Simon Martin <simon@nasilyan.com>
Date: Wed, 15 Jan 2025 09:54:37 +0100
Subject: [PATCH] c++: Reject cdtors and conversion operators with a single *
 as return type [PR118304, PR118306]

We currently accept the following constructor declaration (clang, EDG
and MSVC do as well), and ICE on the destructor declaration

=== cut here ===
struct A {
  *A ();
  ~A () = default;
};
=== cut here ===

The problem is that we end up in grokdeclarator with a cp_declarator of
kind cdk_pointer but no type, and we happily go through (if we have a
reference instead we eventually error out trying to form a reference to
void).

This patch makes sure that grokdeclarator errors out and returns
error_mark_node when processing a cdtor or a conversion operator with no
return type specified but also a declarator representing a pointer or a
reference type.

Successfully tested on x86_64-pc-linux-gnu.

	PR c++/118306
	PR c++/118304

gcc/cp/ChangeLog:

	* decl.cc (indirect_ref_declarator_p): New.
	(check_special_function_return_type): Take declarator
	as input. Call indirect_ref_declarator_p and reject return type
	specifiers with only a * or &. Return error_mark_node in case of
	error.
	(grokdeclarator): Update call to
	check_special_function_return_type.

gcc/testsuite/ChangeLog:

	* g++.dg/parse/friend7.C: Adjust test expectation.
	* g++.old-deja/g++.brendan/crash16.C: Ditto.
	* g++.old-deja/g++.jason/operator.C: Ditto.
	* g++.old-deja/g++.law/ctors5.C: Ditto.
	* g++.dg/parse/constructor4.C: New test.
	* g++.dg/parse/constructor5.C: New test.
	* g++.dg/parse/conv_op2.C: New test.
	* g++.dg/parse/default_to_int.C: New test.

---
 gcc/cp/decl.cc                                | 71 ++++++++++++++-----
 gcc/testsuite/g++.dg/parse/constructor4.C     | 54 ++++++++++++++
 gcc/testsuite/g++.dg/parse/constructor5.C     | 14 ++++
 gcc/testsuite/g++.dg/parse/conv_op2.C         | 10 +++
 gcc/testsuite/g++.dg/parse/default_to_int.C   | 37 ++++++++++
 gcc/testsuite/g++.dg/parse/friend7.C          |  2 -
 .../g++.old-deja/g++.brendan/crash16.C        |  5 +-
 .../g++.old-deja/g++.jason/operator.C         |  1 -
 gcc/testsuite/g++.old-deja/g++.law/ctors5.C   |  1 -
 9 files changed, 171 insertions(+), 24 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/parse/constructor4.C
 create mode 100644 gcc/testsuite/g++.dg/parse/constructor5.C
 create mode 100644 gcc/testsuite/g++.dg/parse/conv_op2.C
 create mode 100644 gcc/testsuite/g++.dg/parse/default_to_int.C

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index b7af33b3231..9db1bd1cf42 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -101,7 +101,8 @@ static void end_cleanup_fn (void);
 static tree cp_make_fname_decl (location_t, tree, int);
 static void initialize_predefined_identifiers (void);
 static tree check_special_function_return_type
-       (special_function_kind, tree, tree, int, const location_t*);
+       (special_function_kind, tree, tree, int, const cp_declarator*,
+	const location_t*);
 static tree push_cp_library_fn (enum tree_code, tree, int);
 static tree build_cp_library_fn (tree, enum tree_code, tree, int);
 static void store_parm_decls (tree);
@@ -12427,10 +12428,19 @@ smallest_type_location (const cp_decl_specifier_seq *declspecs)
   return smallest_type_location (type_quals, declspecs->locations);
 }
 
-/* Check that it's OK to declare a function with the indicated TYPE
-   and TYPE_QUALS.  SFK indicates the kind of special function (if any)
-   that this function is.  OPTYPE is the type given in a conversion
-   operator declaration, or the class type for a constructor/destructor.
+/* Returns whether DECLARATOR represents a pointer or a reference.  */
+
+static bool
+indirect_ref_declarator_p (const cp_declarator* declarator)
+{
+  return declarator && (declarator->kind == cdk_pointer
+			|| declarator->kind == cdk_reference);
+}
+
+/* Check that it's OK to declare a function with the indicated TYPE, TYPE_QUALS
+   and DECLARATOR.  SFK indicates the kind of special function (if any) that
+   this function is.  OPTYPE is the type given in a conversion operator
+   declaration, or the class type for a constructor/destructor.
    Returns the actual return type of the function; that may be different
    than TYPE if an error occurs, or for certain special functions.  */
 
@@ -12439,16 +12449,32 @@ check_special_function_return_type (special_function_kind sfk,
 				    tree type,
 				    tree optype,
 				    int type_quals,
+				    const cp_declarator* declarator,
 				    const location_t* locations)
 {
+#define ERROR_AT(...)	      \
+    do {		      \
+      has_error = true;	      \
+      error_at (__VA_ARGS__); \
+    } while (0)
+#define ERROR(...)	    \
+    do {		    \
+      has_error = true;     \
+      error (__VA_ARGS__);  \
+    } while (0)
+
+  bool has_error = false;
   switch (sfk)
     {
     case sfk_constructor:
       if (type)
-	error_at (smallest_type_location (type_quals, locations),
+	ERROR_AT (smallest_type_location (type_quals, locations),
+		  "return type specification for constructor invalid");
+      else if (indirect_ref_declarator_p (declarator))
+	ERROR_AT (declarator->id_loc,
 		  "return type specification for constructor invalid");
       else if (type_quals != TYPE_UNQUALIFIED)
-	error_at (smallest_type_quals_location (type_quals, locations),
+	ERROR_AT (smallest_type_quals_location (type_quals, locations),
 		  "qualifiers are not allowed on constructor declaration");
 
       if (targetm.cxx.cdtor_returns_this ())
@@ -12459,10 +12485,13 @@ check_special_function_return_type (special_function_kind sfk,
 
     case sfk_destructor:
       if (type)
-	error_at (smallest_type_location (type_quals, locations),
+	ERROR_AT (smallest_type_location (type_quals, locations),
+		  "return type specification for destructor invalid");
+      else if (indirect_ref_declarator_p (declarator))
+	ERROR_AT (declarator->id_loc,
 		  "return type specification for destructor invalid");
       else if (type_quals != TYPE_UNQUALIFIED)
-	error_at (smallest_type_quals_location (type_quals, locations),
+	ERROR_AT (smallest_type_quals_location (type_quals, locations),
 		  "qualifiers are not allowed on destructor declaration");
 
       /* We can't use the proper return type here because we run into
@@ -12475,10 +12504,13 @@ check_special_function_return_type (special_function_kind sfk,
 
     case sfk_conversion:
       if (type)
-	error_at (smallest_type_location (type_quals, locations),
+	ERROR_AT (smallest_type_location (type_quals, locations),
+		  "return type specified for %<operator %T%>", optype);
+      else if (indirect_ref_declarator_p (declarator))
+	ERROR_AT (declarator->id_loc,
 		  "return type specified for %<operator %T%>", optype);
       else if (type_quals != TYPE_UNQUALIFIED)
-	error_at (smallest_type_quals_location (type_quals, locations),
+	ERROR_AT (smallest_type_quals_location (type_quals, locations),
 		  "qualifiers are not allowed on declaration of "
 		  "%<operator %T%>", optype);
 
@@ -12487,31 +12519,35 @@ check_special_function_return_type (special_function_kind sfk,
 
     case sfk_deduction_guide:
       if (type)
-	error_at (smallest_type_location (type_quals, locations),
+	ERROR_AT (smallest_type_location (type_quals, locations),
+		  "return type specified for deduction guide");
+      else if (indirect_ref_declarator_p (declarator))
+	ERROR_AT (declarator->id_loc,
 		  "return type specified for deduction guide");
       else if (type_quals != TYPE_UNQUALIFIED)
-	error_at (smallest_type_quals_location (type_quals, locations),
+	ERROR_AT (smallest_type_quals_location (type_quals, locations),
 		  "qualifiers are not allowed on declaration of "
 		  "deduction guide");
       if (TREE_CODE (optype) == TEMPLATE_TEMPLATE_PARM)
 	{
-	  error ("template template parameter %qT in declaration of "
+	  ERROR ("template template parameter %qT in declaration of "
 		 "deduction guide", optype);
-	  type = error_mark_node;
 	}
       else
 	type = make_template_placeholder (CLASSTYPE_TI_TEMPLATE (optype));
       for (int i = 0; i < ds_last; ++i)
 	if (i != ds_explicit && locations[i])
-	  error_at (locations[i],
+	  ERROR_AT (locations[i],
 		    "%<decl-specifier%> in declaration of deduction guide");
       break;
 
     default:
       gcc_unreachable ();
     }
+#undef ERROR_AT
+#undef ERROR
 
-  return type;
+  return !has_error ? type : error_mark_node;
 }
 
 /* A variable or data member (whose unqualified name is IDENTIFIER)
@@ -13167,6 +13203,7 @@ grokdeclarator (const cp_declarator *declarator,
       type = check_special_function_return_type (sfk, type,
 						 ctor_return_type,
 						 type_quals,
+						 declarator,
 						 declspecs->locations);
       type_quals = TYPE_UNQUALIFIED;
     }
diff --git a/gcc/testsuite/g++.dg/parse/constructor4.C b/gcc/testsuite/g++.dg/parse/constructor4.C
new file mode 100644
index 00000000000..f7e4cace451
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/constructor4.C
@@ -0,0 +1,54 @@
+// PR c++/118306
+// { dg-do "compile" }
+
+// Constructors.
+struct A {
+  *A ();	    // { dg-error "return type specification" }
+};
+struct B {
+  **B ();	    // { dg-error "return type specification" }
+};
+struct C {
+  ***C ();	    // { dg-error "return type specification" }
+};
+struct D {
+  &D ();	    // { dg-error "return type specification|reference to" }
+};
+struct E {
+  *&E ();	    // { dg-error "return type specification|reference to" }
+};
+struct F {
+  **&F ();	    // { dg-error "return type specification|reference to" }
+};
+struct G {
+  *G (const G&);    // { dg-error "return type specification" }
+};
+struct H {
+  **H (const H&);    // { dg-error "return type specification" }
+};
+struct I {
+  &I (const I&);    // { dg-error "return type specification|reference to" }
+};
+struct J {
+  const J();	    // { dg-error "expected unqualified-id" }
+};
+
+// Destructors.
+struct K {
+  * ~K ();	    // { dg-error "return type specification" }
+};
+struct L {
+  ** ~L ();	    // { dg-error "return type specification" }
+};
+struct M {
+  & ~M ();	    // { dg-error "return type specification|reference to" }
+};
+struct N {
+  virtual * ~N ();  // { dg-error "return type specification" }
+};
+struct O {
+  virtual & ~O ();  // { dg-error "return type specification|reference to" }
+};
+struct P {
+  volatile ~P();    // { dg-error "qualifiers are not allowed" }
+};
diff --git a/gcc/testsuite/g++.dg/parse/constructor5.C b/gcc/testsuite/g++.dg/parse/constructor5.C
new file mode 100644
index 00000000000..34de286fb67
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/constructor5.C
@@ -0,0 +1,14 @@
+// PR c++/118304
+// { dg-do "compile" { target c++11 } }
+
+struct A {
+  *A() = default; // { dg-error "return type specification" }
+};
+
+struct B {
+  &B (const B&) = default; // { dg-error "return type specification|reference to" }
+};
+
+struct C {
+  & ~C () = default; // { dg-error "return type specification|reference to" }
+};
diff --git a/gcc/testsuite/g++.dg/parse/conv_op2.C b/gcc/testsuite/g++.dg/parse/conv_op2.C
new file mode 100644
index 00000000000..f2719135e00
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/conv_op2.C
@@ -0,0 +1,10 @@
+// PR c++/118306
+// { dg-do "compile" }
+
+struct K {
+  char operator int();	// { dg-error "return type specified for" }
+  * operator short();	// { dg-error "return type specified for" }
+  ** operator float();	// { dg-error "return type specified for" }
+  &* operator double();	// { dg-error "return type specified for|pointer to 'double&'" }
+  & operator long();	// { dg-error "return type specified for" }
+};
diff --git a/gcc/testsuite/g++.dg/parse/default_to_int.C b/gcc/testsuite/g++.dg/parse/default_to_int.C
new file mode 100644
index 00000000000..681298ce634
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/default_to_int.C
@@ -0,0 +1,37 @@
+// PR c++/118306 - "Document" various behaviours wrt. defaulting types to int.
+// { dg-do "compile" }
+// { dg-additional-options "-fpermissive" }
+
+// Members.
+struct K {
+  * mem1;	    // { dg-warning "forbids declaration" }
+  * mem2;	    // { dg-warning "forbids declaration" }
+  const * mem3;	    // { dg-warning "forbids declaration" }
+  const ** mem4;    // { dg-warning "forbids declaration" }
+  & mem5;	    // { dg-warning "forbids declaration" }
+  volatile & mem6;  // { dg-warning "forbids declaration" }
+
+  void foo (const& permissive_fine,		// { dg-warning "forbids declaration" }
+	    volatile* permissive_fine_as_well); // { dg-warning "forbids declaration" }
+
+  * bar () { return 0; }  // { dg-warning "forbids declaration" }
+  const& baz ();	  // { dg-warning "forbids declaration" }
+
+  void bazz () {
+    try {}
+    catch (const *i) {}	// { dg-warning "forbids" }
+    catch (const &i) {}	// { dg-warning "forbids" }
+  }
+};
+
+// Template parameters.
+template<const *i, const &j>  // { dg-warning "forbids" }
+void baz() {}
+
+// Functions.
+foo(int) { return 42; }		    // { dg-warning "forbids declaration" }
+*bar(int) { return 0; }		    // { dg-warning "forbids declaration" }
+**bazz(int) { return 0; }	    // { dg-warning "forbids declaration" }
+*&bazzz(int) { return 0; }	    // { dg-warning "forbids declaration|bind non-const" }
+const bazzzz (int) { return 0; }    // { dg-warning "forbids declaration" }
+const* bazzzzz (int) { return 0; }  // { dg-warning "forbids declaration" }
diff --git a/gcc/testsuite/g++.dg/parse/friend7.C b/gcc/testsuite/g++.dg/parse/friend7.C
index 19e31795c24..d78c1cf94cc 100644
--- a/gcc/testsuite/g++.dg/parse/friend7.C
+++ b/gcc/testsuite/g++.dg/parse/friend7.C
@@ -20,7 +20,6 @@ struct C
 {
   friend int C ();
   friend int ~C ();		// { dg-error "10:return type" }
-  // { dg-error "14:expected qualified name in friend decl" "" { target *-*-* } .-1 }
   friend int C (const C &);
 };
 
@@ -28,7 +27,6 @@ struct D
 {
   friend int D () {}
   friend int ~D () {}		// { dg-error "10:return type" }
-  // { dg-error "14:expected qualified name in friend decl" "" { target *-*-* } .-1 }
   friend int D (const D &) {}
 };
 
diff --git a/gcc/testsuite/g++.old-deja/g++.brendan/crash16.C b/gcc/testsuite/g++.old-deja/g++.brendan/crash16.C
index 2edb2a8711b..97d810f14dd 100644
--- a/gcc/testsuite/g++.old-deja/g++.brendan/crash16.C
+++ b/gcc/testsuite/g++.old-deja/g++.brendan/crash16.C
@@ -6,10 +6,9 @@ class Graph { // { dg-error "1:new types|1: note: \\(perhaps" }
 // { dg-error "1:return type" "" { target *-*-* } .-1 }
 public:
       unsigned         char N;
-      Graph(void) {} // { dg-message "7:'Graph" }
+      Graph(void) {}
 }
 
-Graph::Graph(void) // { dg-error "1:redefinition" }
+Graph::Graph(void)
 {    N = 10;
 }
-
diff --git a/gcc/testsuite/g++.old-deja/g++.jason/operator.C b/gcc/testsuite/g++.old-deja/g++.jason/operator.C
index c18790190b5..ad02f262d6e 100644
--- a/gcc/testsuite/g++.old-deja/g++.jason/operator.C
+++ b/gcc/testsuite/g++.old-deja/g++.jason/operator.C
@@ -29,4 +29,3 @@ void * operator new (A a);	// { dg-error ".operator new. takes type .size_t." }
 void operator delete (A a);	// { dg-error ".operator delete. takes type .void\\*. as first parameter" }
 
 char * operator char * (int);	// { dg-error "return type" "ret" }
-// { dg-error "8:.operator char\\*\\*\\(int\\). must be a non-static member function" "mem" { target *-*-* } .-1 }
diff --git a/gcc/testsuite/g++.old-deja/g++.law/ctors5.C b/gcc/testsuite/g++.old-deja/g++.law/ctors5.C
index 492c429ab17..d892414ce7c 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/ctors5.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/ctors5.C
@@ -24,7 +24,6 @@ class Y // { dg-error "1:new types may not be defined in a return type" "err" }
     Y();
 }
 X::X( int xi )
-// { dg-message "1:X::X|candidate expects" "match candidate text" { target *-*-* } .-1 }
 {
     x = xi;
 }
-- 
2.44.0
  

On 2/5/25 2:21 PM, Simon Martin wrote:
> Hi Jason,
> 
> On 4 Feb 2025, at 21:23, Jason Merrill wrote:
> 
>> On 2/4/25 3:03 PM, Jason Merrill wrote:
>>> On 2/4/25 11:45 AM, Simon Martin wrote:
>>>> On 4 Feb 2025, at 17:17, Jason Merrill wrote:
>>>>
>>>>> On 2/4/25 10:56 AM, Simon Martin wrote:
>>>>>> Hi Jason,
>>>>>>
>>>>>> On 4 Feb 2025, at 16:39, Jason Merrill wrote:
>>>>>>
>>>>>>> On 1/15/25 9:56 AM, Jason Merrill wrote:
>>>>>>>> On 1/15/25 7:24 AM, Simon Martin wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> On 14 Jan 2025, at 23:31, Jason Merrill wrote:
>>>>>>>>>
>>>>>>>>>> On 1/14/25 2:13 PM, Simon Martin wrote:
>>>>>>>>>>> On 10 Jan 2025, at 19:10, Andrew Pinski wrote:
>>>>>>>>>>>> On Fri, Jan 10, 2025 at 3:18 AM Simon Martin
>>>>>>>>>>>> <simon@nasilyan.com>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> We currently accept the following invalid code (EDG and
>>>>>>>>>>>>> MSVC
>>>>>>>>>>>>> do
>>>>>>>>>>>>> as
>>>>>>>>>>>>> well)
>>>>>>>>>>>>
>>>>>>>>>>>> clang does too:
>>>>>>>>>>>> https://github.com/llvm/llvm-project/issues/121706 .
>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Note it might be useful if a testcase with multiply `*` is
>>>>>>>>>>>> included
>>>>>>>>>>>
>>>>>>>>>>>> too:
>>>>>>>>>>>> ```
>>>>>>>>>>>> struct A {
>>>>>>>>>>>>        ****A ();
>>>>>>>>>>>> };
>>>>>>>>>>>> ```
>>>>>>>>>>> Thanks, makes sense to add those. Done in the attached
>>>>>>>>>>> updated
>>>>>>>>>>> revision,
>>>>>>>>>>> successfully tested on x86_64-pc-linux-gnu.
>>>>>>>>>>
>>>>>>>>>>> +/* Check that it's OK to declare a function at ID_LOC with
>>>>>>>>>>> the
>>>>>>>>>>> indicated TYPE,
>>>>>>>>>>> +   TYPE_QUALS and DECLARATOR.  SFK indicates the kind of
>>>>>>>>>>> special
>>>>>>>>>>> function (if
>>>>>>>>>>> +   any) that this function is.  OPTYPE is the type given
>>>>>>>>>>> in
>>>>>>>>>>> a
>>>>>>>>>>> conversion
>>>>>>>>>>>         operator declaration, or the class type for a
>>>>>>>>>>> constructor/destructor.
>>>>>>>>>>>         Returns the actual return type of the function;
>>>>>>>>>>> that
>>>>>>>>>>> may
>>>>>>>>>>> be
>>>>>>>>>>> different
>>>>>>>>>>>         than TYPE if an error occurs, or for certain
>>>>>>>>>>> special
>>>>>>>>>>> functions.
>>>>>>>>>>> */
>>>>>>>>>>> @@ -12361,8 +12362,19 @@ check_special_function_return_type
>>>>>>>>>>> (special_function_kind sfk,
>>>>>>>>>>>                          tree type,
>>>>>>>>>>>                          tree optype,
>>>>>>>>>>>                          int type_quals,
>>>>>>>>>>> +                    const cp_declarator
>>>>>>>>>>> *declarator,
>>>>>>>>>>> +                    location_t id_loc,
>>>>>>>>>>
>>>>>>>>>> id_loc should be the same as declarator->id_loc?
>>>>>>>>> You’re right.
>>>>>>>>>
>>>>>>>>>>>                          const
>>>>>>>>>>> location_t*
>>>>>>>>>>> locations)
>>>>>>>>>>>      {
>>>>>>>>>>> +  /* If TYPE is unspecified, DECLARATOR, if set, should not
>>>>>>>>>>> represent a pointer
>>>>>>>>>>> +     or a reference type.  */
>>>>>>>>>>> +  if (type == NULL_TREE
>>>>>>>>>>> +      && declarator
>>>>>>>>>>> +      && (declarator->kind == cdk_pointer
>>>>>>>>>>> +      || declarator->kind == cdk_reference))
>>>>>>>>>>> +    error_at (id_loc, "expected unqualified-id before %qs
>>>>>>>>>>> token",
>>>>>>>>>>> +          declarator->kind == cdk_pointer ? "*" :
>>>>>>>>>>> "&");
>>>>>>>>>>
>>>>>>>>>> ...and id_loc isn't the location of the ptr-operator, it's the
> 
>>>>>>>>>> location of the identifier, so this indicates the wrong
>>>>>>>>>> column.
>>>>>>>>>> I
>>>>>>>>>> think using declarator->id_loc makes sense, just not
>>>>>>>>>> pretending
>>>>>>>>>> it's
>>>>>>>>>> the location of the *.
>>>>>>>>> Good catch, thanks.
>>>>>>>>>
>>>>>>>>>> Let's give diagnostics more like the others later in the
>>>>>>>>>> function
>>>>>>>>>> instead of trying to emulate cp_parser_error.
>>>>>>>>> Makes sense. This is what the updated patch does, successfully
>>>>>>>>> tested on
>>>>>>>>> x86_64-pc-linux-gnu. OK for GCC 16?
>>>>>>>>
>>>>>>>> OK.
>>>>>>>
>>>>>>> Does this also fix 118304?  If so, let's go ahead and apply it
>>>>>>> to
>>>>>>> GCC
>>>>>>> 15.
>>>>>> I have checked just now, and we still ICE for 118304’s testcase
>>>>>> with
>>>>>> that fix.
>>>>>
>>>>> Why doesn't the preeexisting
>>>>>
>>>>> type = void_type_node;
>>>>>
>>>>> in check_special_function_return_type fix the return type and avoid
> 
>>>>> the ICE?
>>>
>>>> We hit the gcc_assert at method.cc:3593, that Marek’s fix
> 
>>>> bypasses.
>>>
>>> Yes, but why doesn't check_special_function_return_type prevent that?
>>
>> Ah, because we call it before walking the declarator.  We need to
>> check again later, perhaps in grokfndecl, that the type is correct.
>> Perhaps instead of your patch.
> One “issue” with adding another check in or close to grokfndecl is
> that DECLARATOR will have “been moved to the ID”, and the fact that
> we had a CDK_POINTER kind is “lost”. We could obviously somehow
> propagate this information, but there might be something easier.

The information isn't lost: it's now reflected in the (wrong) return 
type.  One place it would make sense to check would be

>             if (ctype && (sfk == sfk_constructor
>                           || sfk == sfk_destructor))
>               {
>                 /* We are within a class's scope. If our declarator name                                                             
>                    is the same as the class name, and we are defining                                                                
>                    a function, then it is a constructor/destructor, and                                                              
>                    therefore returns a void type.  */

Here 'type' is still the return type, we haven't gotten to 
build_function_type yet.

Jason
  

Hi Jason,

On 6 Feb 2025, at 16:48, Jason Merrill wrote:

> On 2/5/25 2:21 PM, Simon Martin wrote:
>> Hi Jason,
>>
>> On 4 Feb 2025, at 21:23, Jason Merrill wrote:
>>
>>> On 2/4/25 3:03 PM, Jason Merrill wrote:
>>>> On 2/4/25 11:45 AM, Simon Martin wrote:
>>>>> On 4 Feb 2025, at 17:17, Jason Merrill wrote:
>>>>>
>>>>>> On 2/4/25 10:56 AM, Simon Martin wrote:
>>>>>>> Hi Jason,
>>>>>>>
>>>>>>> On 4 Feb 2025, at 16:39, Jason Merrill wrote:
>>>>>>>
>>>>>>>> On 1/15/25 9:56 AM, Jason Merrill wrote:
>>>>>>>>> On 1/15/25 7:24 AM, Simon Martin wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> On 14 Jan 2025, at 23:31, Jason Merrill wrote:
>>>>>>>>>>
>>>>>>>>>>> On 1/14/25 2:13 PM, Simon Martin wrote:
>>>>>>>>>>>> On 10 Jan 2025, at 19:10, Andrew Pinski wrote:
>>>>>>>>>>>>> On Fri, Jan 10, 2025 at 3:18 AM Simon Martin
>>>>>>>>>>>>> <simon@nasilyan.com>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> We currently accept the following invalid code (EDG and
>>>>>>>>>>>>>> MSVC
>>>>>>>>>>>>>> do
>>>>>>>>>>>>>> as
>>>>>>>>>>>>>> well)
>>>>>>>>>>>>>
>>>>>>>>>>>>> clang does too:
>>>>>>>>>>>>> https://github.com/llvm/llvm-project/issues/121706 .
>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Note it might be useful if a testcase with multiply `*` is
>>>>>>>>>>>>> included
>>>>>>>>>>>>
>>>>>>>>>>>>> too:
>>>>>>>>>>>>> ```
>>>>>>>>>>>>> struct A {
>>>>>>>>>>>>>        ****A ();
>>>>>>>>>>>>> };
>>>>>>>>>>>>> ```
>>>>>>>>>>>> Thanks, makes sense to add those. Done in the attached
>>>>>>>>>>>> updated
>>>>>>>>>>>> revision,
>>>>>>>>>>>> successfully tested on x86_64-pc-linux-gnu.
>>>>>>>>>>>
>>>>>>>>>>>> +/* Check that it's OK to declare a function at ID_LOC with
>>>>>>>>>>>> the
>>>>>>>>>>>> indicated TYPE,
>>>>>>>>>>>> +   TYPE_QUALS and DECLARATOR.  SFK indicates the kind 
>>>>>>>>>>>> of
>>>>>>>>>>>> special
>>>>>>>>>>>> function (if
>>>>>>>>>>>> +   any) that this function is.  OPTYPE is the type 
>>>>>>>>>>>> given
>>>>>>>>>>>> in
>>>>>>>>>>>> a
>>>>>>>>>>>> conversion
>>>>>>>>>>>>         operator declaration, or the class type for a
>>>>>>>>>>>> constructor/destructor.
>>>>>>>>>>>>         Returns the actual return type of the 
>>>>>>>>>>>> function;
>>>>>>>>>>>> that
>>>>>>>>>>>> may
>>>>>>>>>>>> be
>>>>>>>>>>>> different
>>>>>>>>>>>>         than TYPE if an error occurs, or for certain
>>>>>>>>>>>> special
>>>>>>>>>>>> functions.
>>>>>>>>>>>> */
>>>>>>>>>>>> @@ -12361,8 +12362,19 @@ check_special_function_return_type
>>>>>>>>>>>> (special_function_kind sfk,
>>>>>>>>>>>>                          tree type,
>>>>>>>>>>>>                          tree optype,
>>>>>>>>>>>>                          int 
>>>>>>>>>>>> type_quals,
>>>>>>>>>>>> +                    const cp_declarator
>>>>>>>>>>>> *declarator,
>>>>>>>>>>>> +                    location_t id_loc,
>>>>>>>>>>>
>>>>>>>>>>> id_loc should be the same as declarator->id_loc?
>>>>>>>>>> You’re right.
>>>>>>>>>>
>>>>>>>>>>>>                          const
>>>>>>>>>>>> location_t*
>>>>>>>>>>>> locations)
>>>>>>>>>>>>      {
>>>>>>>>>>>> +  /* If TYPE is unspecified, DECLARATOR, if set, should 
>>>>>>>>>>>> not
>>>>>>>>>>>> represent a pointer
>>>>>>>>>>>> +     or a reference type.  */
>>>>>>>>>>>> +  if (type == NULL_TREE
>>>>>>>>>>>> +      && declarator
>>>>>>>>>>>> +      && (declarator->kind == cdk_pointer
>>>>>>>>>>>> +      || declarator->kind == cdk_reference))
>>>>>>>>>>>> +    error_at (id_loc, "expected unqualified-id before 
>>>>>>>>>>>> %qs
>>>>>>>>>>>> token",
>>>>>>>>>>>> +          declarator->kind == cdk_pointer ? "*" :
>>>>>>>>>>>> "&");
>>>>>>>>>>>
>>>>>>>>>>> ...and id_loc isn't the location of the ptr-operator, it's 
>>>>>>>>>>> the
>>
>>>>>>>>>>> location of the identifier, so this indicates the wrong
>>>>>>>>>>> column.
>>>>>>>>>>> I
>>>>>>>>>>> think using declarator->id_loc makes sense, just not
>>>>>>>>>>> pretending
>>>>>>>>>>> it's
>>>>>>>>>>> the location of the *.
>>>>>>>>>> Good catch, thanks.
>>>>>>>>>>
>>>>>>>>>>> Let's give diagnostics more like the others later in the
>>>>>>>>>>> function
>>>>>>>>>>> instead of trying to emulate cp_parser_error.
>>>>>>>>>> Makes sense. This is what the updated patch does, 
>>>>>>>>>> successfully
>>>>>>>>>> tested on
>>>>>>>>>> x86_64-pc-linux-gnu. OK for GCC 16?
>>>>>>>>>
>>>>>>>>> OK.
>>>>>>>>
>>>>>>>> Does this also fix 118304?  If so, let's go ahead and apply it
>>>>>>>> to
>>>>>>>> GCC
>>>>>>>> 15.
>>>>>>> I have checked just now, and we still ICE for 118304’s 
>>>>>>> testcase
>>>>>>> with
>>>>>>> that fix.
>>>>>>
>>>>>> Why doesn't the preeexisting
>>>>>>
>>>>>> type = void_type_node;
>>>>>>
>>>>>> in check_special_function_return_type fix the return type and 
>>>>>> avoid
>>
>>>>>> the ICE?
>>>>
>>>>> We hit the gcc_assert at method.cc:3593, that Marek’s fix
>>
>>>>> bypasses.
>>>>
>>>> Yes, but why doesn't check_special_function_return_type prevent 
>>>> that?
>>>
>>> Ah, because we call it before walking the declarator.  We need to
>>> check again later, perhaps in grokfndecl, that the type is correct.
>>> Perhaps instead of your patch.
>> One “issue” with adding another check in or close to grokfndecl 
>> is
>> that DECLARATOR will have “been moved to the ID”, and the fact 
>> that
>> we had a CDK_POINTER kind is “lost”. We could obviously somehow
>> propagate this information, but there might be something easier.
>
> The information isn't lost: it's now reflected in the (wrong) return 
> type.  One place it would make sense to check would be
>
>>             if (ctype && (sfk == sfk_constructor
>>                           || sfk == sfk_destructor))
>>               {
>>                 /* We are within a class's scope. If our declarator 
>> name                                                                  
>>               is the same as the class name, and we are defining      
>>                                                                              a 
>> function, then it is a constructor/destructor, and                    
>>                                                              therefore 
>> returns a void type.  */
>
> Here 'type' is still the return type, we haven't gotten to 
> build_function_type yet.
That’s true. However, doesn’t it make sense to cram all the checks 
about the return type of special functions in 
check_special_function_return_type, and return an error if that return 
type is invalid?

Since we’re in stage 4, I understand we want to have the “most 
localised” fixes possible. Would it work for you if I submitted a 
patch that adds the check where you mentioned for GCC 15 (fixing 108304 
only), and another (similar to the one in my previous message) for GCC 
16 (fixing both 108304 and 108306)? Or is there some more fundamental 
issue I’m missing with this approach?

Thanks, Simon
  

On 2/6/25 3:05 PM, Simon Martin wrote:
> Hi Jason,
> 
> On 6 Feb 2025, at 16:48, Jason Merrill wrote:
> 
>> On 2/5/25 2:21 PM, Simon Martin wrote:
>>> Hi Jason,
>>>
>>> On 4 Feb 2025, at 21:23, Jason Merrill wrote:
>>>
>>>> On 2/4/25 3:03 PM, Jason Merrill wrote:
>>>>> On 2/4/25 11:45 AM, Simon Martin wrote:
>>>>>> On 4 Feb 2025, at 17:17, Jason Merrill wrote:
>>>>>>
>>>>>>> On 2/4/25 10:56 AM, Simon Martin wrote:
>>>>>>>> Hi Jason,
>>>>>>>>
>>>>>>>> On 4 Feb 2025, at 16:39, Jason Merrill wrote:
>>>>>>>>
>>>>>>>>> On 1/15/25 9:56 AM, Jason Merrill wrote:
>>>>>>>>>> On 1/15/25 7:24 AM, Simon Martin wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> On 14 Jan 2025, at 23:31, Jason Merrill wrote:
>>>>>>>>>>>
>>>>>>>>>>>> On 1/14/25 2:13 PM, Simon Martin wrote:
>>>>>>>>>>>>> On 10 Jan 2025, at 19:10, Andrew Pinski wrote:
>>>>>>>>>>>>>> On Fri, Jan 10, 2025 at 3:18 AM Simon Martin
>>>>>>>>>>>>>> <simon@nasilyan.com>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> We currently accept the following invalid code (EDG and
>>>>>>>>>>>>>>> MSVC
>>>>>>>>>>>>>>> do
>>>>>>>>>>>>>>> as
>>>>>>>>>>>>>>> well)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> clang does too:
>>>>>>>>>>>>>> https://github.com/llvm/llvm-project/issues/121706 .
>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Note it might be useful if a testcase with multiply `*` is
>>>>>>>>>>>>>> included
>>>>>>>>>>>>>
>>>>>>>>>>>>>> too:
>>>>>>>>>>>>>> ```
>>>>>>>>>>>>>> struct A {
>>>>>>>>>>>>>>         ****A ();
>>>>>>>>>>>>>> };
>>>>>>>>>>>>>> ```
>>>>>>>>>>>>> Thanks, makes sense to add those. Done in the attached
>>>>>>>>>>>>> updated
>>>>>>>>>>>>> revision,
>>>>>>>>>>>>> successfully tested on x86_64-pc-linux-gnu.
>>>>>>>>>>>>
>>>>>>>>>>>>> +/* Check that it's OK to declare a function at ID_LOC with
>>>>>>>>>>>>> the
>>>>>>>>>>>>> indicated TYPE,
>>>>>>>>>>>>> +   TYPE_QUALS and DECLARATOR.  SFK indicates the kind
>>>>>>>>>>>>> of
>>>>>>>>>>>>> special
>>>>>>>>>>>>> function (if
>>>>>>>>>>>>> +   any) that this function is.  OPTYPE is the type
>>>>>>>>>>>>> given
>>>>>>>>>>>>> in
>>>>>>>>>>>>> a
>>>>>>>>>>>>> conversion
>>>>>>>>>>>>>          operator declaration, or the class type for a
>>>>>>>>>>>>> constructor/destructor.
>>>>>>>>>>>>>          Returns the actual return type of the
>>>>>>>>>>>>> function;
>>>>>>>>>>>>> that
>>>>>>>>>>>>> may
>>>>>>>>>>>>> be
>>>>>>>>>>>>> different
>>>>>>>>>>>>>          than TYPE if an error occurs, or for certain
>>>>>>>>>>>>> special
>>>>>>>>>>>>> functions.
>>>>>>>>>>>>> */
>>>>>>>>>>>>> @@ -12361,8 +12362,19 @@ check_special_function_return_type
>>>>>>>>>>>>> (special_function_kind sfk,
>>>>>>>>>>>>>                           tree type,
>>>>>>>>>>>>>                           tree optype,
>>>>>>>>>>>>>                           int
>>>>>>>>>>>>> type_quals,
>>>>>>>>>>>>> +                    const cp_declarator
>>>>>>>>>>>>> *declarator,
>>>>>>>>>>>>> +                    location_t id_loc,
>>>>>>>>>>>>
>>>>>>>>>>>> id_loc should be the same as declarator->id_loc?
>>>>>>>>>>> You’re right.
>>>>>>>>>>>
>>>>>>>>>>>>>                           const
>>>>>>>>>>>>> location_t*
>>>>>>>>>>>>> locations)
>>>>>>>>>>>>>       {
>>>>>>>>>>>>> +  /* If TYPE is unspecified, DECLARATOR, if set, should
>>>>>>>>>>>>> not
>>>>>>>>>>>>> represent a pointer
>>>>>>>>>>>>> +     or a reference type.  */
>>>>>>>>>>>>> +  if (type == NULL_TREE
>>>>>>>>>>>>> +      && declarator
>>>>>>>>>>>>> +      && (declarator->kind == cdk_pointer
>>>>>>>>>>>>> +      || declarator->kind == cdk_reference))
>>>>>>>>>>>>> +    error_at (id_loc, "expected unqualified-id before
>>>>>>>>>>>>> %qs
>>>>>>>>>>>>> token",
>>>>>>>>>>>>> +          declarator->kind == cdk_pointer ? "*" :
>>>>>>>>>>>>> "&");
>>>>>>>>>>>>
>>>>>>>>>>>> ...and id_loc isn't the location of the ptr-operator, it's
>>>>>>>>>>>> the
>>>
>>>>>>>>>>>> location of the identifier, so this indicates the wrong
>>>>>>>>>>>> column.
>>>>>>>>>>>> I
>>>>>>>>>>>> think using declarator->id_loc makes sense, just not
>>>>>>>>>>>> pretending
>>>>>>>>>>>> it's
>>>>>>>>>>>> the location of the *.
>>>>>>>>>>> Good catch, thanks.
>>>>>>>>>>>
>>>>>>>>>>>> Let's give diagnostics more like the others later in the
>>>>>>>>>>>> function
>>>>>>>>>>>> instead of trying to emulate cp_parser_error.
>>>>>>>>>>> Makes sense. This is what the updated patch does,
>>>>>>>>>>> successfully
>>>>>>>>>>> tested on
>>>>>>>>>>> x86_64-pc-linux-gnu. OK for GCC 16?
>>>>>>>>>>
>>>>>>>>>> OK.
>>>>>>>>>
>>>>>>>>> Does this also fix 118304?  If so, let's go ahead and apply it
>>>>>>>>> to
>>>>>>>>> GCC
>>>>>>>>> 15.
>>>>>>>> I have checked just now, and we still ICE for 118304’s
>>>>>>>> testcase
>>>>>>>> with
>>>>>>>> that fix.
>>>>>>>
>>>>>>> Why doesn't the preeexisting
>>>>>>>
>>>>>>> type = void_type_node;
>>>>>>>
>>>>>>> in check_special_function_return_type fix the return type and
>>>>>>> avoid
>>>
>>>>>>> the ICE?
>>>>>
>>>>>> We hit the gcc_assert at method.cc:3593, that Marek’s fix
>>>
>>>>>> bypasses.
>>>>>
>>>>> Yes, but why doesn't check_special_function_return_type prevent
>>>>> that?
>>>>
>>>> Ah, because we call it before walking the declarator.  We need to
>>>> check again later, perhaps in grokfndecl, that the type is correct.
>>>> Perhaps instead of your patch.
>>> One “issue” with adding another check in or close to grokfndecl
>>> is
>>> that DECLARATOR will have “been moved to the ID”, and the fact
>>> that
>>> we had a CDK_POINTER kind is “lost”. We could obviously somehow
>>> propagate this information, but there might be something easier.
>>
>> The information isn't lost: it's now reflected in the (wrong) return
>> type.  One place it would make sense to check would be
>>
>>>              if (ctype && (sfk == sfk_constructor
>>>                            || sfk == sfk_destructor))
>>>                {
>>>                  /* We are within a class's scope. If our declarator
>>> name
>>>                is the same as the class name, and we are defining
>>>                                                                               a
>>> function, then it is a constructor/destructor, and
>>>                                                               therefore
>>> returns a void type.  */
>>
>> Here 'type' is still the return type, we haven't gotten to
>> build_function_type yet.
> That’s true. However, doesn’t it make sense to cram all the checks
> about the return type of special functions in
> check_special_function_return_type, and return an error if that return
> type is invalid?

This error seems easily recoverable since we know what the type needs to 
be, there's no need for error return from grokdeclarator.

However, an alternative to my suggestion above would be to build on your 
patch by making check_special_function_return_type actually strip the 
invalid declarators, not just complain about them.

Jason