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 <https://reviews.llvm.org/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<DeclaratorChunk::ParamInfo, 8> CParamInfo;
if (Tok.isNot(tok::colon)) {
// If attributes exist after the method, parse them.
- if (getLangOpts().ObjC)
- MaybeParseGNUAttributes(methodAttrs);
- MaybeParseCXX11Attributes(methodAttrs);
+ MaybeParseAttributes(PAKM_CXX11 | (getLangOpts().ObjC ? PAKM_GNU : 0),
+ methodAttrs);
Selector Sel = PP.getSelectorTable().getNullarySelector(SelIdent);
Decl *Result = Actions.ActOnMethodDeclaration(
@@ -1412,9 +1410,8 @@
// If attributes exist before the argument name, parse them.
// Regardless, collect all the attributes we've parsed so far.
- if (getLangOpts().ObjC)
- MaybeParseGNUAttributes(paramAttrs);
- MaybeParseCXX11Attributes(paramAttrs);
+ MaybeParseAttributes(PAKM_CXX11 | (getLangOpts().ObjC ? PAKM_GNU : 0),
+ paramAttrs);
ArgInfo.ArgAttrs = paramAttrs;
// Code completion for the next piece of the selector.
@@ -1496,9 +1493,8 @@
// FIXME: Add support for optional parameter list...
// If attributes exist after the method, parse them.
- if (getLangOpts().ObjC)
- MaybeParseGNUAttributes(methodAttrs);
- MaybeParseCXX11Attributes(methodAttrs);
+ MaybeParseAttributes(PAKM_CXX11 | (getLangOpts().ObjC ? PAKM_GNU : 0),
+ methodAttrs);
if (KeyIdents.size() == 0)
return nullptr;
Index: clang/lib/Parse/ParseExprCXX.cpp
===================================================================
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -1338,12 +1338,9 @@
SourceLocation DeclEndLoc = RParenLoc;
// GNU-style attributes must be parsed before the mutable specifier to be
- // compatible with GCC.
- MaybeParseGNUAttributes(Attr, &DeclEndLoc);
-
- // MSVC-style attributes must be parsed before the mutable specifier to be
- // compatible with MSVC.
- MaybeParseMicrosoftDeclSpecs(Attr, &DeclEndLoc);
+ // compatible with GCC. MSVC-style attributes must be parsed before the
+ // mutable specifier to be compatible with MSVC.
+ MaybeParseAttributes(PAKM_GNU | PAKM_Declspec, Attr);
// Parse mutable-opt and/or constexpr-opt or consteval-opt, and update the
// DeclEndLoc.
Index: clang/lib/Parse/ParseDeclCXX.cpp
===================================================================
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -684,8 +684,7 @@
bool InvalidDeclarator = ParseUsingDeclarator(Context, D);
ParsedAttributesWithRange Attrs(AttrFactory);
- MaybeParseGNUAttributes(Attrs);
- MaybeParseCXX11Attributes(Attrs);
+ MaybeParseAttributes(PAKM_GNU | PAKM_CXX11, Attrs);
// Maybe this is an alias-declaration.
if (Tok.is(tok::equal)) {
@@ -1435,8 +1434,7 @@
ParsedAttributesWithRange attrs(AttrFactory);
// If attributes exist after tag, parse them.
- MaybeParseGNUAttributes(attrs);
- MaybeParseMicrosoftDeclSpecs(attrs);
+ MaybeParseAttributes(PAKM_CXX11 | PAKM_Declspec | PAKM_GNU, attrs);
// Parse inheritance specifiers.
if (Tok.isOneOf(tok::kw___single_inheritance,
@@ -1444,10 +1442,8 @@
tok::kw___virtual_inheritance))
ParseMicrosoftInheritanceClassAttributes(attrs);
- // If C++0x attributes exist here, parse them.
- // FIXME: Are we consistent with the ordering of parsing of different
- // styles of attributes?
- MaybeParseCXX11Attributes(attrs);
+ // Allow attributes to precede or succeed the inheritance specifiers.
+ MaybeParseAttributes(PAKM_CXX11 | PAKM_Declspec | PAKM_GNU, attrs);
// Source location used by FIXIT to insert misplaced
// C++11 attributes
Index: clang/lib/Parse/ParseDecl.cpp
===================================================================
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -103,6 +103,24 @@
return AttrStartIsInMacro && AttrEndIsInMacro;
}
+void Parser::ParseAttributes(unsigned WhichAttrKinds,
+ ParsedAttributesWithRange &Attrs,
+ SourceLocation *End,
+ LateParsedAttrList *LateAttrs) {
+ bool MoreToParse;
+ do {
+ // Assume there's nothing left to parse, but if any attributes are in fact
+ // parsed, loop to ensure all specified attribute combinations are parsed.
+ MoreToParse = false;
+ if (WhichAttrKinds & PAKM_CXX11)
+ MoreToParse |= MaybeParseCXX11Attributes(Attrs, End);
+ if (WhichAttrKinds & PAKM_GNU)
+ MoreToParse |= MaybeParseGNUAttributes(Attrs, End, LateAttrs);
+ if (WhichAttrKinds & PAKM_Declspec)
+ MoreToParse |= MaybeParseMicrosoftDeclSpecs(Attrs, End);
+ } while (MoreToParse);
+}
+
/// ParseGNUAttributes - Parse a non-empty attributes list.
///
/// [GNU] attributes:
@@ -3485,14 +3503,11 @@
continue;
}
- // GNU attributes support.
+ // Attributes support.
case tok::kw___attribute:
- ParseGNUAttributes(DS.getAttributes(), nullptr, LateAttrs);
- continue;
-
- // Microsoft declspec support.
case tok::kw___declspec:
- ParseMicrosoftDeclSpecs(DS.getAttributes());
+ ParseAttributes(PAKM_GNU | PAKM_Declspec, DS.getAttributes(), nullptr,
+ LateAttrs);
continue;
// Microsoft single token adornments.
@@ -4354,9 +4369,7 @@
// If attributes exist after tag, parse them.
ParsedAttributesWithRange attrs(AttrFactory);
- MaybeParseGNUAttributes(attrs);
- MaybeParseCXX11Attributes(attrs);
- MaybeParseMicrosoftDeclSpecs(attrs);
+ MaybeParseAttributes(PAKM_GNU | PAKM_Declspec | PAKM_CXX11, attrs);
SourceLocation ScopedEnumKWLoc;
bool IsScopedUsingClassTag = false;
@@ -4373,9 +4386,7 @@
ProhibitAttributes(attrs);
// They are allowed afterwards, though.
- MaybeParseGNUAttributes(attrs);
- MaybeParseCXX11Attributes(attrs);
- MaybeParseMicrosoftDeclSpecs(attrs);
+ MaybeParseAttributes(PAKM_GNU | PAKM_Declspec | PAKM_CXX11, attrs);
}
// C++11 [temp.explicit]p12:
Index: clang/include/clang/Parse/Parser.h
===================================================================
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -2656,6 +2656,61 @@
IdentifierInfo *ScopeName, SourceLocation ScopeLoc,
ParsedAttr::Syntax Syntax);
+ enum ParseAttrKindMask {
+ PAKM_GNU = 1 << 0,
+ PAKM_Declspec = 1 << 1,
+ PAKM_CXX11 = 1 << 2,
+ };
+
+ /// \brief Parse attributes based on what syntaxes are desired, allowing for
+ /// the order to vary. e.g. with PAKM_GNU | PAKM_Declspec:
+ /// __attribute__((...)) __declspec(...) __attribute__((...)))
+ /// Note that Microsoft attributes (spelled with single square brackets) are
+ /// not supported by this because of parsing ambiguities with other
+ /// constructs.
+ ///
+ /// There are some attribute parse orderings that should not be allowed in
+ /// arbitrary order. e.g.,
+ ///
+ /// [[]] __attribute__(()) int i; // OK
+ /// __attribute__(()) [[]] int i; // Not OK
+ ///
+ /// Such situations should use the specific attribute parsing functionality.
+ void ParseAttributes(unsigned WhichAttrKinds,
+ ParsedAttributesWithRange &Attrs,
+ SourceLocation *End = nullptr,
+ LateParsedAttrList *LateAttrs = nullptr);
+ void ParseAttributes(unsigned WhichAttrKinds, ParsedAttributes &Attrs,
+ SourceLocation *End = nullptr,
+ LateParsedAttrList *LateAttrs = nullptr) {
+ ParsedAttributesWithRange AttrsWithRange(AttrFactory);
+ ParseAttributes(WhichAttrKinds, AttrsWithRange, End, LateAttrs);
+ Attrs.takeAllFrom(AttrsWithRange);
+ }
+ /// \brief Possibly parse attributes based on what syntaxes are desired,
+ /// allowing for the order to vary.
+ bool MaybeParseAttributes(unsigned WhichAttrKinds,
+ ParsedAttributesWithRange &Attrs,
+ SourceLocation *End = nullptr,
+ LateParsedAttrList *LateAttrs = nullptr) {
+ if (Tok.isOneOf(tok::kw___attribute, tok::kw___declspec) ||
+ standardAttributesAllowed() && isCXX11AttributeSpecifier()) {
+ ParseAttributes(WhichAttrKinds, Attrs, End, LateAttrs);
+ return true;
+ }
+ return false;
+ }
+ bool MaybeParseAttributes(unsigned WhichAttrKinds, ParsedAttributes &Attrs,
+ SourceLocation *End = nullptr,
+ LateParsedAttrList *LateAttrs = nullptr) {
+ if (Tok.isOneOf(tok::kw___attribute, tok::kw___declspec) ||
+ standardAttributesAllowed() && isCXX11AttributeSpecifier()) {
+ ParseAttributes(WhichAttrKinds, Attrs, End, LateAttrs);
+ return true;
+ }
+ return false;
+ }
+
void MaybeParseGNUAttributes(Declarator &D,
LateParsedAttrList *LateAttrs = nullptr) {
if (Tok.is(tok::kw___attribute)) {
@@ -2665,11 +2720,14 @@
D.takeAttributes(attrs, endLoc);
}
}
- void MaybeParseGNUAttributes(ParsedAttributes &attrs,
+ bool MaybeParseGNUAttributes(ParsedAttributes &attrs,
SourceLocation *endLoc = nullptr,
LateParsedAttrList *LateAttrs = nullptr) {
- if (Tok.is(tok::kw___attribute))
+ if (Tok.is(tok::kw___attribute)) {
ParseGNUAttributes(attrs, endLoc, LateAttrs);
+ return true;
+ }
+ return false;
}
void ParseGNUAttributes(ParsedAttributes &attrs,
SourceLocation *endLoc = nullptr,
@@ -2706,12 +2764,15 @@
}
return false;
}
- void MaybeParseCXX11Attributes(ParsedAttributesWithRange &attrs,
+ bool MaybeParseCXX11Attributes(ParsedAttributesWithRange &attrs,
SourceLocation *endLoc = nullptr,
bool OuterMightBeMessageSend = false) {
if (standardAttributesAllowed() &&
- isCXX11AttributeSpecifier(false, OuterMightBeMessageSend))
+ isCXX11AttributeSpecifier(false, OuterMightBeMessageSend)) {
ParseCXX11Attributes(attrs, endLoc);
+ return true;
+ }
+ return false;
}
void ParseCXX11AttributeSpecifier(ParsedAttributes &attrs,
@@ -2736,11 +2797,14 @@
void ParseMicrosoftUuidAttributeArgs(ParsedAttributes &Attrs);
void ParseMicrosoftAttributes(ParsedAttributes &attrs,
SourceLocation *endLoc = nullptr);
- void MaybeParseMicrosoftDeclSpecs(ParsedAttributes &Attrs,
+ bool MaybeParseMicrosoftDeclSpecs(ParsedAttributes &Attrs,
SourceLocation *End = nullptr) {
const auto &LO = getLangOpts();
- if (LO.DeclSpecKeyword && Tok.is(tok::kw___declspec))
+ if (LO.DeclSpecKeyword && Tok.is(tok::kw___declspec)) {
ParseMicrosoftDeclSpecs(Attrs, End);
+ return true;
+ }
+ return false;
}
void ParseMicrosoftDeclSpecs(ParsedAttributes &Attrs,
SourceLocation *End = nullptr);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
