Index: docs/LanguageExtensions.rst =================================================================== --- docs/LanguageExtensions.rst +++ docs/LanguageExtensions.rst @@ -2697,6 +2697,36 @@ A single push directive accepts only one attribute regardless of the syntax used. +Because multiple push directives can be nested, if you're writing a macro that +expands to ``_Pragma("clang attribute")`` it's good hygiene (though not +required) to add a namespace to your push/pop directives. A pop directive with a +namespace will pop the innermost push that has that same namespace. This will +ensure that another macro's ``pop`` won't inadvertently pop your attribute. Note +that an ``pop`` without a namespace will pop the innermost ``push`` without a +namespace. ``push``es with a namespace can only be popped by ``pop`` with the +same namespace. For instance: + +.. code-block:: c++ + + #define ASSUME_NORETURN_BEGIN _Pragma("clang attribute AssumeNoreturn.push ([[noreturn]], apply_to = function)") + #define ASSUME_NORETURN_END _Pragma("clang attribute AssumeNoreturn.pop") + + #define ASSUME_UNAVAILABLE_BEGIN _Pragma("clang attribute Unavailable.push (__attribute__((unavailable)), apply_to=function)") + #define ASSUME_UNAVAILABLE_END _Pragma("clang attribute Unavailable.pop") + + + ASSUME_NORETURN_BEGIN + ASSUME_UNAVAILABLE_BEGIN + void function(); // function has [[noreturn]] and __attribute__((unavailable)) + ASSUME_NORETURN_END + void other_function(); // function has __attribute__((unavailable)) + ASSUME_UNAVAILABLE_END + +Without the namespaces on the macros, ``other_function`` will be annotated with +``[[noreturn]]`` instead of ``__attribute__((unavailable))``. This may seem like +a contrived example, but its very possible for this kind of situation to appear +in real code if the pragmas are spread out accross a large file. + Subject Match Rules ------------------- Index: include/clang/Basic/DiagnosticParseKinds.td =================================================================== --- include/clang/Basic/DiagnosticParseKinds.td +++ include/clang/Basic/DiagnosticParseKinds.td @@ -1100,6 +1100,13 @@ "sub-rules: %3}2">; def err_pragma_attribute_duplicate_subject : Error< "duplicate attribute subject matcher '%0'">; +def err_pragma_attribute_expected_period : Error< + "expected '.' after pragma attribute namespace %0">; +def err_pragma_attribute_namespace_on_attribute : Error< + "namespace can only apply to 'push' or 'pop' directives">; +def note_pragma_attribute_namespace_on_attribute : Note< + "omit the namespace to add attributes to the most-recently" + " pushed attribute group">; def err_opencl_unroll_hint_on_non_loop : Error< "OpenCL only supports 'opencl_unroll_hint' attribute on for, while, and do statements">; Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -815,7 +815,8 @@ def err_pragma_attribute_invalid_matchers : Error< "attribute %0 can't be applied to %1">; def err_pragma_attribute_stack_mismatch : Error< - "'#pragma clang attribute pop' with no matching '#pragma clang attribute push'">; + "'#pragma clang attribute %select{%1.|}0pop' with no matching" + " '#pragma clang attribute %select{%1.|}0push'">; def warn_pragma_attribute_unused : Warning< "unused attribute %0 in '#pragma clang attribute push' region">, InGroup; Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -503,6 +503,8 @@ struct PragmaAttributeGroup { /// The location of the push attribute. SourceLocation Loc; + /// The namespace of this push group. + const IdentifierInfo *Namespace; SmallVector Entries; }; @@ -8494,10 +8496,12 @@ void ActOnPragmaAttributeAttribute(ParsedAttr &Attribute, SourceLocation PragmaLoc, attr::ParsedSubjectMatchRuleSet Rules); - void ActOnPragmaAttributeEmptyPush(SourceLocation PragmaLoc); + void ActOnPragmaAttributeEmptyPush(SourceLocation PragmaLoc, + const IdentifierInfo *Namespace); /// Called on well-formed '\#pragma clang attribute pop'. - void ActOnPragmaAttributePop(SourceLocation PragmaLoc); + void ActOnPragmaAttributePop(SourceLocation PragmaLoc, + const IdentifierInfo *Namespace); /// Adds the attributes that have been specified using the /// '\#pragma clang attribute push' directives to the given declaration. Index: lib/Parse/ParsePragma.cpp =================================================================== --- lib/Parse/ParsePragma.cpp +++ lib/Parse/ParsePragma.cpp @@ -1139,6 +1139,7 @@ enum ActionType { Push, Pop, Attribute }; ParsedAttributes &Attributes; ActionType Action; + const IdentifierInfo *Namespace = nullptr; ArrayRef Tokens; PragmaAttributeInfo(ParsedAttributes &Attributes) : Attributes(Attributes) {} @@ -1393,7 +1394,7 @@ auto *Info = static_cast(Tok.getAnnotationValue()); if (Info->Action == PragmaAttributeInfo::Pop) { ConsumeAnnotationToken(); - Actions.ActOnPragmaAttributePop(PragmaLoc); + Actions.ActOnPragmaAttributePop(PragmaLoc, Info->Namespace); return; } // Parse the actual attribute with its arguments. @@ -1403,7 +1404,7 @@ if (Info->Action == PragmaAttributeInfo::Push && Info->Tokens.empty()) { ConsumeAnnotationToken(); - Actions.ActOnPragmaAttributeEmptyPush(PragmaLoc); + Actions.ActOnPragmaAttributeEmptyPush(PragmaLoc, Info->Namespace); return; } @@ -1555,7 +1556,7 @@ // Handle a mixed push/attribute by desurging to a push, then an attribute. if (Info->Action == PragmaAttributeInfo::Push) - Actions.ActOnPragmaAttributeEmptyPush(PragmaLoc); + Actions.ActOnPragmaAttributeEmptyPush(PragmaLoc, Info->Namespace); Actions.ActOnPragmaAttributeAttribute(Attribute, PragmaLoc, std::move(SubjectMatchRules)); @@ -3118,12 +3119,22 @@ /// /// The syntax is: /// \code -/// #pragma clang attribute push(attribute, subject-set) +/// #pragma clang attribute push (attribute, subject-set) /// #pragma clang attribute push /// #pragma clang attribute (attribute, subject-set) /// #pragma clang attribute pop /// \endcode /// +/// There are also 'namespace' variants of push and pop directives. The bare +/// '#pragma clang attribute (attribute, subject-set)' version doesn't require a +/// namespace, since it always applies attributes to the most recently pushed +/// group, regardless of namespace. +/// \code +/// #pragma clang attribute namespace.push (attribute, subject-set) +/// #pragma clang attribute namespace.push +/// #pragma clang attribute namespace.pop +/// \endcode +/// /// The subject-set clause defines the set of declarations which receive the /// attribute. Its exact syntax is described in the LanguageExtensions document /// in Clang's documentation. @@ -3139,6 +3150,22 @@ auto *Info = new (PP.getPreprocessorAllocator()) PragmaAttributeInfo(AttributesForPragmaAttribute); + // Parse the optional namespace followed by a period. + if (Tok.is(tok::identifier)) { + IdentifierInfo *II = Tok.getIdentifierInfo(); + if (!II->isStr("push") && !II->isStr("pop")) { + Info->Namespace = II; + PP.Lex(Tok); + + if (!Tok.is(tok::period)) { + PP.Diag(Tok.getLocation(), diag::err_pragma_attribute_expected_period) + << II; + return; + } + PP.Lex(Tok); + } + } + if (!Tok.isOneOf(tok::identifier, tok::l_paren)) { PP.Diag(Tok.getLocation(), diag::err_pragma_attribute_expected_push_pop_paren); @@ -3146,9 +3173,16 @@ } // Determine what action this pragma clang attribute represents. - if (Tok.is(tok::l_paren)) + if (Tok.is(tok::l_paren)) { + if (Info->Namespace) { + PP.Diag(Tok.getLocation(), + diag::err_pragma_attribute_namespace_on_attribute); + PP.Diag(Tok.getLocation(), + diag::note_pragma_attribute_namespace_on_attribute); + return; + } Info->Action = PragmaAttributeInfo::Attribute; - else { + } else { const IdentifierInfo *II = Tok.getIdentifierInfo(); if (II->isStr("push")) Info->Action = PragmaAttributeInfo::Push; Index: lib/Sema/SemaAttr.cpp =================================================================== --- lib/Sema/SemaAttr.cpp +++ lib/Sema/SemaAttr.cpp @@ -631,28 +631,46 @@ {PragmaLoc, &Attribute, std::move(SubjectMatchRules), /*IsUsed=*/false}); } -void Sema::ActOnPragmaAttributeEmptyPush(SourceLocation PragmaLoc) { +void Sema::ActOnPragmaAttributeEmptyPush(SourceLocation PragmaLoc, + const IdentifierInfo *Namespace) { PragmaAttributeStack.emplace_back(); PragmaAttributeStack.back().Loc = PragmaLoc; + PragmaAttributeStack.back().Namespace = Namespace; } -void Sema::ActOnPragmaAttributePop(SourceLocation PragmaLoc) { +void Sema::ActOnPragmaAttributePop(SourceLocation PragmaLoc, + const IdentifierInfo *Namespace) { if (PragmaAttributeStack.empty()) { - Diag(PragmaLoc, diag::err_pragma_attribute_stack_mismatch); + Diag(PragmaLoc, diag::err_pragma_attribute_stack_mismatch) << 1; return; } - for (const PragmaAttributeEntry &Entry : - PragmaAttributeStack.back().Entries) { - if (!Entry.IsUsed) { - assert(Entry.Attribute && "Expected an attribute"); - Diag(Entry.Attribute->getLoc(), diag::warn_pragma_attribute_unused) - << Entry.Attribute->getName(); - Diag(PragmaLoc, diag::note_pragma_attribute_region_ends_here); + // Dig back through the stack trying to find the most recently pushed group + // that in Namespace. Note that this works fine if no namespace is present, + // think of push/pops without namespaces as having an implicit "nullptr" + // namespace. + for (size_t Index = PragmaAttributeStack.size(); Index;) { + --Index; + if (PragmaAttributeStack[Index].Namespace == Namespace) { + for (const PragmaAttributeEntry &Entry : + PragmaAttributeStack[Index].Entries) { + if (!Entry.IsUsed) { + assert(Entry.Attribute && "Expected an attribute"); + Diag(Entry.Attribute->getLoc(), diag::warn_pragma_attribute_unused) + << *Entry.Attribute; + Diag(PragmaLoc, diag::note_pragma_attribute_region_ends_here); + } + } + PragmaAttributeStack.erase(PragmaAttributeStack.begin() + Index); + return; } } - PragmaAttributeStack.pop_back(); + if (Namespace) + Diag(PragmaLoc, diag::err_pragma_attribute_stack_mismatch) + << 0 << Namespace->getName(); + else + Diag(PragmaLoc, diag::err_pragma_attribute_stack_mismatch) << 1; } void Sema::AddPragmaAttributes(Scope *S, Decl *D) { Index: test/Parser/pragma-attribute.cpp =================================================================== --- test/Parser/pragma-attribute.cpp +++ test/Parser/pragma-attribute.cpp @@ -102,7 +102,7 @@ #pragma clang attribute // expected-error {{expected 'push', 'pop', or '(' after '#pragma clang attribute'}} #pragma clang attribute 42 // expected-error {{expected 'push', 'pop', or '(' after '#pragma clang attribute'}} -#pragma clang attribute pushpop // expected-error {{unexpected argument 'pushpop' to '#pragma clang attribute'; expected 'push' or 'pop'}} +#pragma clang attribute pushpop // expected-error {{expected '.' after pragma attribute namespace 'pushpop'}} #pragma clang attribute push #pragma clang attribute pop Index: test/Sema/pragma-attribute-namespace.c =================================================================== --- test/Sema/pragma-attribute-namespace.c +++ test/Sema/pragma-attribute-namespace.c @@ -0,0 +1,37 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +#pragma clang attribute MyNamespace.push (__attribute__((annotate)), apply_to=function) // expected-error 2 {{'annotate' attribute}} + +int some_func(); // expected-note{{when applied to this declaration}} + +#pragma clang attribute pop // expected-error{{'#pragma clang attribute pop' with no matching '#pragma clang attribute push'}} +#pragma clang attribute NotMyNamespace.pop // expected-error{{'#pragma clang attribute NotMyNamespace.pop' with no matching '#pragma clang attribute NotMyNamespace.push'}} + +#pragma clang attribute MyOtherNamespace.push (__attribute__((annotate)), apply_to=function) // expected-error 2 {{'annotate' attribute}} + +int some_other_func(); // expected-note 2 {{when applied to this declaration}} + +// Out of order! +#pragma clang attribute MyNamespace.pop + +int some_other_other_func(); // expected-note 1 {{when applied to this declaration}} + +#pragma clang attribute MyOtherNamespace.pop + +#pragma clang attribute Misc. () // expected-error{{namespace can only apply to 'push' or 'pop' directives}} expected-note {{omit the namespace to add attributes to the most-recently pushed attribute group}} + +#pragma clang attribute Misc push // expected-error{{expected '.' after pragma attribute namespace 'Misc'}} + +// Test how pushes with namespaces interact with pushes without namespaces. + +#pragma clang attribute Merp.push (__attribute__((annotate)), apply_to=function) // expected-error{{'annotate' attribute}} +#pragma clang attribute push (__attribute__((annotate)), apply_to=function) // expected-warning {{unused attribute}} +#pragma clang attribute pop // expected-note{{ends here}} +int test(); // expected-note{{when applied to this declaration}} +#pragma clang attribute Merp.pop + +#pragma clang attribute push (__attribute__((annotate)), apply_to=function) // expected-warning {{unused attribute}} +#pragma clang attribute Merp.push (__attribute__((annotate)), apply_to=function) // expected-error{{'annotate' attribute}} +#pragma clang attribute pop // expected-note{{ends here}} +int test2(); // expected-note{{when applied to this declaration}} +#pragma clang attribute Merp.pop