On 06/28/2010 08:27 AM, Nathan Froyd wrote:
>> defarg8.C:8:23: error: 'i' was not declared in this scope>> Ping. Where does this patch stand? Should I:>> - Commit the original patch?>> - Hold off while you consider what might be wrong with your suggestion?> and/or>> - Debug your suggestion myself in hopes of figuring out what's wrong?
The last, if you would. Is DECL_FRIEND_CONTEXT not getting set for f?
Jason

On 06/30/2010 03:10 PM, Nathan Froyd wrote:
> On Wed, Jun 30, 2010 at 02:33:20PM -0400, Jason Merrill wrote:>> On 06/28/2010 08:27 AM, Nathan Froyd wrote:>>> Ping. Where does this patch stand? Should I:>>>>>> - Commit the original patch?>>>>>> - Hold off while you consider what might be wrong with your suggestion?>>> and/or>>>>>> - Debug your suggestion myself in hopes of figuring out what's wrong?>>>> The last, if you would. Is DECL_FRIEND_CONTEXT not getting set for f?>> Apparently not:>> (gdb) p decl->decl_minimal.context> $4 = (tree) 0x0
That's DECL_CONTEXT, not DECL_FRIEND_CONTEXT; the latter would be
decl->decl_common.lang_specific->u.fn.context, I believe.
In any case, the problem seems to be in do_friend:
> /* Friends must all go through the overload machinery,> even though they may not technically be overloaded.>> Note that because classes all wind up being top-level> in their scope, their friend wind up in top-level scope as well. */> if (funcdef_flag)> SET_DECL_FRIEND_CONTEXT (decl, current_class_type);
What happens if you remove the if?
Jason

On Wed, Jun 30, 2010 at 03:40:13PM -0400, Jason Merrill wrote:
> On 06/30/2010 03:10 PM, Nathan Froyd wrote:>> On Wed, Jun 30, 2010 at 02:33:20PM -0400, Jason Merrill wrote:>>> The last, if you would. Is DECL_FRIEND_CONTEXT not getting set for f?>>>> Apparently not:>>>> (gdb) p decl->decl_minimal.context>> $4 = (tree) 0x0>> That's DECL_CONTEXT, not DECL_FRIEND_CONTEXT; the latter would be > decl->decl_common.lang_specific->u.fn.context, I believe.
Whoops, sorry about that. Fortunately that's not getting set either. :)
> In any case, the problem seems to be in do_friend:>>> /* Friends must all go through the overload machinery,>> even though they may not technically be overloaded.>>>> Note that because classes all wind up being top-level>> in their scope, their friend wind up in top-level scope as well. */>> if (funcdef_flag)>> SET_DECL_FRIEND_CONTEXT (decl, current_class_type);>> What happens if you remove the if?
If I set funcdef_flag to true at the start of do_friend when f is
getting defined, things work as expected. grokdeclarator attempts to
grok f in FIELD context, which explains why funcdef_flag doesn't get set
in grokdeclarator.
What's the right fix? Presumably the test shouldn't be removed in
do_friend...should grokdeclarator be checking declarator->kind?
-Nathan

On 06/30/2010 03:55 PM, Nathan Froyd wrote:
>>> /* Friends must all go through the overload machinery,>>> even though they may not technically be overloaded.>>>>>> Note that because classes all wind up being top-level>>> in their scope, their friend wind up in top-level scope as well. */>>> if (funcdef_flag)>>> SET_DECL_FRIEND_CONTEXT (decl, current_class_type);>>>> What happens if you remove the if?>> If I set funcdef_flag to true at the start of do_friend when f is> getting defined, things work as expected. grokdeclarator attempts to> grok f in FIELD context, which explains why funcdef_flag doesn't get set> in grokdeclarator.
And it shouldn't get set, because f is not being defined, only declared.
> What's the right fix? Presumably the test shouldn't be removed in> do_friend...should grokdeclarator be checking declarator->kind?
Why presumably? The comment for DECL_FRIEND_CONTEXT doesn't specify
that it's only set for definitions.
But I would be sure to test that we give the appropriate errors if you
have a friend declaration followed by a global definition that refers to
static members of the type without explicit qualification.
Jason

Jason Merrill wrote:
>> What's the right fix? Presumably the test shouldn't be removed in>> do_friend...should grokdeclarator be checking declarator->kind?> > Why presumably? The comment for DECL_FRIEND_CONTEXT doesn't specify> that it's only set for definitions.
Indeed, but I think it should specify that, probably. For example,
push_access_scope honors it -- and that only makes sense for
definitions, I think?
Also, what about duplicate_decls? If we merge two declarations of the
same function, declared as a friend in two different classes, what
should DECL_FRIEND_CONTEXT be?

On 07/01/2010 01:09 AM, Mark Mitchell wrote:
> Jason Merrill wrote:>>>> What's the right fix? Presumably the test shouldn't be removed in>>> do_friend...should grokdeclarator be checking declarator->kind?>>>> Why presumably? The comment for DECL_FRIEND_CONTEXT doesn't specify>> that it's only set for definitions.>> Indeed, but I think it should specify that, probably. For example,> push_access_scope honors it -- and that only makes sense for> definitions, I think?>> Also, what about duplicate_decls? If we merge two declarations of the> same function, declared as a friend in two different classes, what> should DECL_FRIEND_CONTEXT be?
Right. I was thinking that we could clear DECL_FRIEND_CONTEXT after
we're done processing the class, but I guess it's safer to just update
the comment and go with Nathan's earlier patch.
Jason