erik.pilkington updated this revision to Diff 178254.
erik.pilkington marked 7 inline comments as done.
erik.pilkington added a comment.
Address @aaron.ballman comments. Thanks!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55628/new/
https://reviews.llvm.org/D55628
Files:
clang/docs/LanguageExtensions.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParsePragma.cpp
clang/lib/Sema/SemaAttr.cpp
clang/test/Sema/pragma-attribute-label.c
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
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/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<Token> Tokens;
PragmaAttributeInfo(ParsedAttributes &Attributes) : Attributes(Attributes) {}
@@ -1393,7 +1394,7 @@
auto *Info = static_cast<PragmaAttributeInfo *>(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/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<PragmaAttributeEntry, 2> 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/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<PragmaClangAttribute>;
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
-------------------
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
