c++: Improve diagnostics about conflicting specifiers

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;
      | ~~~~~~ ^~~~~~

2022-09-27  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.

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 80467c192546feed0c44672b8448f14921f9b195..f4460c911b30b34618bfe64c95013315dd7db07d 100644 (file)
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -12095,12 +12095,7 @@ grokdeclarator (const cp_declarator *declarator,
     }
 
   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;

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 3dedd14042852efa14de52b8e84d329d0c07506a..d876a8623b4539f1fe597b992f231e78fc737fac 100644 (file)
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -11729,9 +11729,6 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr)
     {
       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);
@@ -15731,6 +15728,13 @@ cp_parser_decomposition_declaration (cp_parser *parser,
   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:
@@ -15952,8 +15956,18 @@ cp_parser_decl_specifier_seq (cp_parser* 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:
@@ -32845,26 +32859,6 @@ cp_parser_set_storage_class (cp_parser *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:
@@ -32885,6 +32879,38 @@ cp_parser_set_storage_class (cp_parser *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);
 
@@ -32892,8 +32918,16 @@ cp_parser_set_storage_class (cp_parser *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

diff --git a/gcc/testsuite/g++.dg/cpp2a/constinit3.C b/gcc/testsuite/g++.dg/cpp2a/constinit3.C
index a29c594657ac07f371b8498e58c03edd6351c183..ffa6184782438ef46644a012cc8d14a02b6b47a8 100644 (file)
--- a/gcc/testsuite/g++.dg/cpp2a/constinit3.C
+++ b/gcc/testsuite/g++.dg/cpp2a/constinit3.C
@@ -5,7 +5,7 @@ constinit constinit int v1; // { dg-error "duplicate .constinit." }
 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;
 

diff --git a/gcc/testsuite/g++.dg/diagnostic/conflicting-specifiers-1.C b/gcc/testsuite/g++.dg/diagnostic/conflicting-specifiers-1.C
index 1a8ac02b77efe85ab9c4a4a11e21e06626cef5eb..89e2ebdebf6b427409d96b1602566f8b987a013e 100644 (file)
--- a/gcc/testsuite/g++.dg/diagnostic/conflicting-specifiers-1.C
+++ b/gcc/testsuite/g++.dg/diagnostic/conflicting-specifiers-1.C
@@ -1 +1 @@
-static typedef int i __attribute__((unused));  // { dg-error "1:conflicting specifiers" }
+static typedef int i __attribute__((unused));  // { dg-error "8:'typedef' specifier conflicts with 'static'" }

diff --git a/gcc/testsuite/g++.dg/other/mult-stor1.C b/gcc/testsuite/g++.dg/other/mult-stor1.C
index 1eaec4f14f9b37d94656e7990cda3250832d1dcb..e582b03ee60dc3542b8e02d3b76e98e082565c7d 100644 (file)
--- a/gcc/testsuite/g++.dg/other/mult-stor1.C
+++ b/gcc/testsuite/g++.dg/other/mult-stor1.C
@@ -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'" }
 };

diff --git a/gcc/testsuite/g++.dg/parse/crash39.C b/gcc/testsuite/g++.dg/parse/crash39.C
index 2f39c10e31de237ab8800cf30a1a2ff1af609d12..5d4e02d79ca0e457eb0249fe4121c496ba52bf11 100644 (file)
--- a/gcc/testsuite/g++.dg/parse/crash39.C
+++ b/gcc/testsuite/g++.dg/parse/crash39.C
@@ -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'" }

diff --git a/gcc/testsuite/g++.dg/parse/typedef8.C b/gcc/testsuite/g++.dg/parse/typedef8.C
index 60b8f39ee5e015eef35deb52ee1f7c4dd3f74ddc..e21bdb911d9a0b4258064e5e933534502de0e49c 100644 (file)
--- a/gcc/testsuite/g++.dg/parse/typedef8.C
+++ b/gcc/testsuite/g++.dg/parse/typedef8.C
@@ -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" }
 }