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

  We currently accept the following invalid code (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 (check_special_function_return_type): Take declarator
	and location as input. 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                              | 21 +++++++++---
 gcc/testsuite/g++.dg/parse/constructor4.C   | 36 +++++++++++++++++++++
 gcc/testsuite/g++.dg/parse/conv_op2.C       |  8 +++++
 gcc/testsuite/g++.dg/parse/default_to_int.C | 33 +++++++++++++++++++
 4 files changed, 94 insertions(+), 4 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
  

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,
Andrew

>
> === 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 (check_special_function_return_type): Take declarator
>         and location as input. 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                              | 21 +++++++++---
>  gcc/testsuite/g++.dg/parse/constructor4.C   | 36 +++++++++++++++++++++
>  gcc/testsuite/g++.dg/parse/conv_op2.C       |  8 +++++
>  gcc/testsuite/g++.dg/parse/default_to_int.C | 33 +++++++++++++++++++
>  4 files changed, 94 insertions(+), 4 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
>
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 5c6a4996a89..b57df261e76 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*,
> +       location_t, 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);
> @@ -12349,9 +12350,9 @@ 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
> +/* 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,
>                                     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 ? "*" : "&");
> +
>    switch (sfk)
>      {
>      case sfk_constructor:
> @@ -13089,6 +13101,7 @@ grokdeclarator (const cp_declarator *declarator,
>        type = check_special_function_return_type (sfk, type,
>                                                  ctor_return_type,
>                                                  type_quals,
> +                                                declarator, id_loc,
>                                                  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..7d5a8ecaa97
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/parse/constructor4.C
> @@ -0,0 +1,36 @@
> +// PR c++/118306
> +// { dg-do "compile" }
> +
> +// Constructors.
> +struct A {
> +  *A ();           // { dg-error "expected unqualified-id" }
> +};
> +struct B {
> +  &B ();           // { dg-error "expected unqualified-id|reference to" }
> +};
> +struct C {
> +  *C (const C&);    // { dg-error "expected unqualified-id" }
> +};
> +struct D {
> +  &D (const D&);    // { dg-error "expected unqualified-id|reference to" }
> +};
> +struct E {
> +  const E();       // { dg-error "expected unqualified-id" }
> +};
> +
> +// Destructors.
> +struct F {
> +  * ~F ();         // { dg-error "expected unqualified-id" }
> +};
> +struct G {
> +  & ~G ();         // { dg-error "expected unqualified-id|reference to" }
> +};
> +struct H {
> +  virtual * ~H ();  // { dg-error "expected unqualified-id" }
> +};
> +struct I {
> +  virtual & ~I ();  // { dg-error "expected unqualified-id|reference to" }
> +};
> +struct J {
> +  volatile ~J();    // { dg-error "qualifiers are not allowed" }
> +};
> 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..33477de6274
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/parse/conv_op2.C
> @@ -0,0 +1,8 @@
> +// PR c++/118306
> +// { dg-do "compile" }
> +
> +struct K {
> +  char operator int(); // { dg-error "return type specified for" }
> +  * operator short();  // { dg-error "expected unqualified-id" }
> +  & operator long();   // { dg-error "expected unqualified-id" }
> +};
> 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..5c3ec37e007
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/parse/default_to_int.C
> @@ -0,0 +1,33 @@
> +// 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" }
> +  const * mem2;            // { dg-warning "forbids declaration" }
> +  & mem3;          // { dg-warning "forbids declaration" }
> +  volatile & mem4;  // { 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" }
> +const bazz (int) { return 0; }   // { dg-warning "forbids declaration" }
> +const* bazzz (int) { return 0; }  // { dg-warning "forbids declaration" }
> --
> 2.44.0
>
  

Hi,

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.

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

We currently accept the following invalid code (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 (check_special_function_return_type): Take declarator
	and location as input. 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                              | 21 ++++++--
 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, 118 insertions(+), 4 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

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 5c6a4996a89..b57df261e76 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*,
+	location_t, 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);
@@ -12349,9 +12350,9 @@ 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
+/* 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,
 				    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 ? "*" : "&");
+
   switch (sfk)
     {
     case sfk_constructor:
@@ -13089,6 +13101,7 @@ grokdeclarator (const cp_declarator *declarator,
       type = check_special_function_return_type (sfk, type,
 						 ctor_return_type,
 						 type_quals,
+						 declarator, id_loc,
 						 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..d9cb8b6ef03
--- /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 "expected unqualified-id" }
+};
+struct B {
+  **B ();	    // { dg-error "expected unqualified-id" }
+};
+struct C {
+  ***C ();	    // { dg-error "expected unqualified-id" }
+};
+struct D {
+  &D ();	    // { dg-error "expected unqualified-id|reference to" }
+};
+struct E {
+  *&E ();	    // { dg-error "expected unqualified-id|reference to" }
+};
+struct F {
+  **&F ();	    // { dg-error "expected unqualified-id|reference to" }
+};
+struct G {
+  *G (const G&);    // { dg-error "expected unqualified-id" }
+};
+struct H {
+  **H (const H&);    // { dg-error "expected unqualified-id" }
+};
+struct I {
+  &I (const I&);    // { dg-error "expected unqualified-id|reference to" }
+};
+struct J {
+  const J();	    // { dg-error "expected unqualified-id" }
+};
+
+// Destructors.
+struct K {
+  * ~K ();	    // { dg-error "expected unqualified-id" }
+};
+struct L {
+  ** ~L ();	    // { dg-error "expected unqualified-id" }
+};
+struct M {
+  & ~M ();	    // { dg-error "expected unqualified-id|reference to" }
+};
+struct N {
+  virtual * ~N ();  // { dg-error "expected unqualified-id" }
+};
+struct O {
+  virtual & ~O ();  // { dg-error "expected unqualified-id|reference to" }
+};
+struct P {
+  volatile ~P();    // { dg-error "qualifiers are not allowed" }
+};
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..19b055115c8
--- /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 "expected unqualified-id" }
+  ** operator float();	// { dg-error "expected unqualified-id" }
+  &* operator double();	// { dg-error "expected unqualified-id|pointer to 'double&'" }
+  & operator long();	// { dg-error "expected unqualified-id" }
+};
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" }
-- 
2.44.0
  

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?

>  				    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 *.

Let's give diagnostics more like the others later in the function 
instead of trying to emulate cp_parser_error.

Jason