Re: [PATCH] Fix PR c/6343 (was: Re: GCC 3.1 Prerelease)

[Jason Merrill <jason@redhat.com>]

>>>>> "Franz" == Franz Sirl <Franz.Sirl-kernel@lauterbach.com> writes:

> On Sunday 28 April 2002 23:57, Jason Merrill wrote:

>> I intended that TREE_SYMBOL_REFERENCED should be used rather than
>> TREE_USED, not in addition.  I'd probably check DECL_ASSEMBLER_NAME_SET_P
>> here, too; if the name hasn't been generated, it hasn't been referenced.
>> Perhaps we want a new macro, say
>> 
>> #define DECL_SYMBOL_REFERENCED(DECL) \
>> (DECL_ASSEMBLER_NAME_SET_P (DECL) \
>> && TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (DECL)))

> But this would warn for these cases, which I think isn't correct:

> // case 1
> extern void * ffoo1g (void);
> extern void * ffoox1g (void);
> extern void * ffoo1g (void)  __attribute__((weak, alias ("ffoox1g")));
> void * foo1g (void)
> {
>   if (ffoo1g)
>     ffoo1g ();
>   return 0;
> }

I don't see why this would get a warning.

> // case 2
> extern int vfoo1i __attribute__((weak));
> void * foo1i (void)
> {
>   return (void *)&vfoo1i;
> }
> extern int vfoo1i __attribute__((weak));
> extern int vfoo1i;

I suppose this (and case 3) might get a warning from calling declare_weak
on the later decls.  It seems to me that we don't want to call declare_weak
on newdecl, since we're only going to throw it away.  Adding it to
weak_decls prevents gc from discarding it.

> Actually, looking at handle_alias_attribute(), TREE_USED is exactly the
> flag to test here.

I suppose that if there's a reference which hasn't been emitted yet, we
could get a false negative here.  So yes, TREE_USED is right for
declare_weak, and not TREE_SYMBOL_REFERENCED.

My patch actually used TREE_SYMBOL_REFERENCED in weak_finish, at which
point all references should have been emitted.  Of course, if you make
that change while all the duplicate decls go on weak_decls, we'd end up
weakening the symbol once for each decl.

> The addition of DECL_ASSEMBLER_NAME_SET_P seems reasonable though.

If we aren't looking at DECL_ASSEMBLER_NAME, there's no reason to look at
_SET_P.

So never mind.  If you want to tweak the patch to avoid duplication on
weak_decls, that might be good for the trunk, but your last patch should be
fine for the branch.

Thanks,
Jason