[PATCH] avoid -Wredundant-tags on a first declaration in use (PR 93824)

[Martin Sebor <msebor@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 579 bytes --]

-Wredundant-tags doesn't consider type declarations that are also
the first uses of the type, such as in 'void f (struct S);' and
issues false positives for those.  According to the reported that's
making it harder to use the warning to clean up LibreOffice.

The attached patch extends -Wredundant-tags to avoid these false
positives by relying on the same class_decl_loc_t::class2loc mapping
as -Wmismatched-tags.  The patch also somewhat improves the detection
of both issues in template declarations (though more work is still
needed there).

Tested on x86_64-linux.

Martin

[-- Attachment #2: gcc-93824.diff --]
[-- Type: text/x-patch, Size: 11661 bytes --]

PR c++/93824 - bogus -Wredundant-tags on a first declaration in use
PR c++/93810 - missing -Wmismatched-tags and -Wredundant-tags on a typedef of an implicit class template specialization

gcc/cp/ChangeLog:

	PR c++/93824
	PR c++/93810
	* parser.c (cp_parser_check_class_key): Move code...
	(class_decl_loc_t::add): ...to here.  Add parameters.  Avoid issuing
	-Wredundant-tags on first-time declarations in other declarators.
	(class_decl_loc_t::diag_mismatched_tags): Also expect to be called
	when -Wredundant-tags is enabled.

gcc/testsuite/ChangeLog:

	PR c++/93824
	PR c++/93810
	* g++.dg/warn/Wmismatched-tags-3.C: New test.
	* g++.dg/warn/Wredundant-tags-3.C: Remove xfails.
	* g++.dg/warn/Wredundant-tags-6.C: New test.

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index ca85d899427..3cf2f2d8b34 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -30924,8 +30924,9 @@ class class_decl_loc_t
   /* Issues -Wmismatched-tags for all classes.  */
   static void diag_mismatched_tags ();
 
-  /* Adds TYPE_DECL to the collection of class decls.  */
-  static void add (tree, tag_types, bool, bool);
+  /* Adds TYPE_DECL to the collection of class decls and diagnoses
+     redundant tags (if -Wredundant-tags is enabled).  */
+  static void add (cp_parser *, location_t, tag_types, tree, bool, bool);
 
   /* Either adds this decl to the collection of class decls
      or diagnoses it, whichever is appropriate.  */
@@ -31021,6 +31022,19 @@ cp_parser_check_class_key (cp_parser *parser, location_t key_loc,
       && class_key != union_type)
     return;
 
+  class_decl_loc_t::add (parser, key_loc, class_key, type, def_p, decl_p);
+}
+
+/* Adds TYPE to the collection of class decls and diagnoses redundant
+   tags (if -Wredundant-tags is enabled).
+   DEF_P is expected to be set for a definition of class TYPE.  DECL_P
+   is set for a declaration of class TYPE and clear for a reference to
+   it that is not a declaration of it.  */
+
+void
+class_decl_loc_t::add (cp_parser *parser, location_t key_loc,
+		       tag_types class_key, tree type, bool def_p, bool decl_p)
+{
   tree type_decl = TYPE_MAIN_DECL (type);
   tree name = DECL_NAME (type_decl);
   /* Look up the NAME to see if it unambiguously refers to the TYPE
@@ -31032,7 +31046,10 @@ cp_parser_check_class_key (cp_parser *parser, location_t key_loc,
   /* The class-key is redundant for uses of the CLASS_TYPE that are
      neither definitions of it nor declarations, and for which name
      lookup returns just the type itself.  */
-  bool key_redundant = !def_p && !decl_p && decl == type_decl;
+  bool key_redundant = (!def_p && !decl_p
+			&& (decl == type_decl
+			    || TREE_CODE (decl) == TEMPLATE_DECL
+			    || TYPE_BEING_DEFINED (type)));
 
   if (key_redundant
       && class_key != class_type
@@ -31050,29 +31067,8 @@ cp_parser_check_class_key (cp_parser *parser, location_t key_loc,
 	key_redundant = false;
     }
 
-  if (key_redundant)
-    {
-      gcc_rich_location richloc (key_loc);
-      richloc.add_fixit_remove (key_loc);
-      warning_at (&richloc, OPT_Wredundant_tags,
-		  "redundant class-key %qs in reference to %q#T",
-		  class_key == union_type ? "union"
-		  : class_key == record_type ? "struct" : "class",
-		  type);
-    }
-
-  if (seen_as_union || !warn_mismatched_tags)
-    return;
-
-  class_decl_loc_t::add (type_decl, class_key, key_redundant, def_p);
-}
-
-/* Adds TYPE_DECL to the collection of class decls.  */
-
-void
-class_decl_loc_t::add (tree type_decl, tag_types class_key, bool redundant,
-		       bool def_p)
-{
+  /* Set if a declaration of TYPE has previously been seen or if it
+     must exist in a precompiled header.  */
   bool exist;
   class_decl_loc_t *rdl = &class2loc.get_or_insert (type_decl, &exist);
   if (!exist)
@@ -31082,30 +31078,51 @@ class_decl_loc_t::add (tree type_decl, tag_types class_key, bool redundant,
 	{
 	  /* TYPE_DECL is the first declaration or definition of the type
 	     (outside precompiled headers -- see below).  Just create
-	     a new entry for it.  */
+	     a new entry for it and return unless it's a declaration
+	     involving a template that may need to be diagnosed by
+	     -Wredundant-tags.  */
 	  *rdl = class_decl_loc_t (class_key, false, def_p);
-	  return;
+	  if (TREE_CODE (decl) != TEMPLATE_DECL)
+	    return;
+	}
+      else
+	{
+	  /* TYPE was previously defined in some unknown precompiled hdeader.
+	     Simply add a record of its definition at an unknown location and
+	     proceed below to add a reference to it at the current location.
+	     (Declarations in precompiled headers that are not definitions
+	     are ignored.)  */
+	  tag_types def_key
+	    = CLASSTYPE_DECLARED_CLASS (type) ? class_type : record_type;
+	  location_t def_loc = DECL_SOURCE_LOCATION (type_decl);
+	  *rdl = class_decl_loc_t (def_key, false, true, def_loc);
+	  exist = true;
 	}
-
-      /* TYPE was previously defined in some unknown precompiled hdeader.
-	 Simply add a record of its definition at an unknown location and
-	 proceed below to add a reference to it at the current location.
-	 (Declarations in precompiled headers that are not definitions
-	 are ignored.)  */
-      tag_types def_key
-	= CLASSTYPE_DECLARED_CLASS (type) ? class_type : record_type;
-      location_t def_loc = DECL_SOURCE_LOCATION (type_decl);
-      *rdl = class_decl_loc_t (def_key, false, true, def_loc);
     }
 
   /* A prior declaration of TYPE_DECL has been seen.  */
 
+  if (key_redundant)
+    {
+      gcc_rich_location richloc (key_loc);
+      richloc.add_fixit_remove (key_loc);
+      warning_at (&richloc, OPT_Wredundant_tags,
+		  "redundant class-key %qs in reference to %q#T",
+		  class_key == union_type ? "union"
+		  : class_key == record_type ? "struct" : "class",
+		  type);
+    }
+
+  if (!exist)
+    /* Do nothing if this is the first declaration of the type.  */
+    return;
+
   if (rdl->idxdef != UINT_MAX && rdl->def_class_key == class_key)
     /* Do nothing if the class-key in this declaration matches
        the definition.  */
     return;
 
-  rdl->add_or_diag_mismatched_tag (type_decl, class_key, redundant, def_p);
+  rdl->add_or_diag_mismatched_tag (type_decl, class_key, key_redundant, def_p);
 }
 
 /* Either adds this DECL corresponding to the TYPE_DECL to the collection
@@ -31158,6 +31175,9 @@ class_decl_loc_t::add_or_diag_mismatched_tag (tree type_decl,
 void
 class_decl_loc_t::diag_mismatched_tags (tree type_decl)
 {
+  if (!warn_mismatched_tags)
+    return;
+
   unsigned ndecls = locvec.length ();
 
   /* Skip a declaration that consistently uses the same class-key
@@ -31249,20 +31269,26 @@ class_decl_loc_t::diag_mismatched_tags (tree type_decl)
 void
 class_decl_loc_t::diag_mismatched_tags ()
 {
-  /* CLASS2LOC should be empty if -Wmismatched-tags is disabled.  */
-  gcc_assert (warn_mismatched_tags || class2loc.is_empty ());
+  /* CLASS2LOC should be empty if both -Wmismatched-tags and
+     -Wredundant-tags are disabled.  */
+  gcc_assert (warn_mismatched_tags
+	      || warn_redundant_tags
+	      || class2loc.is_empty ());
 
   /* Save the current function before changing it below.  It should
      be null at this point.  */
   tree save_func = current_function_decl;
 
-  /* Iterate over the collected class/struct declarations.  */
-  typedef class_to_loc_map_t::iterator iter_t;
-  for (iter_t it = class2loc.begin (); it != class2loc.end (); ++it)
+  if (warn_mismatched_tags)
     {
-      tree type_decl = (*it).first;
-      class_decl_loc_t &recloc = (*it).second;
-      recloc.diag_mismatched_tags (type_decl);
+      /* Iterate over the collected class/struct declarations.  */
+      typedef class_to_loc_map_t::iterator iter_t;
+      for (iter_t it = class2loc.begin (); it != class2loc.end (); ++it)
+	{
+	  tree type_decl = (*it).first;
+	  class_decl_loc_t &recloc = (*it).second;
+	  recloc.diag_mismatched_tags (type_decl);
+	}
     }
 
   class2loc.empty ();
diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-tags-3.C b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-3.C
new file mode 100644
index 00000000000..ecbe66d037e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-3.C
@@ -0,0 +1,14 @@
+/* { dg-do compile }
+   { dg-options "-Wall -Wmismatched-tags" } */
+
+extern class C1 c1;             // { dg-message "declared as 'class'" }
+extern struct C1 c1;            // { dg-warning "\\\[-Wmismatched-tags" }
+
+void fs1 (struct S1);           // { dg-message "declared as 'struct'" }
+void fs1 (class S1);            // { dg-warning "\\\[-Wmismatched-tags" }
+
+enum
+{
+  ec2 = sizeof (struct C2*),    // { dg-message "declared as 'struct'" }
+  fc2 = sizeof (class C2*)      // { dg-warning "\\\[-Wmismatched-tags" }
+};
diff --git a/gcc/testsuite/g++.dg/warn/Wredundant-tags-3.C b/gcc/testsuite/g++.dg/warn/Wredundant-tags-3.C
index 7b30e949d0c..0eeee34dae7 100644
--- a/gcc/testsuite/g++.dg/warn/Wredundant-tags-3.C
+++ b/gcc/testsuite/g++.dg/warn/Wredundant-tags-3.C
@@ -34,12 +34,12 @@ union N::U u3;        // { dg-warning "-Wredundant-tags" }
 
 typedef N::TC<0> TC0;
 typedef typename N::TC<0> TC0;
-typedef class N::TC<0> TC0;   // { dg-warning "-Wredundant-tags" "pr93809" { xfail *-*-*} .-1 }
+typedef class N::TC<0> TC0;   // { dg-warning "-Wredundant-tags" }
 
 typedef N::TS<0> TS0;
 typedef typename N::TS<0> TS0;
-typedef struct N::TS<0> TS0;  // { dg-warning "-Wredundant-tags" "pr93809" { xfail *-*-*} .-1 }
+typedef struct N::TS<0> TS0;  // { dg-warning "-Wredundant-tags" }
 
 typedef N::TS<0> TS0;
 typedef typename N::TS<0> TS0;
-typedef struct N::TS<0> TS0;  // { dg-warning "-Wredundant-tags" "pr93809" { xfail *-*-*} .-1 }
+typedef struct N::TS<0> TS0;  // { dg-warning "-Wredundant-tags" }
diff --git a/gcc/testsuite/g++.dg/warn/Wredundant-tags-6.C b/gcc/testsuite/g++.dg/warn/Wredundant-tags-6.C
new file mode 100644
index 00000000000..a21cc14b2d8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wredundant-tags-6.C
@@ -0,0 +1,51 @@
+/* PR c++/93824 - bogus -Wredundant-tags on a first declaration in use
+   { dg-do compile }
+   { dg-options "-Wredundant-tags" } */
+
+extern class C1 &c1;              // { dg-bogus "\\\[-Wredundant-tags" }
+extern class C1 &c1;              // { dg-warning "\\\[-Wredundant-tags" }
+
+void fc2 (class C2);              // { dg-bogus "\\\[-Wredundant-tags" }
+void fc2 (class C2);              // { dg-warning "\\\[-Wredundant-tags" }
+
+const int
+npc3 = sizeof (class C3*);        // { dg-bogus "\\\[-Wredundant-tags" }
+const int
+nppc3 = sizeof (class C3**);      // { dg-warning "\\\[-Wredundant-tags" }
+
+extern struct S1 *s1p;            // { dg-bogus "\\\[-Wredundant-tags" }
+extern struct S1 s1a[];           // { dg-warning "\\\[-Wredundant-tags" }
+
+struct S3
+{
+  struct S3 *p1s3;                // { dg-warning "\\\[-Wredundant-tags" }
+  S3 *p2s3;
+
+  union U1 *p1u1;                 // { dg-bogus "\\\[-Wredundant-tags" }
+  union U1 *p2u1;                 // { dg-warning "\\\[-Wredundant-tags" }
+} s3;
+
+typedef struct S3 TS3;            // { dg-warning "\\\[-Wredundant-tags" }
+
+typedef struct S4 TS4;
+
+struct S5
+{
+  struct S6
+  {
+  private:
+    // 'struct' is redundant in a declaration of a pointer to ::S5;
+    struct S5 *ps5;               // { dg-warning "\\\[-Wredundant-tags" }
+    // 'struct' is required in a definition of a new type.
+    struct S5 { } *ps5_2;
+    struct S5 *p2s5_2;            // { dg-warning "\\\[-Wredundant-tags" }
+  };
+};
+
+
+template <class> struct S7;
+template <class> struct S7 { };
+
+template <> struct S7<int>;
+template <> struct S7<int> { };
+struct S7<void> s7v;              // { dg-warning "\\\[-Wredundant-tags" }