[PATCH] D12375: [PATCH] Relax parse ordering rules for attributes

[Aaron Ballman via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, hans.
aaron.ballman added a subscriber: cfe-commits.

PR24559 brings up a long-standing issue in Clang where attribute order with differing syntax impacts whether a parse will succeed or fail with poor diagnostics. For instance:

struct __attribute__((lockable)) __declspec(dllexport) S { }; // ok
struct __declspec(dllexport) __attribute__((lockable)) T { }; // error about declaring anonymous structs and a warning about declarations not declaring anything

When you consider attributes as extraneous semantic information to attach to entities, ordering of the attributes really should not matter between the various attribute syntaxes we support -- they all serve the same purpose.

This patch begins to address the issue by providing a MaybeParseAttributes() function accepting a mask of what attribute syntaxes are allowed and parses the various syntaxes in a loop.

However, this patch is not complete, as the test cases for function attributes currently fail for the C++11-style attributes.

[[noreturn]] __attribute__((cdecl)) __declspec(dllexport) void f();

In this case, the [[noreturn]] is parsed as part of the top-level declaration, while the GNU and declspec attributes are parsed as part of the declaration specifier, and everything is accepted.

__attribute__((cdecl)) [[noreturn]] __declspec(dllexport) void f();

In this case, nothing is parsed as part of the top-level declaration, and everything is parsed as part of the declaration specifier. However, the C++ attribute is prohibited from appearing on a declaration specifier and so is diagnosed.

I think ParseDeclarationSpecifiers() needs to accept a ParsedAttributesWithRange object representing the top-level declaration attributes, and the parser needs to attach the C++ attributes to it up until we the first non-attribute token is found. But I'm not 100% certain that's the right approach.

Before taking the process further, I was wondering about two things:

1) Is the direction with MaybeParseAttributes() an acceptable approach?
2) If it is, is it reasonable to commit that, and work on the ParseDeclarationSpecifiers() portion in a follow-up commit?

Thanks!

~Aaron

http://reviews.llvm.org/D12375

Files:
  include/clang/Parse/Parser.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/Parser.cpp
  test/Parser/attr-order.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D12375.33221.patch
Type: text/x-patch
Size: 12678 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150826/e6372ef9/attachment-0001.bin>