aaron.ballman updated this revision to Diff 33318.
aaron.ballman added a comment.
I've updated the patch to disallow reordering of C++11 attributes with relation
to other attribute syntaxes. Additionally, if there is a reordering issue
found, we now warn the user.
http://reviews.llvm.org/D12375
Files:
include/clang/Basic/DiagnosticParseKinds.td
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
Index: test/Parser/attr-order.cpp
===================================================================
--- test/Parser/attr-order.cpp
+++ test/Parser/attr-order.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -std=c++14 -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 {}; // expected-warning {{C++11 attributes should precede all other attributes in an attribute list}}
+struct __declspec(dllexport) __attribute__((lockable)) [[]] E {}; // expected-warning {{C++11 attributes should precede all other attributes in an attribute list}}
+struct __attribute__((lockable)) __declspec(dllexport) [[]] F {}; // expected-warning {{C++11 attributes should precede all other attributes in an attribute list}}
+struct __attribute__((lockable)) [[]] __declspec(dllexport) G {}; // expected-warning {{C++11 attributes should precede all other attributes in an attribute list}}
+struct [[]] __attribute__((lockable)) [[]] __declspec(dllexport) H {}; // expected-warning {{C++11 attributes should precede all other attributes in an attribute list}}
+
+[[noreturn]] __attribute__((cdecl)) __declspec(dllexport) void a(); // ok
+[[noreturn]] __declspec(dllexport) __attribute__((cdecl)) void b(); // ok
+[[]] [[noreturn]] __attribute__((cdecl)) __declspec(dllexport) void c(); // ok
+
+// FIXME: The following cases should report the same warning diagnostic as
+// above. However, that requires changing the way we parse decl specifiers vs
+// top-level declarations. See PR24559 for more details.
+__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)) [[]] __declspec(dllexport) void h(); // expected-error {{an attribute list cannot appear here}}
Index: lib/Parse/Parser.cpp
===================================================================
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -586,8 +586,7 @@
}
ParsedAttributesWithRange attrs(AttrFactory);
- MaybeParseCXX11Attributes(attrs);
- MaybeParseMicrosoftAttributes(attrs);
+ MaybeParseAttributes(PAKM_CXX11 | PAKM_Microsoft, attrs);
Result = ParseExternalDeclaration(attrs);
return false;
@@ -1962,8 +1961,7 @@
// FIXME: Support module import within __if_exists?
while (Tok.isNot(tok::r_brace) && !isEofOrEom()) {
ParsedAttributesWithRange attrs(AttrFactory);
- MaybeParseCXX11Attributes(attrs);
- MaybeParseMicrosoftAttributes(attrs);
+ MaybeParseAttributes(PAKM_CXX11 | PAKM_Microsoft, attrs);
DeclGroupPtrTy Result = ParseExternalDeclaration(attrs);
if (Result && !getCurScope()->getParent())
Actions.getASTConsumer().HandleTopLevelDecl(Result.get());
Index: lib/Parse/ParseExprCXX.cpp
===================================================================
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1090,14 +1090,10 @@
SourceLocation RParenLoc = T.getCloseLocation();
DeclEndLoc = RParenLoc;
- // GNU-style attributes must be parsed before the mutable specifier to be
- // compatible with GCC.
- MaybeParseGNUAttributes(Attr, &DeclEndLoc);
+ // GNU-style and __declspec attributes must be parsed before the mutable
+ // specifier to be compatible with GCC/MSVC.
+ MaybeParseAttributes(PAKM_GNU | PAKM_Declspec, Attr, &DeclEndLoc);
- // MSVC-style attributes must be parsed before the mutable specifier to be
- // compatible with MSVC.
- MaybeParseMicrosoftDeclSpecs(Attr, &DeclEndLoc);
-
// Parse 'mutable'[opt].
SourceLocation MutableLoc;
if (TryConsumeToken(tok::kw_mutable, MutableLoc))
Index: lib/Parse/ParseDeclCXX.cpp
===================================================================
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -212,8 +212,7 @@
if (index == Ident.size()) {
while (Tok.isNot(tok::r_brace) && !isEofOrEom()) {
ParsedAttributesWithRange attrs(AttrFactory);
- MaybeParseCXX11Attributes(attrs);
- MaybeParseMicrosoftAttributes(attrs);
+ MaybeParseAttributes(PAKM_CXX11 | PAKM_Microsoft, attrs);
ParseExternalDeclaration(attrs);
}
@@ -303,8 +302,7 @@
Tok.is(tok::l_brace) ? Tok.getLocation() : SourceLocation());
ParsedAttributesWithRange attrs(AttrFactory);
- MaybeParseCXX11Attributes(attrs);
- MaybeParseMicrosoftAttributes(attrs);
+ MaybeParseAttributes(PAKM_CXX11 | PAKM_Microsoft, attrs);
if (Tok.isNot(tok::l_brace)) {
// Reset the source range in DS, as the leading "extern"
@@ -354,8 +352,7 @@
// Fall through.
default:
ParsedAttributesWithRange attrs(AttrFactory);
- MaybeParseCXX11Attributes(attrs);
- MaybeParseMicrosoftAttributes(attrs);
+ MaybeParseAttributes(PAKM_CXX11 | PAKM_Microsoft, attrs);
ParseExternalDeclaration(attrs);
continue;
}
@@ -554,8 +551,7 @@
}
ParsedAttributesWithRange Attrs(AttrFactory);
- MaybeParseGNUAttributes(Attrs);
- MaybeParseCXX11Attributes(Attrs);
+ MaybeParseAttributes(PAKM_CXX11 | PAKM_GNU, Attrs);
// Maybe this is an alias-declaration.
TypeResult TypeAlias;
@@ -1230,8 +1226,7 @@
ParsedAttributesWithRange attrs(AttrFactory);
// If attributes exist after tag, parse them.
- MaybeParseGNUAttributes(attrs);
- MaybeParseMicrosoftDeclSpecs(attrs);
+ MaybeParseAttributes(PAKM_Declspec | PAKM_GNU | PAKM_CXX11, attrs);
// Parse inheritance specifiers.
if (Tok.isOneOf(tok::kw___single_inheritance,
@@ -1239,11 +1234,6 @@
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);
-
// Source location used by FIXIT to insert misplaced
// C++11 attributes
SourceLocation AttrFixitLoc = Tok.getLocation();
@@ -3929,3 +3919,43 @@
Braces.consumeClose();
}
+
+void Parser::MaybeParseAttributes(unsigned WhichAttrKinds,
+ ParsedAttributesWithRange &Attrs,
+ SourceLocation *End,
+ LateParsedAttrList *LateAttrs) {
+ // C++11-style attributes cannot be reordered with respect to other
+ // attribute syntaxes. Per [dcl.spec]p1, those attributes written in a
+ // decl-specifier position will appertain to the decl-specifier, and GNU-
+ // and Declspec-style attributes are decl-specifiers. So if C++ attributes
+ // are parsed, they must be the first attributes parsed in the group,
+ // otherwise we diagnose.
+ bool CXX11AttrAllowed = true;
+ bool MoreToParse;
+ do {
+ // Assume there's nothing left to parse, but if any attributes are in fact
+ // parsed, loop to ensure all attribute combinations are parsed.
+ MoreToParse = false;
+ if (WhichAttrKinds & PAKM_CXX11) {
+ SourceLocation AttrStartLoc = Tok.getLocation();
+ MoreToParse |= MaybeParseCXX11Attributes(Attrs, End);
+ // If C++11 attributes are not allowed because other attributes have
+ // already been parsed, we should diagnose if such an attribute was
+ // parsed.
+ if (MoreToParse && !CXX11AttrAllowed)
+ Diag(AttrStartLoc, diag::warn_attribute_ordering);
+ }
+ if (WhichAttrKinds & PAKM_GNU) {
+ MoreToParse |= MaybeParseGNUAttributes(Attrs, End, LateAttrs);
+ CXX11AttrAllowed = false;
+ }
+ if (WhichAttrKinds & PAKM_Declspec) {
+ MoreToParse |= MaybeParseMicrosoftDeclSpecs(Attrs, End);
+ CXX11AttrAllowed = false;
+ }
+ if (WhichAttrKinds & PAKM_Microsoft) {
+ MoreToParse |= MaybeParseMicrosoftAttributes(Attrs, End);
+ CXX11AttrAllowed = false;
+ }
+ } while (MoreToParse);
+}
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -3046,14 +3046,11 @@
continue;
}
- // GNU attributes support.
+ // Attribute support.
case tok::kw___attribute:
- ParseGNUAttributes(DS.getAttributes(), nullptr, LateAttrs);
- continue;
-
- // Microsoft declspec support.
case tok::kw___declspec:
- ParseMicrosoftDeclSpecs(DS.getAttributes());
+ MaybeParseAttributes(PAKM_GNU | PAKM_Declspec, DS.getAttributes(),
+ nullptr, LateAttrs);
continue;
// Microsoft single token adornments.
@@ -3731,9 +3728,7 @@
// If attributes exist after tag, parse them.
ParsedAttributesWithRange attrs(AttrFactory);
- MaybeParseGNUAttributes(attrs);
- MaybeParseCXX11Attributes(attrs);
- MaybeParseMicrosoftDeclSpecs(attrs);
+ MaybeParseAttributes(PAKM_GNU | PAKM_CXX11 | PAKM_Declspec, attrs);
SourceLocation ScopedEnumKWLoc;
bool IsScopedUsingClassTag = false;
@@ -3750,9 +3745,7 @@
ProhibitAttributes(attrs);
// They are allowed afterwards, though.
- MaybeParseGNUAttributes(attrs);
- MaybeParseCXX11Attributes(attrs);
- MaybeParseMicrosoftDeclSpecs(attrs);
+ MaybeParseAttributes(PAKM_GNU | PAKM_CXX11 | PAKM_Declspec, attrs);
}
// C++11 [temp.explicit]p12:
@@ -5775,12 +5768,9 @@
// Just use the ParsingDeclaration "scope" of the declarator.
DeclSpec DS(AttrFactory);
- // Parse any C++11 attributes.
- MaybeParseCXX11Attributes(DS.getAttributes());
+ // Parse any C++11 and Microsoft attributes.
+ MaybeParseAttributes(PAKM_CXX11 | PAKM_Microsoft, DS.getAttributes());
- // Skip any Microsoft attributes before a param.
- MaybeParseMicrosoftAttributes(DS.getAttributes());
-
SourceLocation DSStart = Tok.getLocation();
// If the caller parsed attributes for the first argument, add them now.
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -2097,11 +2097,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,
@@ -2125,20 +2128,25 @@
D.takeAttributes(attrs, endLoc);
}
}
- void MaybeParseCXX11Attributes(ParsedAttributes &attrs,
+ bool MaybeParseCXX11Attributes(ParsedAttributes &attrs,
SourceLocation *endLoc = nullptr) {
if (getLangOpts().CPlusPlus11 && isCXX11AttributeSpecifier()) {
ParsedAttributesWithRange attrsWithRange(AttrFactory);
ParseCXX11Attributes(attrsWithRange, endLoc);
attrs.takeAllFrom(attrsWithRange);
+ return true;
}
+ return false;
}
- void MaybeParseCXX11Attributes(ParsedAttributesWithRange &attrs,
+ bool MaybeParseCXX11Attributes(ParsedAttributesWithRange &attrs,
SourceLocation *endLoc = nullptr,
bool OuterMightBeMessageSend = false) {
if (getLangOpts().CPlusPlus11 &&
- isCXX11AttributeSpecifier(false, OuterMightBeMessageSend))
+ isCXX11AttributeSpecifier(false, OuterMightBeMessageSend)) {
ParseCXX11Attributes(attrs, endLoc);
+ return true;
+ }
+ return false;
}
void ParseCXX11AttributeSpecifier(ParsedAttributes &attrs,
@@ -2155,19 +2163,25 @@
IdentifierInfo *TryParseCXX11AttributeIdentifier(SourceLocation &Loc);
- void MaybeParseMicrosoftAttributes(ParsedAttributes &attrs,
+ bool MaybeParseMicrosoftAttributes(ParsedAttributes &attrs,
SourceLocation *endLoc = nullptr) {
- if (getLangOpts().MicrosoftExt && Tok.is(tok::l_square))
+ if (getLangOpts().MicrosoftExt && Tok.is(tok::l_square)) {
ParseMicrosoftAttributes(attrs, endLoc);
+ return true;
+ }
+ return false;
}
void ParseMicrosoftAttributes(ParsedAttributes &attrs,
SourceLocation *endLoc = nullptr);
- void MaybeParseMicrosoftDeclSpecs(ParsedAttributes &Attrs,
+ bool MaybeParseMicrosoftDeclSpecs(ParsedAttributes &Attrs,
SourceLocation *End = nullptr) {
const auto &LO = getLangOpts();
if ((LO.MicrosoftExt || LO.Borland || LO.CUDA) &&
- Tok.is(tok::kw___declspec))
+ Tok.is(tok::kw___declspec)) {
ParseMicrosoftDeclSpecs(Attrs, End);
+ return true;
+ }
+ return false;
}
void ParseMicrosoftDeclSpecs(ParsedAttributes &Attrs,
SourceLocation *End = nullptr);
@@ -2174,6 +2188,32 @@
bool ParseMicrosoftDeclSpecArgs(IdentifierInfo *AttrName,
SourceLocation AttrNameLoc,
ParsedAttributes &Attrs);
+
+ enum ParseAttrKindMask {
+ PAKM_GNU = 1 << 0,
+ PAKM_Declspec = 1 << 1,
+ PAKM_CXX11 = 1 << 2,
+ PAKM_Microsoft = 1 << 3
+ };
+
+ /// \brief Possibly parse attributes based on what syntaxes are desired,
+ /// allowing for the order to vary. e.g. with PAKM_GNU | PAKM_Declspec:
+ /// __attribute__((...)) __declspec(...) __attribute__((...)))
+ void MaybeParseAttributes(unsigned WhichAttrKinds,
+ ParsedAttributesWithRange &Attrs,
+ SourceLocation *End = nullptr,
+ LateParsedAttrList *LateAttrs = nullptr);
+ /// \brief Possibly parse attributes based on what syntaxes are desired,
+ /// allowing for the order to vary. e.g. with PAKM_GNU | PAKM_Declspec:
+ /// __attribute__((...)) __declspec(...) __attribute__((...)))
+ void MaybeParseAttributes(unsigned WhichAttrKinds, ParsedAttributes &Attrs,
+ SourceLocation *End = nullptr,
+ LateParsedAttrList *LateAttrs = nullptr) {
+ ParsedAttributesWithRange AttrsWithRange(AttrFactory);
+ MaybeParseAttributes(WhichAttrKinds, AttrsWithRange, End, LateAttrs);
+ Attrs.takeAllFrom(AttrsWithRange);
+ }
+
void ParseMicrosoftTypeAttributes(ParsedAttributes &attrs);
void DiagnoseAndSkipExtendedMicrosoftTypeAttributes();
SourceLocation SkipExtendedMicrosoftTypeAttributes();
Index: include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- include/clang/Basic/DiagnosticParseKinds.td
+++ include/clang/Basic/DiagnosticParseKinds.td
@@ -187,6 +187,9 @@
def warn_attribute_no_decl : Warning<
"attribute %0 ignored, because it is not attached to a declaration">,
InGroup<IgnoredAttributes>;
+def warn_attribute_ordering : Warning<
+ "C++11 attributes should precede all other attributes in an attribute list">,
+ InGroup<IgnoredAttributes>;
def err_expected_method_body : Error<"expected method body">;
def err_declspec_after_virtspec : Error<
"'%0' qualifier may not appear after the virtual specifier '%1'">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
