Likely problem in attribute parsing

[Aaron Ballman]

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?

Thanks!

~Aaron