Re: [PATCH] restore -Wunused-variable on a typedef'd variable in a function template (PR 79548)

[Jason Merrill <jason@redhat.com>]

On Tue, Feb 21, 2017 at 11:00 AM, Martin Sebor <msebor@gmail.com> wrote:
> On 02/21/2017 11:08 AM, Jason Merrill wrote:
>>
>> On 02/17/2017 05:07 PM, Martin Sebor wrote:
>>>
>>>     * decl.c (poplevel): Avoid diagnosing entities declared with
>>>     attribute unused.
>>
>>
>> This change is OK.
>>
>>>     (initialize_local_var): Do not consider the type of a variable
>>>     when determining whether or not it's used.
>>
>>
>> This is not; the documentation for attribute unused says,
>>
>> When attached to a type (including a @code{union} or a @code{struct}),
>> this attribute means that variables of that type are meant to appear
>> possibly unused.  GCC does not produce a warning for any variables of
>> that type, even if the variable appears to do nothing.  This is often
>> the case with lock or thread classes, which are usually defined and then
>> not referenced, but contain constructors and destructors that have
>> nontrivial bookkeeping functions.
>>
>> So a TREE_USED type should imply TREE_USED on variables of that type.
>
> Yes, I'm familiar with the effect of the attribute on types but
> in my testing this change doesn't affect how it works (i.e., it
> passes a full bootstrap and regression tests and I haven't been
> able to construct a failing test case.)  It looks like that's
> because TREE_USED(decl) is already set here for a decl whose
> type is declared attribute used.
>
> While trying to come up with a test case to exercise the scenario
> you describe I see that current trunk doesn't handle it correctly
> but the patch just happens to fix that too.  For example:
>
> template <class T>
> void f ()
> {
>   T t;   // trunk warns for f<int> (good)
>
>   typedef T U;
>   U u;   // trunk doesn't warn for f<int> (bug 79548)
> }
>
> template void f<int>();
>
> struct __attribute__ ((unused)) S { };
>
> template void f<S>();   // no warnings here (good)
>
> void g ()
> {
>   S s;
>
>   typedef S T;
>   T t;   // trunk warns here (new bug), doesn't with the patch
> }
>
> In the test case above, TREE_USED (TREE_TYPE (decl)) is set for
> t in g() so trunk sets it on t too and warbs.  The patch doesn't
> and so it doesn't warn as expected.
>
> But it's entirely possible I'm missing a case.  Do you have
> a suggestion for a test that trunk handles correctly but that
> fails with the patch?

Ah, I see, your patch changes attribute unused handling for local
variables from tracking TREE_USED to lookup_attribute.  I'm not
opposed to this change, but I'd like to understand why the TREE_USED
handling wasn't working.

Jason