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; +def warn_attribute_ordering : Warning< + "C++11 attributes should precede all other attributes in an attribute list">, + InGroup; 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'">; 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: 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: 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/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/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: 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}}