diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -298,6 +298,9 @@ and Clang 15 accidentally stopped predeclaring those functions in that language mode. Clang 16 now predeclares those functions again. This fixes `Issue 56607 `_. +- GNU attributes being applied prior to standard attributes would be handled + improperly, which was corrected to match the behaviour exhibited by GCC. + `Issue 58229 `_ Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -1602,15 +1602,16 @@ //===--------------------------------------------------------------------===// // C99 6.9: External Definitions. - DeclGroupPtrTy ParseExternalDeclaration(ParsedAttributes &Attrs, + DeclGroupPtrTy ParseExternalDeclaration(ParsedAttributes &DeclAttrs, + ParsedAttributes &DeclSpecAttrs, ParsingDeclSpec *DS = nullptr); bool isDeclarationAfterDeclarator(); bool isStartOfFunctionDefinition(const ParsingDeclarator &Declarator); - DeclGroupPtrTy - ParseDeclarationOrFunctionDefinition(ParsedAttributes &Attrs, - ParsingDeclSpec *DS = nullptr, - AccessSpecifier AS = AS_none); + DeclGroupPtrTy ParseDeclarationOrFunctionDefinition( + ParsedAttributes &DeclAttrs, ParsedAttributes &DeclSpecAttrs, + ParsingDeclSpec *DS = nullptr, AccessSpecifier AS = AS_none); DeclGroupPtrTy ParseDeclOrFunctionDefInternal(ParsedAttributes &Attrs, + ParsedAttributes &DeclSpecAttrs, ParsingDeclSpec &DS, AccessSpecifier AS); @@ -1625,7 +1626,8 @@ // Objective-C External Declarations void MaybeSkipAttributes(tok::ObjCKeywordKind Kind); - DeclGroupPtrTy ParseObjCAtDirectives(ParsedAttributes &Attrs); + DeclGroupPtrTy ParseObjCAtDirectives(ParsedAttributes &DeclAttrs, + ParsedAttributes &DeclSpecAttrs); DeclGroupPtrTy ParseObjCAtClassDeclaration(SourceLocation atLoc); Decl *ParseObjCAtInterfaceDeclaration(SourceLocation AtLoc, ParsedAttributes &prefixAttrs); diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -254,9 +254,10 @@ if (index == InnerNSs.size()) { while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof)) { - ParsedAttributes Attrs(AttrFactory); - MaybeParseCXX11Attributes(Attrs); - ParseExternalDeclaration(Attrs); + ParsedAttributes DeclAttrs(AttrFactory); + MaybeParseCXX11Attributes(DeclAttrs); + ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); + ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs); } // The caller is what called check -- we are simply calling @@ -359,6 +360,7 @@ ParsedAttributes DeclAttrs(AttrFactory); MaybeParseCXX11Attributes(DeclAttrs); + ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); if (Tok.isNot(tok::l_brace)) { // Reset the source range in DS, as the leading "extern" @@ -367,7 +369,7 @@ DS.SetRangeEnd(SourceLocation()); // ... but anyway remember that such an "extern" was seen. DS.setExternInLinkageSpec(true); - ParseExternalDeclaration(DeclAttrs, &DS); + ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs, &DS); return LinkageSpec ? Actions.ActOnFinishLinkageSpecification( getCurScope(), LinkageSpec, SourceLocation()) : nullptr; @@ -407,9 +409,9 @@ break; [[fallthrough]]; default: - ParsedAttributes Attrs(AttrFactory); - MaybeParseCXX11Attributes(Attrs); - ParseExternalDeclaration(Attrs); + ParsedAttributes DeclAttrs(AttrFactory); + MaybeParseCXX11Attributes(DeclAttrs); + ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs); continue; } @@ -439,9 +441,10 @@ if (Tok.isNot(tok::l_brace)) { // FIXME: Factor out a ParseExternalDeclarationWithAttrs. - ParsedAttributes Attrs(AttrFactory); - MaybeParseCXX11Attributes(Attrs); - ParseExternalDeclaration(Attrs); + ParsedAttributes DeclAttrs(AttrFactory); + MaybeParseCXX11Attributes(DeclAttrs); + ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); + ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs); return Actions.ActOnFinishExportDecl(getCurScope(), ExportDecl, SourceLocation()); } @@ -458,9 +461,10 @@ while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof)) { - ParsedAttributes Attrs(AttrFactory); - MaybeParseCXX11Attributes(Attrs); - ParseExternalDeclaration(Attrs); + ParsedAttributes DeclAttrs(AttrFactory); + MaybeParseCXX11Attributes(DeclAttrs); + ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); + ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs); } T.consumeClose(); diff --git a/clang/lib/Parse/ParseHLSL.cpp b/clang/lib/Parse/ParseHLSL.cpp --- a/clang/lib/Parse/ParseHLSL.cpp +++ b/clang/lib/Parse/ParseHLSL.cpp @@ -77,9 +77,11 @@ while (Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof)) { // FIXME: support attribute on constants inside cbuffer/tbuffer. - ParsedAttributes Attrs(AttrFactory); + ParsedAttributes DeclAttrs(AttrFactory); + ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); - DeclGroupPtrTy Result = ParseExternalDeclaration(Attrs); + DeclGroupPtrTy Result = + ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs); if (!validateDeclsInsideHLSLBuffer(Result, IdentifierLoc, IsCBuffer, *this)) { T.skipToEnd(); diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp --- a/clang/lib/Parse/ParseObjc.cpp +++ b/clang/lib/Parse/ParseObjc.cpp @@ -46,7 +46,11 @@ /// [OBJC] objc-protocol-definition /// [OBJC] objc-method-definition /// [OBJC] '@' 'end' -Parser::DeclGroupPtrTy Parser::ParseObjCAtDirectives(ParsedAttributes &Attrs) { +Parser::DeclGroupPtrTy +Parser::ParseObjCAtDirectives(ParsedAttributes &DeclAttrs, + ParsedAttributes &DeclSpecAttrs) { + DeclAttrs.takeAllFrom(DeclSpecAttrs); + SourceLocation AtLoc = ConsumeToken(); // the "@" if (Tok.is(tok::code_completion)) { @@ -55,17 +59,29 @@ return nullptr; } + switch (Tok.getObjCKeywordID()) { + case tok::objc_interface: + case tok::objc_protocol: + case tok::objc_implementation: + break; + default: + llvm::for_each(DeclAttrs, [this](const auto &Attr) { + if (Attr.isGNUAttribute()) + Diag(Tok.getLocation(), diag::err_objc_unexpected_attr); + }); + } + Decl *SingleDecl = nullptr; switch (Tok.getObjCKeywordID()) { case tok::objc_class: return ParseObjCAtClassDeclaration(AtLoc); case tok::objc_interface: - SingleDecl = ParseObjCAtInterfaceDeclaration(AtLoc, Attrs); + SingleDecl = ParseObjCAtInterfaceDeclaration(AtLoc, DeclAttrs); break; case tok::objc_protocol: - return ParseObjCAtProtocolDeclaration(AtLoc, Attrs); + return ParseObjCAtProtocolDeclaration(AtLoc, DeclAttrs); case tok::objc_implementation: - return ParseObjCAtImplementationDeclaration(AtLoc, Attrs); + return ParseObjCAtImplementationDeclaration(AtLoc, DeclAttrs); case tok::objc_end: return ParseObjCAtEndDeclaration(AtLoc); case tok::objc_compatibility_alias: @@ -651,7 +667,8 @@ if (Tok.is(tok::r_brace)) break; - ParsedAttributes EmptyAttrs(AttrFactory); + ParsedAttributes EmptyDeclAttrs(AttrFactory); + ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); // Since we call ParseDeclarationOrFunctionDefinition() instead of // ParseExternalDeclaration() below (so that this doesn't parse nested @@ -659,13 +676,14 @@ if (Tok.isOneOf(tok::kw_static_assert, tok::kw__Static_assert)) { SourceLocation DeclEnd; ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); - allTUVariables.push_back(ParseDeclaration( - DeclaratorContext::File, DeclEnd, EmptyAttrs, EmptyDeclSpecAttrs)); + allTUVariables.push_back(ParseDeclaration(DeclaratorContext::File, + DeclEnd, EmptyDeclAttrs, + EmptyDeclSpecAttrs)); continue; } - allTUVariables.push_back( - ParseDeclarationOrFunctionDefinition(EmptyAttrs)); + allTUVariables.push_back(ParseDeclarationOrFunctionDefinition( + EmptyDeclAttrs, EmptyDeclSpecAttrs)); continue; } @@ -2236,9 +2254,11 @@ { ObjCImplParsingDataRAII ObjCImplParsing(*this, ObjCImpDecl); while (!ObjCImplParsing.isFinished() && !isEofOrEom()) { - ParsedAttributes attrs(AttrFactory); - MaybeParseCXX11Attributes(attrs); - if (DeclGroupPtrTy DGP = ParseExternalDeclaration(attrs)) { + ParsedAttributes DeclAttrs(AttrFactory); + MaybeParseCXX11Attributes(DeclAttrs); + ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); + if (DeclGroupPtrTy DGP = + ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs)) { DeclGroupRef DG = DGP.get(); DeclsInGroup.append(DG.begin(), DG.end()); } diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp --- a/clang/lib/Parse/ParseOpenMP.cpp +++ b/clang/lib/Parse/ParseOpenMP.cpp @@ -2304,9 +2304,10 @@ // Here we expect to see some function declaration. if (AS == AS_none) { assert(TagType == DeclSpec::TST_unspecified); + ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); MaybeParseCXX11Attributes(Attrs); ParsingDeclSpec PDS(*this); - Ptr = ParseExternalDeclaration(Attrs, &PDS); + Ptr = ParseExternalDeclaration(Attrs, EmptyDeclSpecAttrs, &PDS); } else { Ptr = ParseCXXClassMemberDeclarationWithPragmas(AS, Attrs, TagType, Tag); diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -731,10 +731,17 @@ break; } - ParsedAttributes attrs(AttrFactory); - MaybeParseCXX11Attributes(attrs); - - Result = ParseExternalDeclaration(attrs); + ParsedAttributes DeclAttrs(AttrFactory); + ParsedAttributes DeclSpecAttrs(AttrFactory); + // GNU attributes are applied to the declaration specification while the + // standard attributes are applied to the declaration. We parse the two + // attribute sets into different containters so we can apply them during + // the regular parsing process. + while (MaybeParseCXX11Attributes(DeclAttrs) || + MaybeParseGNUAttributes(DeclSpecAttrs)) + ; + + Result = ParseExternalDeclaration(DeclAttrs, DeclSpecAttrs); // An empty Result might mean a line with ';' or some parsing error, ignore // it. if (Result) { @@ -777,8 +784,10 @@ /// /// [Modules-TS] module-import-declaration /// -Parser::DeclGroupPtrTy Parser::ParseExternalDeclaration(ParsedAttributes &Attrs, - ParsingDeclSpec *DS) { +Parser::DeclGroupPtrTy +Parser::ParseExternalDeclaration(ParsedAttributes &Attrs, + ParsedAttributes &DeclSpecAttrs, + ParsingDeclSpec *DS) { DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(*this); ParenBraceBracketBalancer BalancerRAIIObj(*this); @@ -866,7 +875,7 @@ // __extension__ silences extension warnings in the subexpression. ExtensionRAIIObject O(Diags); // Use RAII to do this. ConsumeToken(); - return ParseExternalDeclaration(Attrs); + return ParseExternalDeclaration(Attrs, DeclSpecAttrs); } case tok::kw_asm: { ProhibitAttributes(Attrs); @@ -894,7 +903,7 @@ break; } case tok::at: - return ParseObjCAtDirectives(Attrs); + return ParseObjCAtDirectives(Attrs, DeclSpecAttrs); case tok::minus: case tok::plus: if (!getLangOpts().ObjC) { @@ -942,18 +951,16 @@ // A function definition cannot start with any of these keywords. { SourceLocation DeclEnd; - ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); return ParseDeclaration(DeclaratorContext::File, DeclEnd, Attrs, - EmptyDeclSpecAttrs); + DeclSpecAttrs); } case tok::kw_cbuffer: case tok::kw_tbuffer: if (getLangOpts().HLSL) { SourceLocation DeclEnd; - ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); return ParseDeclaration(DeclaratorContext::File, DeclEnd, Attrs, - EmptyDeclSpecAttrs); + DeclSpecAttrs); } goto dont_know; @@ -964,9 +971,8 @@ Diag(ConsumeToken(), diag::warn_static_inline_explicit_inst_ignored) << 0; SourceLocation DeclEnd; - ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); return ParseDeclaration(DeclaratorContext::File, DeclEnd, Attrs, - EmptyDeclSpecAttrs); + DeclSpecAttrs); } goto dont_know; @@ -977,9 +983,8 @@ // Inline namespaces. Allowed as an extension even in C++03. if (NextKind == tok::kw_namespace) { SourceLocation DeclEnd; - ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); return ParseDeclaration(DeclaratorContext::File, DeclEnd, Attrs, - EmptyDeclSpecAttrs); + DeclSpecAttrs); } // Parse (then ignore) 'inline' prior to a template instantiation. This is @@ -988,9 +993,8 @@ Diag(ConsumeToken(), diag::warn_static_inline_explicit_inst_ignored) << 1; SourceLocation DeclEnd; - ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); return ParseDeclaration(DeclaratorContext::File, DeclEnd, Attrs, - EmptyDeclSpecAttrs); + DeclSpecAttrs); } } goto dont_know; @@ -1026,7 +1030,7 @@ return nullptr; } // We can't tell whether this is a function-definition or declaration yet. - return ParseDeclarationOrFunctionDefinition(Attrs, DS); + return ParseDeclarationOrFunctionDefinition(Attrs, DeclSpecAttrs, DS); } // This routine returns a DeclGroup, if the thing we parsed only contains a @@ -1091,7 +1095,17 @@ /// [OMP] allocate-directive [TODO] /// Parser::DeclGroupPtrTy Parser::ParseDeclOrFunctionDefInternal( - ParsedAttributes &Attrs, ParsingDeclSpec &DS, AccessSpecifier AS) { + ParsedAttributes &Attrs, ParsedAttributes &DeclSpecAttrs, + ParsingDeclSpec &DS, AccessSpecifier AS) { + // Because we assume that the DeclSpec has not yet been initialised, we simply + // overwrite the source range and attribute the provided leading declspec + // attributes. + assert(DS.getSourceRange().isInvalid() && + "expected uninitialised source range"); + DS.SetRangeStart(DeclSpecAttrs.Range.getBegin()); + DS.SetRangeEnd(DeclSpecAttrs.Range.getEnd()); + DS.takeAttributesFrom(DeclSpecAttrs); + MaybeParseMicrosoftAttributes(DS.getAttributes()); // Parse the common declaration-specifiers piece. ParseDeclarationSpecifiers(DS, ParsedTemplateInfo(), AS, @@ -1190,9 +1204,10 @@ } Parser::DeclGroupPtrTy Parser::ParseDeclarationOrFunctionDefinition( - ParsedAttributes &Attrs, ParsingDeclSpec *DS, AccessSpecifier AS) { + ParsedAttributes &Attrs, ParsedAttributes &DeclSpecAttrs, + ParsingDeclSpec *DS, AccessSpecifier AS) { if (DS) { - return ParseDeclOrFunctionDefInternal(Attrs, *DS, AS); + return ParseDeclOrFunctionDefInternal(Attrs, DeclSpecAttrs, *DS, AS); } else { ParsingDeclSpec PDS(*this); // Must temporarily exit the objective-c container scope for @@ -1200,7 +1215,7 @@ // afterwards. ObjCDeclContextSwitch ObjCDC(*this); - return ParseDeclOrFunctionDefInternal(Attrs, PDS, AS); + return ParseDeclOrFunctionDefInternal(Attrs, DeclSpecAttrs, PDS, AS); } } @@ -2342,7 +2357,8 @@ while (Tok.isNot(tok::r_brace) && !isEofOrEom()) { ParsedAttributes Attrs(AttrFactory); MaybeParseCXX11Attributes(Attrs); - DeclGroupPtrTy Result = ParseExternalDeclaration(Attrs); + ParsedAttributes EmptyDeclSpecAttrs(AttrFactory); + DeclGroupPtrTy Result = ParseExternalDeclaration(Attrs, EmptyDeclSpecAttrs); if (Result && !getCurScope()->getParent()) Actions.getASTConsumer().HandleTopLevelDecl(Result.get()); } diff --git a/clang/test/Parser/attr-order.cpp b/clang/test/Parser/attr-order.cpp --- a/clang/test/Parser/attr-order.cpp +++ b/clang/test/Parser/attr-order.cpp @@ -17,8 +17,8 @@ __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}} +__attribute__((cdecl)) [[noreturn]] __declspec(dllexport) void g(); [[noreturn]] __attribute__((cdecl)) -[[]] // expected-error {{an attribute list cannot appear here}} +[[]] __declspec(dllexport) void h(); diff --git a/clang/test/Parser/cxx-attributes.cpp b/clang/test/Parser/cxx-attributes.cpp --- a/clang/test/Parser/cxx-attributes.cpp +++ b/clang/test/Parser/cxx-attributes.cpp @@ -1,5 +1,9 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s +// GH#58229 - rejects-valid +__attribute__((__visibility__("default"))) [[nodiscard]] int f(); +[[nodiscard]] __attribute__((__visibility__("default"))) int f(); + class c { virtual void f1(const char* a, ...) __attribute__ (( __format__(__printf__,2,3) )) = 0; diff --git a/clang/test/SemaCXX/attr-unavailable.cpp b/clang/test/SemaCXX/attr-unavailable.cpp --- a/clang/test/SemaCXX/attr-unavailable.cpp +++ b/clang/test/SemaCXX/attr-unavailable.cpp @@ -42,18 +42,18 @@ // delayed process for 'deprecated'. // and enum DeprecatedEnum { DE_A, DE_B } __attribute__((deprecated)); // expected-note {{'DeprecatedEnum' has been explicitly marked deprecated here}} -__attribute__((deprecated)) typedef enum DeprecatedEnum DeprecatedEnum; typedef enum DeprecatedEnum AnotherDeprecatedEnum; // expected-warning {{'DeprecatedEnum' is deprecated}} +__attribute__((deprecated)) typedef enum DeprecatedEnum DeprecatedEnum; __attribute__((deprecated)) DeprecatedEnum testDeprecated(DeprecatedEnum X) { return X; } enum UnavailableEnum { UE_A, UE_B } __attribute__((unavailable)); // expected-note {{'UnavailableEnum' has been explicitly marked unavailable here}} -__attribute__((unavailable)) typedef enum UnavailableEnum UnavailableEnum; typedef enum UnavailableEnum AnotherUnavailableEnum; // expected-error {{'UnavailableEnum' is unavailable}} + // - +__attribute__((unavailable)) typedef enum UnavailableEnum UnavailableEnum; __attribute__((unavailable)) UnavailableEnum testUnavailable(UnavailableEnum X) { return X; } diff --git a/clang/unittests/Tooling/SourceCodeTest.cpp b/clang/unittests/Tooling/SourceCodeTest.cpp --- a/clang/unittests/Tooling/SourceCodeTest.cpp +++ b/clang/unittests/Tooling/SourceCodeTest.cpp @@ -247,14 +247,25 @@ // Includes attributes. Visitor.runOverAnnotated(R"cpp( - #define ATTR __attribute__((deprecated("message"))) - $r[[ATTR + $r[[__attribute__((deprecated("message"))) int x;]])cpp"); // Includes attributes and comments together. Visitor.runOverAnnotated(R"cpp( - #define ATTR __attribute__((deprecated("message"))) - $r[[ATTR + $r[[__attribute__((deprecated("message"))) + // Comment. + int x;]])cpp"); + + // Includes attributes through macro expansion. + Visitor.runOverAnnotated(R"cpp( + #define MACRO_EXPANSION __attribute__((deprecated("message"))) + $r[[MACRO_EXPANSION + int x;]])cpp"); + + // Includes attributes through macro expansion with comments. + Visitor.runOverAnnotated(R"cpp( + #define MACRO_EXPANSION __attribute__((deprecated("message"))) + $r[[MACRO_EXPANSION // Comment. int x;]])cpp"); } @@ -402,14 +413,25 @@ // Includes attributes. Visit(R"cpp( - #define ATTR __attribute__((deprecated("message"))) - $r[[ATTR + $r[[__attribute__((deprecated("message"))) int x;]])cpp"); // Includes attributes and comments together. Visit(R"cpp( - #define ATTR __attribute__((deprecated("message"))) - $r[[ATTR + $r[[__attribute__((deprecated("message"))) + // Comment. + int x;]])cpp"); + + // Includes attributes through macro expansion. + Visitor.runOverAnnotated(R"cpp( + #define MACRO_EXPANSION __attribute__((deprecated("message"))) + $r[[MACRO_EXPANSION + int x;]])cpp"); + + // Includes attributes through macro expansion with comments. + Visitor.runOverAnnotated(R"cpp( + #define MACRO_EXPANSION __attribute__((deprecated("message"))) + $r[[MACRO_EXPANSION // Comment. int x;]])cpp"); }