c++: Inhibit subsequent warnings/notes in diagnostic_groups with an inhibited warning [PR118163,PR118392]

Those 2 PRs show that even when using a *single* diagnostic_group, it's
possible to end up with a warning being inhibited and its associated
note still emitted, which leads to puzzling user experience. Example
from PR118392:

===
$ gcc/cc1plus inhibit-warn-3.C -w
inhibit-warn-3.C:10:17: note: only here as a ‘friend’
   10 |     friend void bar();
      |                 ^~~
===

Following a suggestion from ppalka@, this patch keeps track of the
"diagnostic depth" at which a warning in inhibited, and makes sure that
all subsequent notes at that depth or deeper are inhibited as well,
until a subsequent warning or error is accepted at that depth, or the
diagnostic_group or nesting level is popped.

	PR c++/118163
	PR c++/118392

gcc/ChangeLog:

	* diagnostic.cc (diagnostic_context::initialize): Initialize
	m_diagnostic_groups.m_inhibiting_notes_from.
	(diagnostic_context::inhibit_notes_in_group): New.
	(diagnostic_context::notes_inhibited_in_group): New
	(diagnostic_context::report_diagnostic): Call
	inhibit_notes_in_group and notes_inhibited_in_group.
	(diagnostic_context::end_group): Call inhibit_notes_in_group.
	(diagnostic_context::pop_nesting_level): Ditto.
	* diagnostic.h (diagnostic_context::m_diagnostic_groups): Add
	member to track the depth at which a warning has been inhibited.
	(diagnostic_context::notes_inhibited_in_group): Declare.
	(diagnostic_context::inhibit_notes_in_group): Declare.
	* doc/ux.texi: Document diagnostic_group behavior with regards
	to inhibited warnings.

gcc/testsuite/ChangeLog:

	* g++.dg/diagnostic/incomplete-type-2.C: New test.
	* g++.dg/diagnostic/incomplete-type-2a.C: New test.
	* g++.dg/diagnostic/inhibit-warn-3.C: New test.

6 files changed, 110 insertions, 3 deletions

diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
index 429c4b1..c3ea1b3 100644
--- a/gcc/diagnostic.cc
+++ b/gcc/diagnostic.cc
@@ -282,6 +282,7 @@ diagnostic_context::initialize (int n_opts)
   m_diagnostic_groups.m_group_nesting_depth = 0;
   m_diagnostic_groups.m_diagnostic_nesting_level = 0;
   m_diagnostic_groups.m_emission_count = 0;
+  m_diagnostic_groups.m_inhibiting_notes_from = 0;
   m_output_sinks.safe_push
     (new diagnostic_text_output_format (*this, nullptr, true));
   m_set_locations_cb = nullptr;
@@ -917,6 +918,7 @@ diagnostic_context::check_max_errors (bool flush)
 
 /* Take any action which is expected to happen after the diagnostic
    is written out.  This function does not always return.  */
+
 void
 diagnostic_context::action_after_output (diagnostic_t diag_kind)
 {
@@ -991,6 +993,50 @@ diagnostic_context::action_after_output (diagnostic_t diag_kind)
     }
 }
 
+/* State whether we should inhibit notes in the current diagnostic_group and
+   its future children if any.  */
+
+void
+diagnostic_context::inhibit_notes_in_group (bool inhibit)
+{
+  int curr_depth = (m_diagnostic_groups.m_group_nesting_depth
+		    + m_diagnostic_groups.m_diagnostic_nesting_level);
+
+  if (inhibit)
+    {
+      /* If we're already inhibiting, there's nothing to do.  */
+      if (m_diagnostic_groups.m_inhibiting_notes_from)
+	return;
+
+      /* Since we're called via warning/error/... that all have their own
+	 diagnostic_group, we must consider that we started inhibiting in their
+	 parent.  */
+      gcc_assert (m_diagnostic_groups.m_group_nesting_depth > 0);
+      m_diagnostic_groups.m_inhibiting_notes_from = curr_depth - 1;
+    }
+  else if (m_diagnostic_groups.m_inhibiting_notes_from)
+    {
+      /* Only cancel inhibition at the depth that set it up.  */
+      if (curr_depth >= m_diagnostic_groups.m_inhibiting_notes_from)
+	return;
+
+      m_diagnostic_groups.m_inhibiting_notes_from = 0;
+    }
+}
+
+/* Return whether notes must be inhibited in the current diagnostic_group.  */
+
+bool
+diagnostic_context::notes_inhibited_in_group () const
+{
+  if (m_diagnostic_groups.m_inhibiting_notes_from
+      && (m_diagnostic_groups.m_group_nesting_depth
+	  + m_diagnostic_groups.m_diagnostic_nesting_level
+	  >= m_diagnostic_groups.m_inhibiting_notes_from))
+    return true;
+  return false;
+}
+
 /* class logical_location.  */
 
 /* Return true iff this is a function or method.  */
@@ -1381,7 +1427,10 @@ diagnostic_context::report_diagnostic (diagnostic_info *diagnostic)
   bool was_warning = (diagnostic->kind == DK_WARNING
 		      || diagnostic->kind == DK_PEDWARN);
   if (was_warning && m_inhibit_warnings)
-    return false;
+    {
+      inhibit_notes_in_group ();
+      return false;
+    }
 
   if (m_adjust_diagnostic_info)
     m_adjust_diagnostic_info (this, diagnostic);
