On Wed, 8 Jan 2025, Jason Merrill wrote:
> On 12/21/24 11:35 AM, Simon Martin wrote:
> > When erroring out due to an incomplete type, we add a contextual note
> > about the type. However, when the error is suppressed by
> > -Wno-template-body, the note remains, making the compiler output quite
> > puzzling.
> >
> > This patch makes sure the note is suppressed if we're processing a
> > template declaration body with -Wno-template-body.
> >
> > Successfully tested on x86_64-pc-linux-gnu.
> >
> > PR c++/118163
> >
> > gcc/cp/ChangeLog:
> >
> > * cp-tree.h (get_current_template): Declare.
> > * error.cc (get_current_template): Make non static.
> > * typeck2.cc (cxx_incomplete_type_inform): Suppress note when
> > parsing a template declaration with -Wno-template-body.
>
> I think rather than adding this sort of thing in lots of places where an error
> is followed by an inform, we should change error to return bool like other
> diagnostic functions, and check its return value before calling
> cxx_incomplete_type_inform or plain inform. This likely involves the same
> number of changes, but they should be smaller.
>
> Patrick, what do you think?
That makes sense to me, it's consistent with the 'warning' API and how
we handle issuing a warning followed by a note. But since the
-Wtemplate-body mechanism is really only useful for compiling legacy
code where you don't really care about any diagnostics anyway, and
the intended way to use it is -fpermissive / -Wno-error=template-body
rather than -Wno-template-body, I'd prefer a less invasive solution that
doesn't change the API of 'error' if possible.
I wonder if we can work around this by taking advantage of the fact that
notes that follow an error are expected to be linked via an active
auto_diagnostic_group? Roughly, if we issued a -Wtemplate-body
diagnostic from an active auto_diagnostic_group then all other
diagnostics from that auto_diagnostic_group should also be associated
with -Wtemplate-body, including notes. That way -Wno-template-body will
effectively suppress subsequent notes followed by an eligible error, and
no 'error' callers need to be changed (unless to use
auto_diagnostic_group).
Another simpler approach, maybe for templates that have already been
deemed erroneous, and -Wno-template-body is active, we could suppress
all subsequent diagnostics, including warnings/notes.
>
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/diagnostic/incomplete-type-2.C: New test.
> > * g++.dg/diagnostic/incomplete-type-2a.C: New test.
> >
> > ---
> > gcc/cp/cp-tree.h | 1 +
> > gcc/cp/error.cc | 2 +-
> > gcc/cp/typeck2.cc | 6 ++++++
> > gcc/testsuite/g++.dg/diagnostic/incomplete-type-2.C | 7 +++++++
> > .../g++.dg/diagnostic/incomplete-type-2a.C | 13 +++++++++++++
> > 5 files changed, 28 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/g++.dg/diagnostic/incomplete-type-2.C
> > create mode 100644 gcc/testsuite/g++.dg/diagnostic/incomplete-type-2a.C
> >
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index 6de8f64b5ee..52f954b63d9 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -7297,6 +7297,7 @@ struct decl_location_traits
> > typedef hash_map<tree, location_t, decl_location_traits>
> > erroneous_templates_t;
> > extern GTY((cache)) erroneous_templates_t *erroneous_templates;
> > +extern tree get_current_template ();
> > extern bool cp_seen_error ();
> > #define seen_error() cp_seen_error ()
> > diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
> > index 8c0644fba7e..7fd03dd6d12 100644
> > --- a/gcc/cp/error.cc
> > +++ b/gcc/cp/error.cc
> > @@ -197,7 +197,7 @@ class cxx_format_postprocessor : public
> > format_postprocessor
> > /* Return the in-scope template that's currently being parsed, or
> > NULL_TREE otherwise. */
> > -static tree
> > +tree
> > get_current_template ()
> > {
> > if (scope_chain && in_template_context && !current_instantiation ())
> > diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc
> > index fce687e83b3..505c143dae7 100644
> > --- a/gcc/cp/typeck2.cc
> > +++ b/gcc/cp/typeck2.cc
> > @@ -273,6 +273,12 @@ cxx_incomplete_type_inform (const_tree type)
> > if (!TYPE_MAIN_DECL (type))
> > return;
> > + /* When processing a template declaration body, the error generated by
> > the
> > + caller (if any) might have been suppressed by -Wno-template-body. If
> > that
> > + is the case, suppress the inform as well. */
> > + if (!warn_template_body && get_current_template ())
> > + return;
> > +
> > location_t loc = DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (type));
> > tree ptype = strip_top_quals (CONST_CAST_TREE (type));
> > 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 00000000000..e2fb20a4ae8
> > --- /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 00000000000..d13021d0b68
> > --- /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" }
>
>
