Likely problem in attribute parsing

[Aaron Ballman]

On Thu, Feb 27, 2014 at 7:51 PM, John McCall <rjmccall at apple.com> wrote:
> On Feb 27, 2014, at 3:45 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> On Thu, Feb 27, 2014 at 5:17 PM, Abramo Bagnara
>> <abramo.bagnara at bugseng.com> wrote:
>>> $ cat z.c
>>> void __attribute__((noinline,noreturn))
>>> f(void);
>>> $ _clang -cc1 -ast-dump z.c
>>> TranslationUnitDecl 0x6630730 <<invalid sloc>>
>>> |-TypedefDecl 0x6630c30 <<invalid sloc>> __int128_t '__int128'
>>> |-TypedefDecl 0x6630c90 <<invalid sloc>> __uint128_t 'unsigned __int128'
>>> |-TypedefDecl 0x6630fe0 <<invalid sloc>> __builtin_va_list
>>> '__va_list_tag [1]'
>>> `-FunctionDecl 0x6631120 <z.c:1:1, line:2:7> f 'void (void)
>>> __attribute__((noreturn))'
>>>  |-NoInlineAttr 0x66311c0 <line:1:21>
>>>  `-NoInlineAttr 0x6631200 <col:21>
>>> $ clang -cc1 -ast-print z.c
>>> void f() __attribute__((noinline)) __attribute__((noinline));
>>>
>>> Can you confirm that the doubled attribute noinline should be considered
>>> a bug?
>>
>> From what I can tell, this is happening because of the position and
>> order of the attributes. They're positioned as type attributes, and
>> noreturn is treated as one, but noinline is treated as a declaration
>> attribute. So noreturn is being spliced out onto the declspec, while
>> noinline remains on the function type, as it should. However, we save
>> the declspec attributes in distributeFunctionTypeAttrFromDeclSpec, so
>> what we spliced out gets restored later -- so the attributes
>> effectively get duplicated.
>>
>> By disallowing the saveDeclSpecAttrs(), the issue is resolved and all
>> regression tests pass, but I do not understand the rationale as to why
>> they were being saved in the first place. The only other usage of
>> saveDeclSpecsAttrs() is with ObjC pointer type attributes, prior to
>> performing a moveAttrFromListToList. However, removing the call to
>> saveDeclSpecAttrs() there also had no effect on the regression tests.
>> John -- it looks like you added this functionality. Can you help me to
>> understand what's going on?
>
> I expect it’s probably an attempt to ensure that attributes in the decl spec
> are applied to all the declarations in a group.  Sounds like there’s a bug,
> though, and then a second bug if we’re not testing it. :)

Yup, that appears to be exactly the case. I'll add a test case for it,
and see if I can solve the bug. Thank you for the help!

~Aaron