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 <si...@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?
SimonFrom d60501bbbb3234f3229ddd3df0691426e00051ee Mon Sep 17 00:00:00 2001
From: Simon Martin <si...@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
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 90e9a7fc7cb..d07cc9ec611 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);
@@ -12349,10 +12350,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. */
@@ -12361,6 +12371,7 @@ check_special_function_return_type
(special_function_kind sfk,
tree type,
tree optype,
int type_quals,
+ const cp_declarator* declarator,
const location_t* locations)
{
switch (sfk)
@@ -12369,6 +12380,9 @@ check_special_function_return_type
(special_function_kind sfk,
if (type)
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),
"qualifiers are not allowed on constructor declaration");
@@ -12383,6 +12397,9 @@ check_special_function_return_type
(special_function_kind sfk,
if (type)
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),
"qualifiers are not allowed on destructor declaration");
@@ -12399,6 +12416,9 @@ check_special_function_return_type
(special_function_kind sfk,
if (type)
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),
"qualifiers are not allowed on declaration of "
@@ -12411,6 +12431,9 @@ check_special_function_return_type
(special_function_kind sfk,
if (type)
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),
"qualifiers are not allowed on declaration of "
@@ -13089,6 +13112,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/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" }
--
2.44.0
