[ Apologies for the previous incomplete, garbled email - I used the
shortcut to send the email by mistake way too early ]
Hi Jason,
On 14 Jan 2025, at 11:43, Simon Martin wrote:
> Hi,
>
> On 14 Jan 2025, at 2:56, Jason Merrill wrote:
>
>> On 1/12/25 2:39 PM, Simon Martin wrote:
>>> [ Fixing David’s email address :-/ ]
>>>
>>> Hi,
>>>
>>> On 9 Jan 2025, at 20:08, Simon Martin wrote:
>>>
>>>> On 9 Jan 2025, at 20:00, Marek Polacek wrote:
>>>>
>>>>> On Thu, Jan 09, 2025 at 12:05:43PM -0500, 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.
>>>>>>
>>>>>> 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).
>>>>>
>>>>> FWIW, I love this auto_diagnostic_group idea.
>>>> Thanks folks, I’ll explore the auto_diagnostic_group idea (and
>>>> maybe
>>>> *also* the error returning bool one because I am not a fan of
>>>> functions
>>>> that “lie” to their callers :-))
>>>>
>>>> I’ll send a follow-up patch in the coming days.
>>> Please find attached an updated version of the patch, that
>>> implements
>>>
>>> Patrick’s idea and fixes both PR118163 and PR118392. It tracks the
>>> depth at which a warning is inhibited, and suppresses all the notes
>>> from
>>> that depth on until an error/warning is emitted or that depth is
>>> left.
>>>
>>> Successfully tested on x86_64-pc-linux-gnu. OK for GCC 16?
>>
>>> + int curr_depth = (m_diagnostic_groups.m_group_nesting_depth
>>> + + m_diagnostic_groups.m_diagnostic_nesting_level);
>>
>> Do we care about the nesting level? I'd lean toward ignoring it and
>> only considering the group.
Unfortunately we have to care about the nesting level.
I initially only considered the group, but this breaks (at least)
initlist-ctor1.C, where the following happens:
1. print_z_candidates (when group==1 and nesting==0) is called, and
calls print_error_for_call_failure
2. print_error_for_call_failure calls print_z_candidates to print 4
candidates (with group==1 and nesting==0)
3. For candidate #1 (when group==1 and nesting==3 because of some
auto_diagnostic_nesting_level sentinels) a “the result of the
conversion is unspecified” warning is suppressed, and
inhibit_notes_in_group would set m_inhibiting_notes_from to 1 (the
group)
4. From then one, all the notes emitted by print_z_candidates would
suppressed because they’re in group #1
Nesting levels and nesting groups make things a bit complicated. Take
the fictitious example at the bottom of this mail.
If the “salut” warning is suppressed, do you want to see
“hello”, “good” and “bye” (what my patch does), or
“hello”, “gcc”, “good” and “bye”? And even without the
my_super_info call, do you want to see “bye” if the “good”
warning is suppressed (you won’t with my patch)?
My patch takes the simple stance that if a warning is suppressed at
“group+nesting” depth D, all subsequent notes at that level or
deeper are suppressed until an error or a warning is accepted, or the
depth goes under D.
An alternative would be that all subsequent notes at that SPECIFIC level
are suppressed until an error / warning is accepted at that SPECIFIC
level (we can implement this by making m_inhibiting_notes_from a
collection of int instead of a single int).
A third solution, to silence all the notes in a group as soon as one
warning is suppressed in it (which might be what folks are actually
expecting of a diagnostic_group (?)), is not really viable, since we
would suppress all the notes about candidates.
I’d lean towards keeping things (relatively) simple with my patch, but
can explore the alternative (or any better idea folks might have) if
needed.
Simon
=== example ==
void foo() {
// ...
auto_diagnostic_group d;
warning (“hello”))
my_super_info ();
warning (“good”)
inform (“bye”);
}
void my_super_info() {
auto_diagnostic_nesting_level l;
// ...
if (warning (“salut”)) {
inform (“les”);
auto_diagnostic_group e;
if (warning (“amis”)) {
inform (“de”);
}
}
inform (“gcc”);
}
=== example ==
