On 2/5/19 8:58 PM, Alexandre Oliva wrote:
On Feb  5, 2019, Jason Merrill <ja...@redhat.com> wrote:

On Tue, Feb 5, 2019 at 1:37 AM Alexandre Oliva <aol...@redhat.com> wrote:
On Jan 31, 2019, Jason Merrill <ja...@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

Reply via email to