r349845 - Add support for namespaces on #pragma clang attribute
Erik Pilkington via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 20 14:32:04 PST 2018
Author: epilk
Date: Thu Dec 20 14:32:04 2018
New Revision: 349845
URL: http://llvm.org/viewvc/llvm-project?rev=349845&view=rev
Log:
Add support for namespaces on #pragma clang attribute
Namespaces are introduced by adding an "identifier." before a
push/pop directive. Pop directives with namespaces can only pop a
attribute group that was pushed with the same namespace. Push and pop
directives that don't opt into namespaces have the same semantics.
This is necessary to prevent a pitfall of using multiple #pragma
clang attribute directives spread out in a large file, particularly
when macros are involved. It isn't easy to see which pop corripsonds
to which push, so its easy to inadvertently pop the wrong group.
Differential revision: https://reviews.llvm.org/D55628
Added:
cfe/trunk/test/Sema/pragma-attribute-namespace.c
Modified:
cfe/trunk/docs/LanguageExtensions.rst
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParsePragma.cpp
cfe/trunk/lib/Sema/SemaAttr.cpp
cfe/trunk/test/Parser/pragma-attribute.cpp
Modified: cfe/trunk/docs/LanguageExtensions.rst
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=349845&r1=349844&r2=349845&view=diff
==============================================================================
--- cfe/trunk/docs/LanguageExtensions.rst (original)
+++ cfe/trunk/docs/LanguageExtensions.rst Thu Dec 20 14:32:04 2018
@@ -2697,6 +2697,36 @@ The ``__declspec`` style syntax is also
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
-------------------
Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=349845&r1=349844&r2=349845&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Thu Dec 20 14:32:04 2018
@@ -1100,6 +1100,13 @@ def err_pragma_attribute_unknown_subject
"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">;
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=349845&r1=349844&r2=349845&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Dec 20 14:32:04 2018
@@ -815,7 +815,8 @@ def err_pragma_attribute_matcher_negated
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<PragmaClangAttribute>;
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=349845&r1=349844&r2=349845&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Dec 20 14:32:04 2018
@@ -503,6 +503,8 @@ public:
struct PragmaAttributeGroup {
/// The location of the push attribute.
SourceLocation Loc;
+ /// The namespace of this push group.
+ const IdentifierInfo *Namespace;
SmallVector<PragmaAttributeEntry, 2> Entries;
};
@@ -8494,10 +8496,12 @@ public:
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.
Modified: cfe/trunk/lib/Parse/ParsePragma.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParsePragma.cpp?rev=349845&r1=349844&r2=349845&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParsePragma.cpp (original)
+++ cfe/trunk/lib/Parse/ParsePragma.cpp Thu Dec 20 14:32:04 2018
@@ -1139,6 +1139,7 @@ struct PragmaAttributeInfo {
enum ActionType { Push, Pop, Attribute };
ParsedAttributes &Attributes;
ActionType Action;
+ const IdentifierInfo *Namespace = nullptr;
ArrayRef<Token> Tokens;
PragmaAttributeInfo(ParsedAttributes &Attributes) : Attributes(Attributes) {}
@@ -1393,7 +1394,7 @@ void Parser::HandlePragmaAttribute() {
auto *Info = static_cast<PragmaAttributeInfo *>(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 @@ void Parser::HandlePragmaAttribute() {
if (Info->Action == PragmaAttributeInfo::Push && Info->Tokens.empty()) {
ConsumeAnnotationToken();
- Actions.ActOnPragmaAttributeEmptyPush(PragmaLoc);
+ Actions.ActOnPragmaAttributeEmptyPush(PragmaLoc, Info->Namespace);
return;
}
@@ -1555,7 +1556,7 @@ void Parser::HandlePragmaAttribute() {
// 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 @@ void PragmaForceCUDAHostDeviceHandler::H
///
/// 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 @@ void PragmaAttributeHandler::HandlePragm
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 @@ void PragmaAttributeHandler::HandlePragm
}
// 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;
Modified: cfe/trunk/lib/Sema/SemaAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaAttr.cpp?rev=349845&r1=349844&r2=349845&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaAttr.cpp Thu Dec 20 14:32:04 2018
@@ -631,28 +631,46 @@ void Sema::ActOnPragmaAttributeAttribute
{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) {
Modified: cfe/trunk/test/Parser/pragma-attribute.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/pragma-attribute.cpp?rev=349845&r1=349844&r2=349845&view=diff
==============================================================================
--- cfe/trunk/test/Parser/pragma-attribute.cpp (original)
+++ cfe/trunk/test/Parser/pragma-attribute.cpp Thu Dec 20 14:32:04 2018
@@ -102,7 +102,7 @@ void function();
#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
Added: cfe/trunk/test/Sema/pragma-attribute-namespace.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/pragma-attribute-namespace.c?rev=349845&view=auto
==============================================================================
--- cfe/trunk/test/Sema/pragma-attribute-namespace.c (added)
+++ cfe/trunk/test/Sema/pragma-attribute-namespace.c Thu Dec 20 14:32:04 2018
@@ -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
More information about the cfe-commits
mailing list