c++: Improve diagnostics about conflicting specifiers
Commit Message
Hi!
On Sat, Sep 17, 2022 at 01:23:59AM +0200, Jason Merrill wrote:
> I wonder why we don't give an error when setting the
> conflicting_specifiers_p flag in cp_parser_set_storage_class? We should be
> able to give a better diagnostic at that point.
I didn't have time to update the whole patch last night, but this part
seems to be independent and I've managed to test it.
The diagnostics then looks like:
a.C:1:9: error: ‘static’ specifier conflicts with ‘typedef’
1 | typedef static int a;
| ~~~~~~~ ^~~~~~
a.C:2:8: error: ‘typedef’ specifier conflicts with ‘static’
2 | static typedef int b;
| ~~~~~~ ^~~~~~~
a.C:3:8: error: duplicate ‘static’ specifier
3 | static static int c;
| ~~~~~~ ^~~~~~
a.C:4:8: error: ‘extern’ specifier conflicts with ‘static’
4 | static extern int d;
| ~~~~~~ ^~~~~~
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2022-09-19 Jakub Jelinek <jakub@redhat.com>
gcc/cp/
* parser.cc (cp_parser_lambda_declarator_opt): Don't diagnose
conflicting specifiers here.
(cp_storage_class_name): New variable.
(cp_parser_decl_specifier_seq): When setting conflicting_specifiers_p
for the first time, diagnose which exact specifiers conflict.
(cp_parser_set_storage_class): Likewise. Move storage_class
computation earlier.
* decl.cc (grokdeclarator): Don't diagnose conflicting specifiers
here, just return error_mark_node.
gcc/testsuite/
* g++.dg/diagnostic/conflicting-specifiers-1.C: Adjust expected
diagnostics.
* g++.dg/parse/typedef8.C: Likewise.
* g++.dg/parse/crash39.C: Likewise.
* g++.dg/other/mult-stor1.C: Likewise.
* g++.dg/cpp2a/constinit3.C: Likewise.
Jakub
Comments
On 9/19/22 03:24, Jakub Jelinek wrote:
> Hi!
>
> On Sat, Sep 17, 2022 at 01:23:59AM +0200, Jason Merrill wrote:
>> I wonder why we don't give an error when setting the
>> conflicting_specifiers_p flag in cp_parser_set_storage_class? We should be
>> able to give a better diagnostic at that point.
>
> I didn't have time to update the whole patch last night, but this part
> seems to be independent and I've managed to test it.
>
> The diagnostics then looks like:
> a.C:1:9: error: ‘static’ specifier conflicts with ‘typedef’
> 1 | typedef static int a;
> | ~~~~~~~ ^~~~~~
> a.C:2:8: error: ‘typedef’ specifier conflicts with ‘static’
> 2 | static typedef int b;
> | ~~~~~~ ^~~~~~~
> a.C:3:8: error: duplicate ‘static’ specifier
> 3 | static static int c;
> | ~~~~~~ ^~~~~~
> a.C:4:8: error: ‘extern’ specifier conflicts with ‘static’
> 4 | static extern int d;
> | ~~~~~~ ^~~~~~
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
> 2022-09-19 Jakub Jelinek <jakub@redhat.com>
>
> gcc/cp/
> * parser.cc (cp_parser_lambda_declarator_opt): Don't diagnose
> conflicting specifiers here.
> (cp_storage_class_name): New variable.
> (cp_parser_decl_specifier_seq): When setting conflicting_specifiers_p
> for the first time, diagnose which exact specifiers conflict.
> (cp_parser_set_storage_class): Likewise. Move storage_class
> computation earlier.
> * decl.cc (grokdeclarator): Don't diagnose conflicting specifiers
> here, just return error_mark_node.
> gcc/testsuite/
> * g++.dg/diagnostic/conflicting-specifiers-1.C: Adjust expected
> diagnostics.
> * g++.dg/parse/typedef8.C: Likewise.
> * g++.dg/parse/crash39.C: Likewise.
> * g++.dg/other/mult-stor1.C: Likewise.
> * g++.dg/cpp2a/constinit3.C: Likewise.
>
> --- gcc/cp/parser.cc.jj 2022-09-16 22:34:28.427708581 +0200
> +++ gcc/cp/parser.cc 2022-09-19 09:18:49.561145347 +0200
> @@ -11718,9 +11718,6 @@ cp_parser_lambda_declarator_opt (cp_pars
> {
> LAMBDA_EXPR_MUTABLE_P (lambda_expr) = 1;
> quals = TYPE_UNQUALIFIED;
> - if (lambda_specs.conflicting_specifiers_p)
> - error_at (lambda_specs.locations[ds_storage_class],
> - "duplicate %<mutable%>");
> }
>
> tx_qual = cp_parser_tx_qualifier_opt (parser);
> @@ -15720,6 +15717,13 @@ cp_parser_decomposition_declaration (cp_
> return decl;
> }
>
> +/* Names of storage classes. */
> +
> +static const char *const
> +cp_storage_class_name[] = {
> + "", "auto", "register", "static", "extern", "mutable"
> +};
> +
> /* Parse a decl-specifier-seq.
>
> decl-specifier-seq:
> @@ -15941,8 +15945,18 @@ cp_parser_decl_specifier_seq (cp_parser*
> may as well commit at this point. */
> cp_parser_commit_to_tentative_parse (parser);
>
> - if (decl_specs->storage_class != sc_none)
> - decl_specs->conflicting_specifiers_p = true;
> + if (decl_specs->storage_class != sc_none)
> + {
> + if (decl_specs->conflicting_specifiers_p)
> + break;
> + gcc_rich_location richloc (token->location);
> + location_t oloc = decl_specs->locations[ds_storage_class];
> + richloc.add_location_if_nearby (oloc);
> + error_at (&richloc,
> + "%<typedef%> specifier conflicts with %qs",
> + cp_storage_class_name[decl_specs->storage_class]);
> + decl_specs->conflicting_specifiers_p = true;
> + }
> break;
>
> /* storage-class-specifier:
> @@ -32826,26 +32840,6 @@ cp_parser_set_storage_class (cp_parser *
> {
> cp_storage_class storage_class;
>
> - if (parser->in_unbraced_linkage_specification_p)
> - {
> - error_at (token->location, "invalid use of %qD in linkage specification",
> - ridpointers[keyword]);
> - return;
> - }
> - else if (decl_specs->storage_class != sc_none)
> - {
> - decl_specs->conflicting_specifiers_p = true;
> - return;
> - }
> -
> - if ((keyword == RID_EXTERN || keyword == RID_STATIC)
> - && decl_spec_seq_has_spec_p (decl_specs, ds_thread)
> - && decl_specs->gnu_thread_keyword_p)
> - {
> - pedwarn (decl_specs->locations[ds_thread], 0,
> - "%<__thread%> before %qD", ridpointers[keyword]);
> - }
> -
> switch (keyword)
> {
> case RID_AUTO:
> @@ -32866,6 +32860,38 @@ cp_parser_set_storage_class (cp_parser *
> default:
> gcc_unreachable ();
> }
> +
> + if (parser->in_unbraced_linkage_specification_p)
> + {
> + error_at (token->location, "invalid use of %qD in linkage specification",
> + ridpointers[keyword]);
> + return;
> + }
> + else if (decl_specs->storage_class != sc_none)
> + {
> + if (decl_specs->conflicting_specifiers_p)
> + return;
> + gcc_rich_location richloc (token->location);
> + richloc.add_location_if_nearby (decl_specs->locations[ds_storage_class]);
> + if (decl_specs->storage_class == storage_class)
> + error_at (&richloc, "duplicate %qD specifier", ridpointers[keyword]);
> + else
> + error_at (&richloc,
> + "%qD specifier conflicts with %qs",
> + ridpointers[keyword],
> + cp_storage_class_name[decl_specs->storage_class]);
> + decl_specs->conflicting_specifiers_p = true;
> + return;
> + }
> +
> + if ((keyword == RID_EXTERN || keyword == RID_STATIC)
> + && decl_spec_seq_has_spec_p (decl_specs, ds_thread)
> + && decl_specs->gnu_thread_keyword_p)
> + {
> + pedwarn (decl_specs->locations[ds_thread], 0,
> + "%<__thread%> before %qD", ridpointers[keyword]);
> + }
> +
> decl_specs->storage_class = storage_class;
> set_and_check_decl_spec_loc (decl_specs, ds_storage_class, token);
>
> @@ -32873,8 +32899,16 @@ cp_parser_set_storage_class (cp_parser *
> specifier. If there is a typedef specifier present then set
> conflicting_specifiers_p which will trigger an error later
> on in grokdeclarator. */
> - if (decl_spec_seq_has_spec_p (decl_specs, ds_typedef))
> - decl_specs->conflicting_specifiers_p = true;
> + if (decl_spec_seq_has_spec_p (decl_specs, ds_typedef)
> + && !decl_specs->conflicting_specifiers_p)
> + {
> + gcc_rich_location richloc (token->location);
> + richloc.add_location_if_nearby (decl_specs->locations[ds_typedef]);
> + error_at (&richloc,
> + "%qD specifier conflicts with %<typedef%>",
> + ridpointers[keyword]);
> + decl_specs->conflicting_specifiers_p = true;
> + }
> }
>
> /* Update the DECL_SPECS to reflect the TYPE_SPEC. If TYPE_DEFINITION_P
> --- gcc/cp/decl.cc.jj 2022-09-16 22:34:28.420708676 +0200
> +++ gcc/cp/decl.cc 2022-09-18 20:15:13.357162931 +0200
> @@ -12089,12 +12089,7 @@ grokdeclarator (const cp_declarator *dec
> }
>
> if (declspecs->conflicting_specifiers_p)
> - {
> - error_at (min_location (declspecs->locations[ds_typedef],
> - declspecs->locations[ds_storage_class]),
> - "conflicting specifiers in declaration of %qs", name);
> - return error_mark_node;
> - }
> + return error_mark_node;
>
> /* Extract the basic type from the decl-specifier-seq. */
> type = declspecs->type;
> --- gcc/testsuite/g++.dg/diagnostic/conflicting-specifiers-1.C.jj 2020-01-14 20:02:46.815609354 +0100
> +++ gcc/testsuite/g++.dg/diagnostic/conflicting-specifiers-1.C 2022-09-18 20:55:53.928964842 +0200
> @@ -1 +1 @@
> -static typedef int i __attribute__((unused)); // { dg-error "1:conflicting specifiers" }
> +static typedef int i __attribute__((unused)); // { dg-error "1:'typedef' specifier conflicts with 'static'" }
> --- gcc/testsuite/g++.dg/parse/typedef8.C.jj 2020-01-14 20:02:46.921607767 +0100
> +++ gcc/testsuite/g++.dg/parse/typedef8.C 2022-09-18 20:55:23.829375095 +0200
> @@ -1,11 +1,11 @@
> //PR c++ 29024
>
> -typedef static int a; // { dg-error "conflicting" }
> -typedef register int b; // { dg-error "conflicting" }
> -typedef extern int c; // { dg-error "conflicting" }
> -static typedef int a; // { dg-error "conflicting" }
> +typedef static int a; // { dg-error "'static' specifier conflicts with 'typedef'" }
> +typedef register int b; // { dg-error "'register' specifier conflicts with 'typedef'" }
> +typedef extern int c; // { dg-error "'extern' specifier conflicts with 'typedef'" }
> +static typedef int a; // { dg-error "'typedef' specifier conflicts with 'static'" }
>
> void foo()
> {
> - typedef auto int bar; // { dg-error "conflicting|two or more data types" }
> + typedef auto int bar; // { dg-error "'auto' specifier conflicts with 'typedef'|two or more data types" }
> }
> --- gcc/testsuite/g++.dg/parse/crash39.C.jj 2020-01-14 20:02:46.911607917 +0100
> +++ gcc/testsuite/g++.dg/parse/crash39.C 2022-09-18 20:56:32.004445908 +0200
> @@ -1,3 +1,3 @@
> // PR c++/31747
>
> -static extern int i; // { dg-error "conflicting specifiers" }
> +static extern int i; // { dg-error "'extern' specifier conflicts with 'static'" }
> --- gcc/testsuite/g++.dg/other/mult-stor1.C.jj 2020-01-14 20:02:46.902608051 +0100
> +++ gcc/testsuite/g++.dg/other/mult-stor1.C 2022-09-18 20:57:43.635469614 +0200
> @@ -4,5 +4,5 @@
>
> struct A
> {
> - extern static int i; // { dg-error "conflicting specifiers" }
> + extern static int i; // { dg-error "'static' specifier conflicts with 'extern'" }
> };
> --- gcc/testsuite/g++.dg/cpp2a/constinit3.C.jj 2020-05-13 21:38:28.356420335 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/constinit3.C 2022-09-18 20:57:12.424894986 +0200
> @@ -5,7 +5,7 @@ constinit constinit int v1; // { dg-erro
> constexpr constinit int v2 = 1; // { dg-error "can use at most one of the .constinit. and .constexpr. specifiers" }
> constinit constexpr int v3 = 1; // { dg-error "an use at most one of the .constinit. and .constexpr. specifiers" }
>
> -extern static constinit int v4; // { dg-error "conflicting specifiers" }
> +extern static constinit int v4; // { dg-error "'static' specifier conflicts with 'extern'" }
> extern thread_local constinit int v5;
> extern constinit int v6;
>
>
> Jakub
>
@@ -11718,9 +11718,6 @@ cp_parser_lambda_declarator_opt (cp_pars
{
LAMBDA_EXPR_MUTABLE_P (lambda_expr) = 1;
quals = TYPE_UNQUALIFIED;
- if (lambda_specs.conflicting_specifiers_p)
- error_at (lambda_specs.locations[ds_storage_class],
- "duplicate %<mutable%>");
}
tx_qual = cp_parser_tx_qualifier_opt (parser);
@@ -15720,6 +15717,13 @@ cp_parser_decomposition_declaration (cp_
return decl;
}
+/* Names of storage classes. */
+
+static const char *const
+cp_storage_class_name[] = {
+ "", "auto", "register", "static", "extern", "mutable"
+};
+
/* Parse a decl-specifier-seq.
decl-specifier-seq:
@@ -15941,8 +15945,18 @@ cp_parser_decl_specifier_seq (cp_parser*
may as well commit at this point. */
cp_parser_commit_to_tentative_parse (parser);
- if (decl_specs->storage_class != sc_none)
- decl_specs->conflicting_specifiers_p = true;
+ if (decl_specs->storage_class != sc_none)
+ {
+ if (decl_specs->conflicting_specifiers_p)
+ break;
+ gcc_rich_location richloc (token->location);
+ location_t oloc = decl_specs->locations[ds_storage_class];
+ richloc.add_location_if_nearby (oloc);
+ error_at (&richloc,
+ "%<typedef%> specifier conflicts with %qs",
+ cp_storage_class_name[decl_specs->storage_class]);
+ decl_specs->conflicting_specifiers_p = true;
+ }
break;
/* storage-class-specifier:
@@ -32826,26 +32840,6 @@ cp_parser_set_storage_class (cp_parser *
{
cp_storage_class storage_class;
- if (parser->in_unbraced_linkage_specification_p)
- {
- error_at (token->location, "invalid use of %qD in linkage specification",
- ridpointers[keyword]);
- return;
- }
- else if (decl_specs->storage_class != sc_none)
- {
- decl_specs->conflicting_specifiers_p = true;
- return;
- }
-
- if ((keyword == RID_EXTERN || keyword == RID_STATIC)
- && decl_spec_seq_has_spec_p (decl_specs, ds_thread)
- && decl_specs->gnu_thread_keyword_p)
- {
- pedwarn (decl_specs->locations[ds_thread], 0,
- "%<__thread%> before %qD", ridpointers[keyword]);
- }
-
switch (keyword)
{
case RID_AUTO:
@@ -32866,6 +32860,38 @@ cp_parser_set_storage_class (cp_parser *
default:
gcc_unreachable ();
}
+
+ if (parser->in_unbraced_linkage_specification_p)
+ {
+ error_at (token->location, "invalid use of %qD in linkage specification",
+ ridpointers[keyword]);
+ return;
+ }
+ else if (decl_specs->storage_class != sc_none)
+ {
+ if (decl_specs->conflicting_specifiers_p)
+ return;
+ gcc_rich_location richloc (token->location);
+ richloc.add_location_if_nearby (decl_specs->locations[ds_storage_class]);
+ if (decl_specs->storage_class == storage_class)
+ error_at (&richloc, "duplicate %qD specifier", ridpointers[keyword]);
+ else
+ error_at (&richloc,
+ "%qD specifier conflicts with %qs",
+ ridpointers[keyword],
+ cp_storage_class_name[decl_specs->storage_class]);
+ decl_specs->conflicting_specifiers_p = true;
+ return;
+ }
+
+ if ((keyword == RID_EXTERN || keyword == RID_STATIC)
+ && decl_spec_seq_has_spec_p (decl_specs, ds_thread)
+ && decl_specs->gnu_thread_keyword_p)
+ {
+ pedwarn (decl_specs->locations[ds_thread], 0,
+ "%<__thread%> before %qD", ridpointers[keyword]);
+ }
+
decl_specs->storage_class = storage_class;
set_and_check_decl_spec_loc (decl_specs, ds_storage_class, token);
@@ -32873,8 +32899,16 @@ cp_parser_set_storage_class (cp_parser *
specifier. If there is a typedef specifier present then set
conflicting_specifiers_p which will trigger an error later
on in grokdeclarator. */
- if (decl_spec_seq_has_spec_p (decl_specs, ds_typedef))
- decl_specs->conflicting_specifiers_p = true;
+ if (decl_spec_seq_has_spec_p (decl_specs, ds_typedef)
+ && !decl_specs->conflicting_specifiers_p)
+ {
+ gcc_rich_location richloc (token->location);
+ richloc.add_location_if_nearby (decl_specs->locations[ds_typedef]);
+ error_at (&richloc,
+ "%qD specifier conflicts with %<typedef%>",
+ ridpointers[keyword]);
+ decl_specs->conflicting_specifiers_p = true;
+ }
}
/* Update the DECL_SPECS to reflect the TYPE_SPEC. If TYPE_DEFINITION_P
@@ -12089,12 +12089,7 @@ grokdeclarator (const cp_declarator *dec
}
if (declspecs->conflicting_specifiers_p)
- {
- error_at (min_location (declspecs->locations[ds_typedef],
- declspecs->locations[ds_storage_class]),
- "conflicting specifiers in declaration of %qs", name);
- return error_mark_node;
- }
+ return error_mark_node;
/* Extract the basic type from the decl-specifier-seq. */
type = declspecs->type;
@@ -1 +1 @@
-static typedef int i __attribute__((unused)); // { dg-error "1:conflicting specifiers" }
+static typedef int i __attribute__((unused)); // { dg-error "1:'typedef' specifier conflicts with 'static'" }
@@ -1,11 +1,11 @@
//PR c++ 29024
-typedef static int a; // { dg-error "conflicting" }
-typedef register int b; // { dg-error "conflicting" }
-typedef extern int c; // { dg-error "conflicting" }
-static typedef int a; // { dg-error "conflicting" }
+typedef static int a; // { dg-error "'static' specifier conflicts with 'typedef'" }
+typedef register int b; // { dg-error "'register' specifier conflicts with 'typedef'" }
+typedef extern int c; // { dg-error "'extern' specifier conflicts with 'typedef'" }
+static typedef int a; // { dg-error "'typedef' specifier conflicts with 'static'" }
void foo()
{
- typedef auto int bar; // { dg-error "conflicting|two or more data types" }
+ typedef auto int bar; // { dg-error "'auto' specifier conflicts with 'typedef'|two or more data types" }
}
@@ -1,3 +1,3 @@
// PR c++/31747
-static extern int i; // { dg-error "conflicting specifiers" }
+static extern int i; // { dg-error "'extern' specifier conflicts with 'static'" }
@@ -4,5 +4,5 @@
struct A
{
- extern static int i; // { dg-error "conflicting specifiers" }
+ extern static int i; // { dg-error "'static' specifier conflicts with 'extern'" }
};
@@ -5,7 +5,7 @@ constinit constinit int v1; // { dg-erro
constexpr constinit int v2 = 1; // { dg-error "can use at most one of the .constinit. and .constexpr. specifiers" }
constinit constexpr int v3 = 1; // { dg-error "an use at most one of the .constinit. and .constexpr. specifiers" }
-extern static constinit int v4; // { dg-error "conflicting specifiers" }
+extern static constinit int v4; // { dg-error "'static' specifier conflicts with 'extern'" }
extern thread_local constinit int v5;
extern constinit int v6;