Likely problem in attribute parsing

[John McCall]

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. :)

John.