print notes only when -Wmismatched-tags succeeds (c++/96063)

Message ID 6db5e138-41d8-d472-bdef-ce380d06f1a6@redhat.com
State New
Headers show
Series
  • print notes only when -Wmismatched-tags succeeds (c++/96063)
Related show

Commit Message

Qing Zhao via Gcc-patches July 7, 2020, 12:21 a.m.
The code for -Wmismatched-tags tests warn_mismatched_tags and when
it's non-zero assumes that calls to warning_at() necessarily succeed,
proceeding to call inform() to unconditionally issue informational
notes.  This assumption is wrong when -Wmismatched-tags is enabled
(and warn_mismatched_tags is nonzero) but the instance of the warning
is for code in a system header and -Wsystem-headers is disabled.  In
that case, the warning is suppressed but the notes are still printed.
The attached near-trivial change makes the calls to inform() conditional
on the result from warning_at().

I think a more robust solution than testing global variables like
warn_mismatched_tags is to introduce a functional API to query
the result of a warning_at() call without actually issuing a warning.
The patch doesn't do that but I have a follow-up enhancement to do
just that.

Martin

Comments

Qing Zhao via Gcc-patches July 7, 2020, 2:46 a.m. | #1
On 7/6/20 8:21 PM, Martin Sebor wrote:
> The code for -Wmismatched-tags tests warn_mismatched_tags and when

> it's non-zero assumes that calls to warning_at() necessarily succeed,

> proceeding to call inform() to unconditionally issue informational

> notes.  This assumption is wrong when -Wmismatched-tags is enabled

> (and warn_mismatched_tags is nonzero) but the instance of the warning

> is for code in a system header and -Wsystem-headers is disabled.  In

> that case, the warning is suppressed but the notes are still printed.

> The attached near-trivial change makes the calls to inform() conditional

> on the result from warning_at().


OK.

Patch

Avoid printing informational notes when -Wmismatched-tags is suppressed in system headers.

Related:
PR c++/96063 - mismatched-tags warnings in stdlib headers

gcc/cp/ChangeLog:

	PR c++/96063
	* parser.c (class_decl_loc_t::diag_mismatched_tags): Print notes only
	if warning_at returns nonzero.

gcc/testsuite/ChangeLog:

	PR c++/96063
	* g++.dg/warn/Wmismatched-tags-7.C: New test.
	* g++.dg/warn/Wmismatched-tags-8.C: New test.

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 6e7637c6016..e58d8eb298c 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -31443,25 +31443,26 @@  class_decl_loc_t::diag_mismatched_tags (tree type_decl)
   /* Issue a warning for the first mismatched declaration.
      Avoid using "%#qT" since the class-key for the same type will
      be the same regardless of which one was used in the declaraion.  */
-  warning_at (loc, OPT_Wmismatched_tags,
-	      "%qT declared with a mismatched class-key %qs",
-	      type_decl, xmatchkstr);
-
-  /* Suggest how to avoid the warning for each instance since
-     the guidance may be different depending on context.  */
-  inform (loc,
-	  (key_redundant_p
-	   ? G_("remove the class-key or replace it with %qs")
-	   : G_("replace the class-key with %qs")),
-	  xpectkstr);
-
-  /* Also point to the first declaration or definition that guided
-     the decision to issue the warning above.  */
-  inform (cdlguide->location (idxguide),
-	  (def_p
-	   ? G_("%qT defined as %qs here")
-	   : G_("%qT first declared as %qs here")),
-	  type_decl, xpectkstr);
+  if (warning_at (loc, OPT_Wmismatched_tags,
+		  "%qT declared with a mismatched class-key %qs",
+		  type_decl, xmatchkstr))
+    {
+      /* Suggest how to avoid the warning for each instance since
+	 the guidance may be different depending on context.  */
+      inform (loc,
+	      (key_redundant_p
+	       ? G_("remove the class-key or replace it with %qs")
+	       : G_("replace the class-key with %qs")),
+	      xpectkstr);
+
+      /* Also point to the first declaration or definition that guided
+	 the decision to issue the warning above.  */
+      inform (cdlguide->location (idxguide),
+	      (def_p
+	       ? G_("%qT defined as %qs here")
+	       : G_("%qT first declared as %qs here")),
+	      type_decl, xpectkstr);
+    }
 
   /* Issue warnings for the remaining inconsistent declarations.  */
   for (unsigned i = idx + 1; i != ndecls; ++i)
@@ -31476,16 +31477,16 @@  class_decl_loc_t::diag_mismatched_tags (tree type_decl)
       key_redundant_p = key_redundant (i);
       /* Set the function declaration to print in diagnostic context.  */
       current_function_decl = function (i);
-      warning_at (loc, OPT_Wmismatched_tags,
-		  "%qT declared with a mismatched class-key %qs",
-		  type_decl, xmatchkstr);
-      /* Suggest how to avoid the warning for each instance since
-	 the guidance may be different depending on context.  */
-      inform (loc,
-	      (key_redundant_p
-	       ? G_("remove the class-key or replace it with %qs")
-	       : G_("replace the class-key with %qs")),
-	      xpectkstr);
+      if (warning_at (loc, OPT_Wmismatched_tags,
+		      "%qT declared with a mismatched class-key %qs",
+		      type_decl, xmatchkstr))
+	/* Suggest how to avoid the warning for each instance since
+	   the guidance may be different depending on context.  */
+	inform (loc,
+		(key_redundant_p
+		 ? G_("remove the class-key or replace it with %qs")
+		 : G_("replace the class-key with %qs")),
+		xpectkstr);
     }
 
   /* Restore the current function in case it was replaced above.  */
diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-tags-7.C b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-7.C
new file mode 100644
index 00000000000..3180c22c054
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-7.C
@@ -0,0 +1,13 @@ 
+/* Verify that -Wmismatched-tags doesn't print stray notes for warnings
+   disabled in system headers.
+  { dg-do "compile" }
+  { dg-options "-Wmismatched-tags" } */
+
+# 6 "Wmismatched-tags-7.C" 1
+# 1 "system-header.h" 1 3 4
+# 9 "system-header.h" 3 4
+class A;            // { dg-bogus "first declared" }
+struct A;           // { dg-bogus "replace" }
+# 12 "Wmismatched-tags-7.C" 2
+class B;            // { dg-message "first declared" }
+struct B;           // { dg-warning "\\\[-Wmismatched-tags" }
diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-tags-8.C b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-8.C
new file mode 100644
index 00000000000..0ebca3db90f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-8.C
@@ -0,0 +1,22 @@ 
+/* Verify that #pragma GCC diagnostic works for -Wmismatched-tags.
+   { dg-do "compile" }
+   { dg-options "-Wmismatched-tags" } */
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic error "-Wmismatched-tags"
+class A;            // { dg-message "first declared"
+struct A;           // { dg-error "\\\[-Werror=mismatched-tags" }
+
+#pragma GCC diagnostic ignored "-Wmismatched-tags"
+class B;            // { dg-bogus "first declared" }
+struct B;
+
+#pragma GCC diagnostic warning "-Wmismatched-tags"
+class C;            // { dg-message "first declared"
+struct C;           // { dg-warning "\\\[-Wmismatched-tags" }
+#pragma GCC diagnostic pop
+
+class D;            // { dg-message "first declared"
+struct D;           // { dg-warning "\\\[-Wmismatched-tags" }
+
+// { dg-prune-output "some warnings being treated as errors" }