[PATCH] D95459: Add helper functionality for parsing different attribute syntaxes in arbitrary order

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

In D95459#2523857 , @dblaikie wrote:

> Looks good to me! (welcome to wait for more eyes, if you like)

Thanks for the review! I think this is safe enough to land and I can deal with 
any concerns post-commit, so I've committed in 
9f2c7effd7f386e95aff3358500bc30974d35b0d 



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95459/new/

https://reviews.llvm.org/D95459

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95459: Add helper functionality for parsing different attribute syntaxes in arbitrary order

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good to me! (welcome to wait for more eyes, if you like)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95459/new/

https://reviews.llvm.org/D95459

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95459: Add helper functionality for parsing different attribute syntaxes in arbitrary order

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, dblaikie, jyknight, rjmccall.
aaron.ballman requested review of this revision.

In Clang today, we parse the different attribute syntaxes (`__attribute__`, 
`__declspec`, and `[[]]`) in a fairly rigid order. This leads to confusion for 
users when they guess the order incorrectly, and leads to bug reports like 
PR24559 or necessitates changes like D94788 .

This patch adds a helper function to allow us to more easily parse attributes 
in arbitrary order, and then updates all of the places where we would parse two 
or more different syntaxes in a rigid order to use the helper method. The patch 
does not attempt to handle Microsoft attributes (`[]`) because those are 
ambiguous with other code constructs and we don't have any attributes that use 
the syntax.

There may be other places that could be modified to start accepting attributes 
with more arbitrary orders (such as in lambda expressions), but I think that 
effort is best left to follow-up work as we notice a need.


https://reviews.llvm.org/D95459

Files:
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/test/Parser/attr-order.cpp
  clang/test/SemaOpenCL/address-spaces.cl

Index: clang/test/SemaOpenCL/address-spaces.cl
===
--- clang/test/SemaOpenCL/address-spaces.cl
+++ clang/test/SemaOpenCL/address-spaces.cl
@@ -256,7 +256,8 @@
 
 void func_multiple_addr2(void) {
   typedef __private int private_int_t;
-  __private __attribute__((opencl_global)) int var1;   // expected-error {{multiple address spaces specified for type}}
+  __private __attribute__((opencl_global)) int var1;   // expected-error {{multiple address spaces specified for type}} \
+   // expected-error {{function scope variable cannot be declared in global address space}}
   __private __attribute__((opencl_global)) int *var2;  // expected-error {{multiple address spaces specified for type}}
   __attribute__((opencl_global)) private_int_t var3;   // expected-error {{multiple address spaces specified for type}}
   __attribute__((opencl_global)) private_int_t *var4;  // expected-error {{multiple address spaces specified for type}}
Index: clang/test/Parser/attr-order.cpp
===
--- /dev/null
+++ clang/test/Parser/attr-order.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+
+struct [[]] __attribute__((lockable)) __declspec(dllexport) A {}; // ok
+struct [[]] __declspec(dllexport) __attribute__((lockable)) B {}; // ok
+struct [[]] [[]] __declspec(dllexport) __attribute__((lockable)) C {}; // ok
+struct __declspec(dllexport) [[]] __attribute__((lockable)) D {}; // ok
+struct __declspec(dllexport) __attribute__((lockable)) [[]] E {}; // ok
+struct __attribute__((lockable)) __declspec(dllexport) [[]] F {}; // ok
+struct __attribute__((lockable)) [[]] __declspec(dllexport) G {}; // ok
+struct [[]] __attribute__((lockable)) [[]] __declspec(dllexport) H {}; // ok
+
+[[noreturn]] __attribute__((cdecl)) __declspec(dllexport) void a(); // ok
+[[noreturn]] __declspec(dllexport) __attribute__((cdecl)) void b(); // ok
+[[]] [[noreturn]] __attribute__((cdecl)) __declspec(dllexport) void c(); // ok
+
+// [[]] attributes before a declaration must be at the start of the line.
+__declspec(dllexport) [[noreturn]] __attribute__((cdecl)) void d(); // expected-error {{an attribute list cannot appear here}}
+__declspec(dllexport) __attribute__((cdecl)) [[noreturn]] void e(); // expected-error {{an attribute list cannot appear here}}
+__attribute__((cdecl)) __declspec(dllexport) [[noreturn]] void f(); // expected-error {{an attribute list cannot appear here}}
+__attribute__((cdecl)) [[noreturn]] __declspec(dllexport) void g(); // expected-error {{an attribute list cannot appear here}}
+
+[[noreturn]] __attribute__((cdecl))
+[[]] // expected-error {{an attribute list cannot appear here}}
+__declspec(dllexport) void h();
Index: clang/lib/Parse/ParseObjc.cpp
===
--- clang/lib/Parse/ParseObjc.cpp
+++ clang/lib/Parse/ParseObjc.cpp
@@ -1350,9 +1350,8 @@
 
   // If attributes exist before the method, parse them.
   ParsedAttributes methodAttrs(AttrFactory);
-  if (getLangOpts().ObjC)
-MaybeParseGNUAttributes(methodAttrs);
-  MaybeParseCXX11Attributes(methodAttrs);
+  MaybeParseAttributes(PAKM_CXX11 | (getLangOpts().ObjC ? PAKM_GNU : 0),
+   methodAttrs);
 
   if (Tok.is(tok::code_completion)) {
 Actions.CodeCompleteObjCMethodDecl(getCurScope(), mType == tok::minus,
@@ -1377,9 +1376,8 @@
   SmallVector CParamInfo;
   if (Tok.isNot(tok::colon)) {
 // If attributes exist after the method,