Parse different attribute syntaxes in arbitrary order

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.
This commit is contained in:
Aaron Ballman 2021-01-27 15:27:53 -05:00
parent 8e67134364
commit 9f2c7effd7
7 changed files with 134 additions and 45 deletions

View File

@ -2656,6 +2656,61 @@ private:
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 @@ private:
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 @@ private:
}
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 @@ private:
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);

View File

@ -103,6 +103,24 @@ static bool FindLocsWithCommonFileID(Preprocessor &PP, SourceLocation StartLoc,
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 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
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 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
// 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 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
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:

View File

@ -684,8 +684,7 @@ Parser::ParseUsingDeclaration(DeclaratorContext Context,
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 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
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 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
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

View File

@ -1338,12 +1338,9 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
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.

View File

@ -1350,9 +1350,8 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc,
// 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 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc,
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 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc,
// 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 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc,
// 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;

View File

@ -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();

View File

@ -256,7 +256,8 @@ __kernel void k() {
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}}