On 9 Jan 2025, at 18:05, Patrick Palka wrote:
> 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.
What I like in Jason’s suggestion is that even though it’s a bit
more invasive, you know exactly what you get when you call error.
Right now you know in >99% of the cases and it’s great. But when the
diagnostics machinery does changes under you, the UX becomes weird for
the end-user (this PR) or the GCC developer (see PR 118388 with
seen_error and permerror).
This PR is clearly not a burning issue, and it’s fine to only fix it
in GCC 16 with a more robust fix.
>
> 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" }
>>
>>
