On 6/9/25 1:19 PM, Jakub Jelinek wrote:
On Mon, Jun 09, 2025 at 12:17:12PM -0400, Jason Merrill wrote:
While the
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2025/p2786r13.html#c03-compatibility-changes-for-annex-c-diff.cpp03.dcl.dcl
hunk dropped because
struct C {}; struct C {} final {};
is actually not valid C++98 (which didn't have list initialization), we
actually also reject
struct D {}; struct D {} override {};
and that IMHO is valid all the way from C++11 onwards.
I don't see how it could be, and other compilers agree with me. Since the
testcases don't put {} between the class name and the contextual keyword, I
think this is just a typo in the comment.
Oops, sure, consider the second {} pair removed. Sorry.
Especially in the light of P2786R13 adding new contextual keywords, I think
it is better to use a separate routine for parsing the
class-virt-specifier-seq (in C++11, there was export next to final),
class-virt-specifier (in C++14 to C++23) and
class-property-specifier-seq (in C++26) instead of using the same function
for virt-specifier-seq and class-property-specifier-seq.
Tested on x86_64-linux and i686-linux, ok for trunk?
2025-06-06 Jakub Jelinek <ja...@redhat.com>
PR c++/120569
* parser.cc (cp_parser_class_virt_specifier_seq_opt): New function.
Let's refer to the new non-terminal rather than the old one, both in the
name and comment of this function.
That was actually my first version of the patch, but thought it would be
weird to talk about C++26 name where the support isn't even in.
Anyway, changed now.
Is it ok like that or do you want also to rename the virt_specifiers and
virt_specifier automatic variables and perhaps use a different enum?
I don't feel as strongly about the variables and (existing) enum as a
new non-terminal parsing function. This patch is OK.
+ if (virt_specifiers & virt_specifier)
+ {
+ gcc_rich_location richloc (token->location);
+ richloc.add_fixit_remove ();
+ error_at (&richloc, "duplicate class-virt-specifier");
And here let's name the actual duplicate specifier rather refer to the
grammar.
Changed. Though if this is ever backported, perhaps it should use
the same diagnostics as before in the backports.
So like this? So far lightly tested.
2025-06-09 Jakub Jelinek <ja...@redhat.com>
PR c++/120569
* parser.cc (cp_parser_class_property_specifier_seq_opt): New
function.
(cp_parser_class_head): Use it instead of
cp_parser_property_specifier_seq_opt. Don't diagnose
VIRT_SPEC_OVERRIDE here. Formatting fix.
* g++.dg/cpp0x/override2.C: Expect different diagnostics with
override or duplicate final.
* g++.dg/cpp0x/override5.C: New test.
* g++.dg/cpp0x/duplicate1.C: Expect different diagnostics with
duplicate final.
--- gcc/cp/parser.cc.jj 2025-06-02 11:00:14.000000000 +0200
+++ gcc/cp/parser.cc 2025-06-06 17:20:16.344079071 +0200
@@ -28005,6 +28005,57 @@ cp_parser_class_specifier (cp_parser* pa
return type;
}
+/* Parse an (optional) class-property-specifier-seq.
+
+ class-property-specifier-seq:
+ class-property-specifier class-property-specifier-seq [opt]
+
+ class-property-specifier:
+ final
+
+ Returns a bitmask representing the class-property-specifiers. */
+
+static cp_virt_specifiers
+cp_parser_class_property_specifier_seq_opt (cp_parser *parser)
+{
+ cp_virt_specifiers virt_specifiers = VIRT_SPEC_UNSPECIFIED;
+
+ while (true)
+ {
+ cp_token *token;
+ cp_virt_specifiers virt_specifier;
+
+ /* Peek at the next token. */
+ token = cp_lexer_peek_token (parser->lexer);
+ /* See if it's a class-property-specifier. */
+ if (token->type != CPP_NAME)
+ break;
+ if (id_equal (token->u.value, "final"))
+ {
+ maybe_warn_cpp0x (CPP0X_OVERRIDE_CONTROLS);
+ virt_specifier = VIRT_SPEC_FINAL;
+ }
+ else if (id_equal (token->u.value, "__final"))
+ virt_specifier = VIRT_SPEC_FINAL;
+ else
+ break;
+
+ if (virt_specifiers & virt_specifier)
+ {
+ gcc_rich_location richloc (token->location);
+ richloc.add_fixit_remove ();
+ error_at (&richloc, "duplicate %qD specifier", token->u.value);
+ cp_lexer_purge_token (parser->lexer);
+ }
+ else
+ {
+ cp_lexer_consume_token (parser->lexer);
+ virt_specifiers |= virt_specifier;
+ }
+ }
+ return virt_specifiers;
+}
+
/* Parse a class-head.
class-head:
@@ -28195,12 +28246,10 @@ cp_parser_class_head (cp_parser* parser,
pop_deferring_access_checks ();
if (id)
- {
- cp_parser_check_for_invalid_template_id (parser, id,
- class_key,
- type_start_token->location);
- }
- virt_specifiers = cp_parser_virt_specifier_seq_opt (parser);
+ cp_parser_check_for_invalid_template_id (parser, id,
+ class_key,
+ type_start_token->location);
+ virt_specifiers = cp_parser_class_property_specifier_seq_opt (parser);
/* If it's not a `:' or a `{' then we can't really be looking at a
class-head, since a class-head only appears as part of a
@@ -28216,13 +28265,6 @@ cp_parser_class_head (cp_parser* parser,
/* At this point, we're going ahead with the class-specifier, even
if some other problem occurs. */
cp_parser_commit_to_tentative_parse (parser);
- if (virt_specifiers & VIRT_SPEC_OVERRIDE)
- {
- cp_parser_error (parser,
- "cannot specify %<override%> for a class");
- type = error_mark_node;
- goto out;
- }
/* Issue the error about the overly-qualified name now. */
if (qualified_p)
{
--- gcc/testsuite/g++.dg/cpp0x/override2.C.jj 2020-01-12 11:54:37.089403210
+0100
+++ gcc/testsuite/g++.dg/cpp0x/override2.C 2025-06-06 17:13:18.239613567
+0200
@@ -23,9 +23,9 @@ struct D5 : B3<D5> {};
struct D6 : B4<D6> {}; // { dg-error "cannot derive from 'final' base" }
-struct B6 final final {}; // { dg-error "duplicate virt-specifier" }
+struct B6 final final {}; // { dg-error "duplicate 'final' specifier" }
-struct B7 override {}; // { dg-error "cannot specify 'override' for a class" }
+struct B7 override {}; // { dg-error "variable 'B7 override' has initializer but
incomplete type" }
namespace N
{
@@ -45,7 +45,7 @@ int main()
struct B2 final; // { dg-error "redeclaration" }
struct B2 override; // { dg-message "previously declared here" }
struct B2 final {}; // { dg-error "redefinition" }
- struct B2 override {}; // { dg-error "cannot specify 'override' for a class"
}
+ struct B2 override {}; // { dg-error "redeclaration of 'main\\\(\\\)::B2
override'" }
B2 override{}; // { dg-error "redeclaration" }
struct foo final {}; // { dg-message "previous definition" }
struct foo final {}; // { dg-error "redefinition" }
--- gcc/testsuite/g++.dg/cpp0x/override5.C.jj 2025-06-06 17:08:53.022001764
+0200
+++ gcc/testsuite/g++.dg/cpp0x/override5.C 2025-06-06 17:24:57.683354950
+0200
@@ -0,0 +1,26 @@
+// PR c++/120569
+// { dg-do compile }
+// { dg-options "" }
+// { dg-additional-options "-pedantic" { target c++14 } }
+
+namespace U {
+ struct A {};
+ struct A override {}; // { dg-warning "extended initializer lists only
available with" "" { target c++98_only } }
+}
+namespace V {
+ template <int N>
+ struct B {};
+ template <int N>
+ struct B<N> override {}; // { dg-warning "extended initializer lists only available
with" "" { target c++98_only } }
+} // { dg-warning "variable templates only available with"
"" { target c++11_down } .-1 }
+struct C {
+ struct D {};
+ struct D override {}; // { dg-warning "extended initializer lists only
available with" "" { target c++98_only } }
+}; // { dg-warning "non-static data member initializers only
available with" "" { target c++98_only } .-1 }
+namespace W {
+ struct E { struct F {}; };
+ struct E::F override {}; // { dg-warning "extended initializer lists only available
with" "" { target c++98_only } }
+}
+template <int N>
+struct V::B<N> override {}; // { dg-warning "extended initializer lists only available
with" "" { target c++98_only } }
+ // { dg-warning "variable templates only available with"
"" { target c++11_down } .-1 }
--- gcc/testsuite/g++.dg/cpp0x/duplicate1.C.jj 2020-01-14 20:02:46.724610718
+0100
+++ gcc/testsuite/g++.dg/cpp0x/duplicate1.C 2025-06-06 18:28:14.178916305
+0200
@@ -6,7 +6,7 @@ struct A
virtual void foo() const;
};
-struct B final final : A /* { dg-error "duplicate virt-specifier" }
+struct B final final : A /* { dg-error "duplicate 'final' specifier" }
{ dg-begin-multiline-output "" }
struct B final final : A
^~~~~
Jakub