@@ -1411,7 +1460,10 @@ diagnostic_context::report_diagnostic (diagnostic_info *diagnostic)
      not disabled by #pragma GCC diagnostic anywhere along the inlining
      stack.  .  */
   if (!diagnostic_enabled (diagnostic))
-    return false;
+    {
+      inhibit_notes_in_group ();
+      return false;
+    }
 
   if ((was_warning || diagnostic->kind == DK_WARNING)
       && ((!m_warn_system_headers
@@ -1421,6 +1473,10 @@ diagnostic_context::report_diagnostic (diagnostic_info *diagnostic)
        inlining stack (if there is one) are in system headers.  */
     return false;
 
+  if (diagnostic->kind == DK_NOTE && notes_inhibited_in_group ())
+    /* Bail for all the notes in the diagnostic_group that started to inhibit notes.  */
+    return false;
+
   if (diagnostic->kind != DK_NOTE && diagnostic->kind != DK_ICE)
     check_max_errors (false);
 
@@ -1436,6 +1492,9 @@ diagnostic_context::report_diagnostic (diagnostic_info *diagnostic)
 	error_recursion ();
     }
 
+  /* We are accepting the diagnostic, so should stop inhibiting notes.  */
+  inhibit_notes_in_group (/*inhibit=*/false);
+
   m_lock++;
 
   if (diagnostic->kind == DK_ICE || diagnostic->kind == DK_ICE_NOBT)
@@ -1769,6 +1828,8 @@ diagnostic_context::end_group ()
 	  sink->on_end_group ();
       m_diagnostic_groups.m_emission_count = 0;
     }
+  /* We're popping one level, so might need to stop inhibiting notes.  */
+  inhibit_notes_in_group (/*inhibit=*/false);
 }
 
 void
@@ -1781,6 +1842,8 @@ void
 diagnostic_context::pop_nesting_level ()
 {
   --m_diagnostic_groups.m_diagnostic_nesting_level;
+  /* We're popping one level, so might need to stop inhibiting notes.  */
+  inhibit_notes_in_group (/*inhibit=*/false);
 }
 
 void
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index 5cde881..88288d0 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -966,8 +966,14 @@ private:
     /* How many diagnostics have been emitted since the bottommost
        diagnostic_group was pushed.  */
     int m_emission_count;
+
+    /* The "group+diagnostic" nesting depth from which to inhibit notes.  */
+    int m_inhibiting_notes_from;
   } m_diagnostic_groups;
 
+  void inhibit_notes_in_group (bool inhibit = true);
+  bool notes_inhibited_in_group () const;
+
   /* The various sinks to which diagnostics are to be outputted
      (text vs structured formats such as SARIF).
      The sinks are owned by the context; this would be a
diff --git a/gcc/doc/ux.texi b/gcc/doc/ux.texi
index 2428d1a..243b3e0 100644
--- a/gcc/doc/ux.texi
+++ b/gcc/doc/ux.texi
@@ -409,7 +409,10 @@ diagnostic subsystem that all diagnostics issued within the lifetime
 of the @code{auto_diagnostic_group} are related.  For example,
 @option{-fdiagnostics-format=json} will treat the first diagnostic
 emitted within the group as a top-level diagnostic, and all subsequent
-diagnostics within the group as its children.
+diagnostics within the group as its children.  Also, if a warning in the
+group is inhibited at nesting depth D, all subsequent notes at that depth
+or deeper will be inhibited as well, until an error or another warning
+is emitted, the depth decreases below D, or the group is popped.
 
 @subsection Quoting
 Text should be quoted by either using the @samp{q} modifier in a directive
diff --git a/gcc/testsuite/g++.dg/diagnostic/incomplete-type-2.C b/gcc/testsuite/g++.dg/diagnostic/incomplete-type-2.C
new file mode 100644
index 0000000..e2fb20a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/incomplete-type-2.C
@@ -0,0 +1,7 @@
+// PR c++/118163
+// { dg-do "compile" }
+
+template<class T>
+struct S {  // { dg-note "until the closing brace" }
+  S s;	    // { dg-error "has incomplete type" }
+};
diff --git a/gcc/testsuite/g++.dg/diagnostic/incomplete-type-2a.C b/gcc/testsuite/g++.dg/diagnostic/incomplete-type-2a.C
new file mode 100644
index 0000000..d13021d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/incomplete-type-2a.C
@@ -0,0 +1,13 @@
+// PR c++/118163
+// { dg-do "compile" }
+// { dg-additional-options "-Wno-template-body" }
+
+template<class T>
+struct S {  // { dg-bogus "until the closing brace" }
+  S s;	    // { dg-bogus "has incomplete type" }
+};
+
+// Check that we don't suppress errors outside of the body.
+struct forward_decl;	    // { dg-note "forward declaration" }
+template<class T>
+void foo (forward_decl) {}  // { dg-error "has incomplete type" }
diff --git a/gcc/testsuite/g++.dg/diagnostic/inhibit-warn-3.C b/gcc/testsuite/g++.dg/diagnostic/inhibit-warn-3.C
new file mode 100644
index 0000000..bd021df
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/inhibit-warn-3.C
@@ -0,0 +1,15 @@
+// PR c++/118392
+// { dg-do "compile" }
+// { dg-additional-options "-w" }
+
+// Fake dg-note to make sure that notes are not pruned and we can use dg-bogus.
+// { dg-note "fake" "" { xfail *-*-* } }
+
+namespace xxx {
+  struct foo {
+    friend void bar(); // { dg-bogus "only here as a friend" }
+  };
+}
+void xxx::bar () {}
+
+void foo () {}