On Tue, Mar 21, 2017 at 2:40 PM, Martin Sebor <mse...@gmail.com> wrote:
> On 03/20/2017 03:11 PM, Jason Merrill wrote:
>>
>> On Thu, Feb 23, 2017 at 6:33 PM, Jason Merrill <ja...@redhat.com> wrote:
>>>
>>> On Thu, Feb 23, 2017 at 12:56 PM, Martin Sebor <mse...@gmail.com> wrote:
>>>>
>>>> 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:
>>>
>>>
>>>>>> 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;
>>>> }
>>>
>>>
>>> Ah. So the problem here is that we're using TREE_USED on a TYPE_DECL
>>> for two things: to track whether the typedef has been used, and to
>>> determine whether to treat a variable of that type as already used.
>>> Normally this isn't too much of a problem because we copy the
>>> TREE_USED flag from decl to type before there could be any uses, but
>>> in a template I guess we mark the typedef within the template when it
>>> is used, and then when we instantiate the typedef we incorrectly pass
>>> that mark along to the type.
>>>
>>> Your patch deals with this by ignoring TREE_USED on the type, so it
>>> doesn't matter that it's wrong. Another approach would be to correct
>>> the setting of TREE_USED by setting it based on looking up the unused
>>> attribute, rather than copying it from the TYPE_DECL. So also going
>>> back to the attribute, but in set_underlying_type rather than
>>> poplevel.
>>
>> Thoughts?
>
> I'm not 100% sure I understand what changes you're suggesting
> but the attached patch does what I think you're after. Is that
> what you had in mind?
Looks good.
Jason
