Hi,
On 30 Oct 2024, at 11:44, Simon Martin wrote:
> Friendly ping.
Friendly ping.
Thanks! Simon
>
> On 16 Oct 2024, at 17:43, Simon Martin wrote:
>
>> Hi Jason,
>>
>> On 12 Oct 2024, at 4:51, Jason Merrill wrote:
>>
>>> On 10/11/24 7:02 AM, Simon Martin wrote:
>>>> Hi Jason,
>>>>
>>>> On 11 Oct 2024, at 0:35, Jason Merrill wrote:
>>>>
>>>>> On 10/7/24 3:35 PM, Simon Martin wrote:
>>>>>> On 7 Oct 2024, at 18:58, Jason Merrill wrote:
>>>>>>> On 10/7/24 11:27 AM, Simon Martin wrote:
>>>>>
>>>>>>>> /* Now give a warning for all base functions without overriders,
>>>>>>>> as they are hidden. */
>>>>>>>> for (tree base_fndecl : base_fndecls)
>>>>>>>> + {
>>>>>>>> + if (!base_fndecl || overriden_base_fndecls.contains
>>>>>>>> (base_fndecl))
>>>>>>>> + continue;
>>>>>>>> + tree *hider = hidden_base_fndecls.get (base_fndecl);
>>>>>>>> + if (hider)
>>>>>>>
>>>>>>> How about looping over hidden_base_fndecls instead of base_fndecls?
>>>>>
>>>>>> Unfortunately it does not work because a given base method can be
>>>>>> hidden
>>>>>> by one overload and overriden by another, in which case we don’t
>>>>>> want
>>>>>> to warn (see for example AA:foo(int) in Woverloaded-virt7.C). So we
>>
>>>>>> need
>>>>>> to take both collections into account.
>>>>>
>>>>> Yes, you'd still need to check overridden_base_fndecls.contains, but
>>
>>>>> that doesn't seem any different iterating over hidden_base_fndecls
>>>>> instead of base_fndecls.
>>>> Sure, and I guess iterating over hidden_base_fndecls is more coherent
>>
>>>>
>>>> with what the warning is about. Changed in the attached updated patch,
>>>> successfully tested on x86_64-pc-linux-gnu. OK?
>>>
>>> OK, thanks.
>> As you know the patch had to be reverted due to PR117114, that
>> highlighted a bunch of issues with comparing DECL_VINDEXes: it might
>> give false positives in case of multiple inheritance (the case in
>> PR117114), but also if there’s single inheritance by the hierarchy has
>> more than two levels (another issue I found while bootstrapping with
>> rust enabled).
>>
>> The attached updated patch introduces an overrides_p function, based on
>> the existing check_final_overrider, and uses it when the signatures match.
>>
>> It’s been successfully tested on x86_64-pc-linux-gnu, and bootstrap
>> works fine with —enable-languages=all (and rust properly configured, so
>> included here). OK for trunk?
>>
>> Thanks, Simon
