Index: clang/docs/LanguageExtensions.rst =================================================================== --- clang/docs/LanguageExtensions.rst +++ clang/docs/LanguageExtensions.rst @@ -2697,6 +2697,22 @@ 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 label to your push/pop directives. A pop directive with a +label will pop the innermost push that has that same label. This will ensure +that another macro's ``pop`` won't inadvertently pop your attribute. For +instance: + +.. code-block:: c++ + + #define ASSUME_NORETURN_BEGIN _Pragma("clang attribute push ASSUME_NORETURN ([[noreturn]], apply_to = function) + #define ASSUME_NORETURN_END _Pragma("clang attribute pop ASSUME_NORETURN) + + ASSUME_NORETURN_BEGIN + void function(); // function has [[noreturn]] + ASSUME_NORETURN_END + Subject Match Rules ------------------- Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -836,7 +836,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 pop%select{ %1|}0' with no matching" + " '#pragma clang attribute push%select{ %1|}0'">; def warn_pragma_attribute_unused : Warning< "unused attribute %0 in '#pragma clang attribute push' region">, InGroup; Index: clang/include/clang/Sema/Sema.h =================================================================== --- clang/include/clang/Sema/Sema.h +++ clang/include/clang/Sema/Sema.h @@ -503,6 +503,8 @@ struct PragmaAttributeGroup { /// The location of the push attribute. SourceLocation Loc; + /// The label of this push group. + const IdentifierInfo *Label; SmallVector Entries; }; @@ -8498,10 +8500,12 @@ void ActOnPragmaAttributeAttribute(ParsedAttr &Attribute, SourceLocation PragmaLoc, attr::ParsedSubjectMatchRuleSet Rules); - void ActOnPragmaAttributeEmptyPush(SourceLocation PragmaLoc); + void ActOnPragmaAttributeEmptyPush(SourceLocation PragmaLoc, + const IdentifierInfo *PushLabel); /// Called on well-formed '\#pragma clang attribute pop'. - void ActOnPragmaAttributePop(SourceLocation PragmaLoc); + void ActOnPragmaAttributePop(SourceLocation PragmaLoc, + const IdentifierInfo *PushLabel); /// Adds the attributes that have been specified using the /// '\#pragma clang attribute push' directives to the given declaration. Index: clang/lib/Parse/ParsePragma.cpp =================================================================== --- clang/lib/Parse/ParsePragma.cpp +++ clang/lib/Parse/ParsePragma.cpp @@ -1139,6 +1139,7 @@ enum ActionType { Push, Pop, Attribute }; ParsedAttributes &Attributes; ActionType Action; + const IdentifierInfo *PushLabel = 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->PushLabel); 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->PushLabel); 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->PushLabel); Actions.ActOnPragmaAttributeAttribute(Attribute, PragmaLoc, std::move(SubjectMatchRules)); @@ -3163,6 +3164,14 @@ PP.Lex(Tok); } + // Parse the optional push/pop label. + if ((Info->Action == PragmaAttributeInfo::Push || + Info->Action == PragmaAttributeInfo::Pop) && + Tok.is(tok::identifier)) { + Info->PushLabel = Tok.getIdentifierInfo(); + PP.Lex(Tok); + } + // Parse the actual attribute. if ((Info->Action == PragmaAttributeInfo::Push && Tok.isNot(tok::eod)) || Info->Action == PragmaAttributeInfo::Attribute) { Index: clang/lib/Sema/SemaAttr.cpp =================================================================== --- clang/lib/Sema/SemaAttr.cpp +++ clang/lib/Sema/SemaAttr.cpp @@ -631,28 +631,45 @@ {PragmaLoc, &Attribute, std::move(SubjectMatchRules), /*IsUsed=*/false}); } -void Sema::ActOnPragmaAttributeEmptyPush(SourceLocation PragmaLoc) { +void Sema::ActOnPragmaAttributeEmptyPush(SourceLocation PragmaLoc, + const IdentifierInfo *PushLabel) { PragmaAttributeStack.emplace_back(); PragmaAttributeStack.back().Loc = PragmaLoc; + PragmaAttributeStack.back().Label = PushLabel; } -void Sema::ActOnPragmaAttributePop(SourceLocation PragmaLoc) { +void Sema::ActOnPragmaAttributePop(SourceLocation PragmaLoc, + const IdentifierInfo *PushLabel) { 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 group that corresponds to + // PushLabel. Note that this works fine if no label is present, think of + // unlabeled push/pops as having an implicit "nullptr" label. + for (size_t Index = PragmaAttributeStack.size(); Index;) { + --Index; + if (PragmaAttributeStack[Index].Label == PushLabel) { + 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 (PushLabel) + Diag(PragmaLoc, diag::err_pragma_attribute_stack_mismatch) + << 0 << PushLabel->getName(); + else + Diag(PragmaLoc, diag::err_pragma_attribute_stack_mismatch) << 1; } void Sema::AddPragmaAttributes(Scope *S, Decl *D) { Index: clang/test/Sema/pragma-attribute-label.c =================================================================== --- /dev/null +++ clang/test/Sema/pragma-attribute-label.c @@ -0,0 +1,26 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +#define ASSUME_MY_ANNOTATE_BEGIN _Pragma("clang attribute push MY_LABEL (__attribute__((annotate)), apply_to=function)") +#define ASSUME_MY_ANNOTATE_END _Pragma("clang attribute pop MY_LABEL") + +#define ASSUME_MY_OTHER_ANNOTATE_BEGIN _Pragma("clang attribute push MY_OTHER_LABEL (__attribute__((annotate)), apply_to=function)") +#define ASSUME_MY_OTHER_ANNOTATE_END _Pragma("clang attribute pop MY_OTHER_LABEL") + + +ASSUME_MY_ANNOTATE_BEGIN // 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 pop NOT_MY_LABEL // expected-error{{'#pragma clang attribute pop NOT_MY_LABEL' with no matching '#pragma clang attribute push NOT_MY_LABEL'}} + +ASSUME_MY_OTHER_ANNOTATE_BEGIN // expected-error 2 {{'annotate' attribute}} + +int some_other_func(); // expected-note 2 {{when applied to this declaration}} + +// Out of order! +ASSUME_MY_ANNOTATE_END + +int some_other_other_func(); // expected-note 1 {{when applied to this declaration}} + +ASSUME_MY_OTHER_ANNOTATE_END