On Tue, Feb 21, 2017 at 11:00 AM, Martin Sebor <mse...@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
