On Fri, Dec 15, 2017 at 3:11 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Fri, Dec 15, 2017 at 03:02:50PM -0500, Jason Merrill wrote:
>> On 12/07/2017 11:45 AM, Jakub Jelinek wrote:
>> > save_template_attributes ignored flags, when ATTR_FLAG_TYPE_IN_PLACE
>> > wasn't set on a type, it would happily attach the attributes to some
>> > existing type (in this case to integer_type_node).
>> >
>> > My first approach was to just call build_type_attribute_variant, but
>> > that ICEs on g++.dg/cpp0x/alias-decl-59.C, because there *decl_p is
>> > UNDERLYING_TYPE, which the generic type_hash_canon
>> > build_type_attribute_variant calls doesn't like.
>>
>> Ah, because it calls layout_type. What if we did this?
>>
>> Jason
>
>> diff --git a/gcc/tree.c b/gcc/tree.c
>> index ed1852b3e66..4883b711624 100644
>> --- a/gcc/tree.c
>> +++ b/gcc/tree.c
>> @@ -6445,7 +6445,8 @@ type_hash_canon (unsigned int hashcode, tree type)
>>
>> /* The TYPE_ALIGN field of a type is set by layout_type(), so we
>> must call that routine before comparing TYPE_ALIGNs. */
>> - layout_type (type);
>> + if (TREE_CODE (type) < NUM_TREE_CODES)
>> + layout_type (type);
>>
>> in.hash = hashcode;
>> in.type = type;
>
> I think that can't be sufficient, because type_cache_hasher::equal
> has:
> switch (TREE_CODE (a->type))
> {
> ...
> default:
> return 0;
> }
>
> if (lang_hooks.types.type_hash_eq != NULL)
> return lang_hooks.types.type_hash_eq (a->type, b->type);
>
> return 1;
> }
>
> so for types it doesn't know about it will just always return 0.
> Or is that what we want for the FE specific types?
>
> Another possibility would be to return 0; for default only if
> lang_hooks.types.type_hash_eq is NULL, and otherwise defer to
> the langhook, plus changing the C++ and Ada langhooks to do
> something with them if needed.
The latter sounds right to me. It's surprising that this hasn't been a
problem before.
But if you don't feel like messing with this now, your patch is OK,
just add a FIXME comment about why the else is necessary.
Jason
