On 02/22/2017 05:43 PM, Jason Merrill wrote:
On Wed, Feb 22, 2017 at 3:44 PM, Martin Sebor <mse...@gmail.com> wrote:
On 02/22/2017 11:02 AM, Jason Merrill wrote:
On Tue, Feb 21, 2017 at 4:27 PM, Martin Sebor <mse...@gmail.com> wrote:
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.
In the test case in the bug:
template <class T>
void g ()
{
T t; // warning, ok
typedef T U;
U u; // no warning, bug
}
template void g<int>();
both TREE_USED(T) and TREE_USED(t) are zero in initialize_local_var
so the function doesn't set already_used or TREE_USED(t) and we get
a warning as expected.
But because TREE_USED(U) is set to 1 in maybe_record_typedef_use
(to implement -Wunused-local-typedefs), initialize_local_var then
sets already_used to 1 and later also TREE_USED(u) to 1, suppressing
the warning.
Hmm, I would expect maybe_record_typedef_use to set TREE_USED on the
TYPE_DECL, not on the *_TYPE which initialize_local_var checks.
That's what it does:
void
maybe_record_typedef_use (tree t)
{
if (!is_typedef_decl (t))
return;
TREE_USED (t) = true;
}
Here, t is a TYPE_DECL of the typedef U.
Yes. It is a TYPE_DECL, not a type.
It has the effect of TREE_USED (TREE_TYPE (decl)) being set in
initialize_local_var. The TREE_USED bit on the type (i.e., on
TREE_TYPE(decl) where decl is the u in the test case above) is
set when the function template is instantiated, in
set_underlying_type called from tsubst_decl.
Aha! That seems like the problem. Does removing that copy of
TREE_USED from the decl to the type break anything?
As far as I can see it breaks just gcc.dg/unused-3.c which is:
typedef short unused_type __attribute__ ((unused));
void f (void)
{
unused_type y;
}
The reason is that the type is TREE_USED isn't set on the variable
(I didn't look where the bit from the type is copied into the var).,
This could be fixed by also looking at the type's attribute in this
case. That would seem to me like the better approach because it
more faithfully represents what's going on in the code. I.e., that
the variable is in fact unused, but that its type says not to complain
about it.
Let me know what you prefer.
While looking into this I noticed that regardless of this change,
the C++ front end warns on the following modified version of the
test case:
static unused_type x; // bogus -Wunused-variable
void f (void)
{
static unused_type y; // bogus -Wunused-variable
}
I opened bug 79695 for it.
Martin
