[C++PATCH] [PR86379] do not use TREE_TYPE for USING_DECL_SCOPE

[Jason Merrill]

On 2/5/19 8:58 PM, Alexandre Oliva wrote:
> On Feb  5, 2019, Jason Merrill <jason@redhat.com> wrote:
> 
>> On Tue, Feb 5, 2019 at 1:37 AM Alexandre Oliva <aoliva@redhat.com> wrote:
>>> On Jan 31, 2019, Jason Merrill <jason@redhat.com> wrote:
>>>
>>>> Let's use strip_using_decl instead
>>>
>>> Aah, nice!  Thanks, I'll make the changes, test them, and post a new patch.
>>>
>>>
>>>>> @@ -13288,7 +13295,8 @@ grok_special_member_properties (tree decl)
>>>>> {
>>>>> tree class_type;
>>>>> -  if (!DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
>>>>> +  if (TREE_CODE (decl) != USING_DECL
>>>>> +      && !DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
>>>>> return;
>>>
>>>> Is there a reason not to use it here, as well?
>>>
>>> The using decl will take us to a member of a different class, and this
>>> function takes the DECL_CONTEXT of decl and adjusts the properties of
>>> that class.  If we followed USING_DECLs in decl that early, we'd adjust
>>> (again?) the member properties of USING_DECL_SCOPE(original using decl),
>>> rather than of DECL_CONTEXT (original using decl) as intended.
> 
>> But a little further in, copy_fn_p will also check
>> DECL_NONSTATIC_MEMBER_FUNCTION_P and crash.
> 
> It would crash if I hadn't adjusted it to test for USING_DECLs, yes.
> 
>> This function used to do nothing for USING_DECL (since its TREE_TYPE
>> wasn't METHOD_TYPE), right?  It should continue to do nothing.
> 
> I thought I was fixing bugs making certain functions follow USING_DECLs
> rather than fail to do what it should, because the test on TREE_TYPE
> happened to not pass.  This one I was not so sure about, for the reasons
> above.  I guess it makes sense to return early if it really shouldn't do
> anything for a USING_DECL, even if it happens to resolve to a method
> that it would otherwise handle if declared directly in the class.
> 
>> Come to think of it, how about fixing DECL_NONSTATIC_MEMBER_FUNCTION_P
>> to be false for non-functions rather than messing with all these
>> places that use it?
> 
> I considered that at first, but it seemed to me that this would not
> bring about the desired behavior in the first functions I touched
> (lookup_member vs shared_member), and I thought we'd be better off
> retaining some means to catch incorrect uses of this and other macros on
> USING_DECLs, so that we can analyze and fix the uses instead of getting
> accidental behavior, correct or not.

OK, that makes sense; it isn't always clear what the right handling of a 
USING_DECL is.  In grok_special_member_properties we want to ignore them 
(as you do) because the handling there only applies to immediate 
members, and any inherited properties will be handled when processing 
the bases.

In protected_accessible_p and shared_member_p, if we're left with a 
USING_DECL after strip_using_decl, we can't give a meaningful answer, 
and should probably abort; we shouldn't get here with a dependent 
expression.

The other two changes look fine.

Jason