Hi, On Tue Jan 14, 2025 at 11:39 PM CET, Jason Merrill wrote: > On 1/14/25 8:25 AM, Simon Martin wrote: >> [ 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 > > Aha. It seems like a bug that ocp_convert goes on to (maybe) warn about > the conversion after giving an error about it, but it does make your point. > > The patch is OK. Just for the record, I pushed this after rebasing and retesting as r16-383-g0f1d55a75b09c1
Simon
