Hi,
On 11 Nov 2024, at 20:36, Simon Martin wrote:
> Hi,
>
> On 30 Oct 2024, at 11:44, Simon Martin wrote:
>
>> Friendly ping.
> Friendly ping.
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
