Support for groups of attributes in #pragma clang attribute

[erik.pilkington]

Summary

This patch allows pushing an empty #pragma clang attribute push, then adding multiple attributes to it, then popping them all with #pragma clang attribute pop, just like #pragma clang diagnostic. We still support the current way of adding these, #pragma clang attribute push(__attribute__((...))), by treating it like a combined push/attribute. This is needed to create macros that behave like:

DO_SOMETHING_BEGIN(attr1, attr2, attr3)
// ...
DO_SOMETHING_END

Which can be called with a variable number of arguments. One possible alternative would be to allow multiple attributes to be specified in the combined push operation, but I chose this method to be more consistent with #pragma clang diagnostic.

Fixes rdar://45496947.

Thanks for taking a look!
Erik

[arphaman]

Thanks! It looks pretty good, just minor nit. could you also update the language extension doc and release notes?

clang/lib/Sema/SemaAttr.cpp

645 ↗

(On Diff #170770)

this for loop should have braces.

[erik.pilkington]

Add documentation and release notes, {}s. Thanks!

[aaron.ballman]

LGTM aside from some small nits.

clang/docs/LanguageExtensions.rst

2666 ↗

(On Diff #171218)

The whitespace changes here are not needed as part of this patch, right? (It's just changing the style of the docs?)

clang/lib/Parse/ParsePragma.cpp

3149 ↗

(On Diff #171218)

I'd prefer dropping the auto here.

[arphaman]

LGTM too when Aaron's comments are addressed

[erik.pilkington]

clang/docs/LanguageExtensions.rst

2666 ↗

(On Diff #171218)

Oh, no it doesn't make a difference to clang. Just thought that it makes it a bit more obvious that this operation is a combination of:

#pragma clang attribute push
#pragma clang attribute (__attribute__((annotate("custom"))), apply_to = function)

clang/lib/Parse/ParsePragma.cpp

3149 ↗

(On Diff #171218)

Sure, done in the commit. Thanks for reviewing!

cfe/trunk/docs/LanguageExtensions.rst

 section of the code. This can be useful when fast contraction is otherwise
 enabled for the translation unit with the ``-ffp-contract=fast`` flag.
 
 Specifying an attribute for multiple declarations (#pragma clang attribute)
 ===========================================================================
 
 The ``#pragma clang attribute`` directive can be used to apply an attribute to
 multiple declarations. The ``#pragma clang attribute push`` variation of the
-directive pushes a new attribute to the attribute stack. The declarations that
-follow the pragma receive the attributes that are on the attribute stack, until
-the stack is cleared using a ``#pragma clang attribute pop`` directive. Multiple
-push directives can be nested inside each other.
+directive pushes a new "scope" of ``#pragma clang attribute`` that attributes
+can be added to. The ``#pragma clang attribute (...)`` variation adds an
+attribute to that scope, and the ``#pragma clang attribute pop`` variation pops
+the scope. You can also use ``#pragma clang attribute push (...)``, which is a
+shorthand for when you want to add one attribute to a new scope. Multiple push
+directives can be nested inside each other.
 
 The attributes that are used in the ``#pragma clang attribute`` directives
 can be written using the GNU-style syntax:
 
 .. code-block:: c++
 
-  #pragma clang attribute push(__attribute__((annotate("custom"))), apply_to = function)
+  #pragma clang attribute push (__attribute__((annotate("custom"))), apply_to = function)
 
   void function(); // The function now has the annotate("custom") attribute
 
   #pragma clang attribute pop
 
 The attributes can also be written using the C++11 style syntax:
 
 .. code-block:: c++
 
-  #pragma clang attribute push([[noreturn]], apply_to = function)
+  #pragma clang attribute push ([[noreturn]], apply_to = function)
 
   void function(); // The function now has the [[noreturn]] attribute
 
   #pragma clang attribute pop
 
 The ``__declspec`` style syntax is also supported:
 
 .. code-block:: c++
 
-  #pragma clang attribute push(__declspec(dllexport), apply_to = function)
+  #pragma clang attribute push (__declspec(dllexport), apply_to = function)
 
   void function(); // The function now has the __declspec(dllexport) attribute
 
   #pragma clang attribute pop
 
 A single push directive accepts only one attribute regardless of the syntax
 used.
 

cfe/trunk/docs/ReleaseNotes.rst

- As of clang 8, `alignof` and `_Alignof` return the ABI alignment of a type,
   as opposed to the preferred alignment. `__alignof` still returns the
   preferred alignment. `-fclang-abi-compat=7` (and previous) will make
   `alignof` and `_Alignof` return preferred alignment again.
 
 
 New Pragmas in Clang
 --------------------
 
-Clang now supports the ...
-
+- Clang now supports adding multiple ``#pragma clang attribute`` attributes into
+  a "scope" of ``push``ed attributes.
 
 Attribute Changes in Clang
 --------------------------
 
 - ...
 
 Windows Support
 ---------------

cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td

def err_pragma_pointers_to_members_unknown_kind : Error<
   "'single_inheritance', 'multiple_inheritance', or 'virtual_inheritance'">;
 // - #pragma clang optimize on/off
 def err_pragma_optimize_invalid_argument : Error<
   "unexpected argument '%0' to '#pragma clang optimize'; "
   "expected 'on' or 'off'">;
 def err_pragma_optimize_extra_argument : Error<
   "unexpected extra argument '%0' to '#pragma clang optimize'">;
 // - #pragma clang attribute
-def err_pragma_attribute_expected_push_pop : Error<
-  "expected 'push' or 'pop' after '#pragma clang attribute'">;
+def err_pragma_attribute_expected_push_pop_paren : Error<
+  "expected 'push', 'pop', or '(' after '#pragma clang attribute'">;
 def err_pragma_attribute_invalid_argument : Error<
   "unexpected argument '%0' to '#pragma clang attribute'; "
   "expected 'push' or 'pop'">;
 def err_pragma_attribute_expected_attribute : Error<
   "expected an attribute after '('">;
 def err_pragma_attribute_expected_attribute_name : Error<
   "expected identifier that represents an attribute name">;
 def err_pragma_attribute_extra_tokens_after_attribute : Error<

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def warn_pragma_attribute_unused : Warning<
   "unused attribute %0 in '#pragma clang attribute push' region">,
   InGroup<PragmaClangAttribute>;
 def note_pragma_attribute_region_ends_here : Note<
   "'#pragma clang attribute push' regions ends here">;
 def err_pragma_attribute_no_pop_eof : Error<"unterminated "
   "'#pragma clang attribute push' at end of file">;
 def note_pragma_attribute_applied_decl_here : Note<
   "when applied to this declaration">;
+def err_pragma_attr_attr_no_push : Error<
+  "'#pragma clang attribute' attribute with no matching "
+  "'#pragma clang attribute push'">;
 
 /// Objective-C parser diagnostics
 def err_duplicate_class_def : Error<
   "duplicate interface definition for class %0">;
 def err_undef_superclass : Error<
   "cannot find interface declaration for %0, superclass of %1">;
 def err_forward_superclass : Error<
   "attempting to use the forward class %0 as superclass of %1">;

cfe/trunk/include/clang/Sema/Sema.h

public:
 
   /// Last section used with #pragma init_seg.
   StringLiteral *CurInitSeg;
   SourceLocation CurInitSegLoc;
 
   /// VisContext - Manages the stack for \#pragma GCC visibility.
   void *VisContext; // Really a "PragmaVisStack*"
 
-  /// This represents the stack of attributes that were pushed by
-  /// \#pragma clang attribute.
+  /// This an attribute introduced by \#pragma clang attribute.
   struct PragmaAttributeEntry {
     SourceLocation Loc;
     ParsedAttr *Attribute;
     SmallVector<attr::SubjectMatchRule, 4> MatchRules;
     bool IsUsed;
   };
-  SmallVector<PragmaAttributeEntry, 2> PragmaAttributeStack;
+
+  /// A push'd group of PragmaAttributeEntries.
+  struct PragmaAttributeGroup {
+    /// The location of the push attribute.
+    SourceLocation Loc;
+    SmallVector<PragmaAttributeEntry, 2> Entries;
+  };
+
+  SmallVector<PragmaAttributeGroup, 2> PragmaAttributeStack;
 
   /// The declaration that is currently receiving an attribute from the
   /// #pragma attribute stack.
   const Decl *PragmaAttributeCurrentTargetDecl;
 
   /// This represents the last location of a "#pragma clang optimize off"
   /// directive if such a directive has not been closed by an "on" yet. If
   /// optimizations are currently "on", this is set to an invalid location.

   /// FreeVisContext - Deallocate and null out VisContext.
   void FreeVisContext();
 
   /// AddCFAuditedAttribute - Check whether we're currently within
   /// '\#pragma clang arc_cf_code_audited' and, if so, consider adding
   /// the appropriate attribute.
   void AddCFAuditedAttribute(Decl *D);
 
-  /// Called on well-formed '\#pragma clang attribute push'.
-  void ActOnPragmaAttributePush(ParsedAttr &Attribute, SourceLocation PragmaLoc,
+  void ActOnPragmaAttributeAttribute(ParsedAttr &Attribute,
+                                     SourceLocation PragmaLoc,
                                      attr::ParsedSubjectMatchRuleSet Rules);
+  void ActOnPragmaAttributeEmptyPush(SourceLocation PragmaLoc);
 
   /// Called on well-formed '\#pragma clang attribute pop'.
   void ActOnPragmaAttributePop(SourceLocation PragmaLoc);
 
   /// Adds the attributes that have been specified using the
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 

cfe/trunk/lib/Parse/ParsePragma.cpp

bool Parser::HandlePragmaLoopHint(LoopHint &Hint) {
 
   Hint.Range = SourceRange(Info->PragmaName.getLocation(),
                            Info->Toks.back().getLocation());
   return true;
 }
 
 namespace {
 struct PragmaAttributeInfo {
-  enum ActionType { Push, Pop };
+  enum ActionType { Push, Pop, Attribute };
   ParsedAttributes &Attributes;
   ActionType Action;
   ArrayRef<Token> Tokens;
 
   PragmaAttributeInfo(ParsedAttributes &Attributes) : Attributes(Attributes) {}
 };
 
 #include "clang/Parse/AttrSubMatchRulesParserStringSwitches.inc"
void Parser::HandlePragmaAttribute() {
   SourceLocation PragmaLoc = Tok.getLocation();
   auto *Info = static_cast<PragmaAttributeInfo *>(Tok.getAnnotationValue());
   if (Info->Action == PragmaAttributeInfo::Pop) {
     ConsumeAnnotationToken();
     Actions.ActOnPragmaAttributePop(PragmaLoc);
     return;
   }
   // Parse the actual attribute with its arguments.
-  assert(Info->Action == PragmaAttributeInfo::Push &&
+  assert(Info->Action == PragmaAttributeInfo::Push ||
+         Info->Action == PragmaAttributeInfo::Attribute &&
              "Unexpected #pragma attribute command");
+
+  if (Info->Action == PragmaAttributeInfo::Push && Info->Tokens.empty()) {
+    ConsumeAnnotationToken();
+    Actions.ActOnPragmaAttributeEmptyPush(PragmaLoc);
+    return;
+  }
+
   PP.EnterTokenStream(Info->Tokens, /*DisableMacroExpansion=*/false);
   ConsumeAnnotationToken();
 
   ParsedAttributes &Attrs = Info->Attributes;
   Attrs.clearListOnly();
 
   auto SkipToEnd = [this]() {
     SkipUntil(tok::eof, StopBeforeMatch);
if (Tok.isNot(tok::eof)) {
     Diag(Tok, diag::err_pragma_attribute_extra_tokens_after_attribute);
     SkipToEnd();
     return;
   }
 
   // Consume the eof terminator token.
   ConsumeToken();
 
-  Actions.ActOnPragmaAttributePush(Attribute, PragmaLoc,
+  // Handle a mixed push/attribute by desurging to a push, then an attribute.
+  if (Info->Action == PragmaAttributeInfo::Push)
+    Actions.ActOnPragmaAttributeEmptyPush(PragmaLoc);
+
+  Actions.ActOnPragmaAttributeAttribute(Attribute, PragmaLoc,
                                         std::move(SubjectMatchRules));
 }
 
 // #pragma GCC visibility comes in two variants:
 //   'push' '(' [visibility] ')'
 //   'pop'
 void PragmaGCCVisibilityHandler::HandlePragma(Preprocessor &PP,
                                               PragmaIntroducerKind Introducer,
                                               Token &VisTok) {
PP.Diag(FirstTok.getLocation(),
             diag::warn_pragma_force_cuda_host_device_bad_arg);
 }
 
 /// Handle the #pragma clang attribute directive.
 ///
 /// The syntax is:
 /// \code
 ///  #pragma clang attribute push(attribute, subject-set)
+///  #pragma clang attribute push
+///  #pragma clang attribute (attribute, subject-set)
 ///  #pragma clang attribute 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.
 ///
 /// This directive instructs the compiler to begin/finish applying the specified
 /// attribute to the set of attribute-specific declarations in the active range
 /// of the pragma.
 void PragmaAttributeHandler::HandlePragma(Preprocessor &PP,
                                           PragmaIntroducerKind Introducer,
                                           Token &FirstToken) {
   Token Tok;
   PP.Lex(Tok);
   auto *Info = new (PP.getPreprocessorAllocator())
       PragmaAttributeInfo(AttributesForPragmaAttribute);
 
-  // Parse the 'push' or 'pop'.
-  if (Tok.isNot(tok::identifier)) {
-    PP.Diag(Tok.getLocation(), diag::err_pragma_attribute_expected_push_pop);
+  if (!Tok.isOneOf(tok::identifier, tok::l_paren)) {
+    PP.Diag(Tok.getLocation(),
+            diag::err_pragma_attribute_expected_push_pop_paren);
     return;
   }
-  const auto *II = Tok.getIdentifierInfo();
+
+  // Determine what action this pragma clang attribute represents.
+  if (Tok.is(tok::l_paren))
+    Info->Action = PragmaAttributeInfo::Attribute;
+  else {
+    const IdentifierInfo *II = Tok.getIdentifierInfo();
     if (II->isStr("push"))
       Info->Action = PragmaAttributeInfo::Push;
     else if (II->isStr("pop"))
       Info->Action = PragmaAttributeInfo::Pop;
     else {
       PP.Diag(Tok.getLocation(), diag::err_pragma_attribute_invalid_argument)
           << PP.getSpelling(Tok);
       return;
     }
+
     PP.Lex(Tok);
+  }
 
   // Parse the actual attribute.
-  if (Info->Action == PragmaAttributeInfo::Push) {
+  if ((Info->Action == PragmaAttributeInfo::Push && Tok.isNot(tok::eod)) ||
+      Info->Action == PragmaAttributeInfo::Attribute) {
     if (Tok.isNot(tok::l_paren)) {
       PP.Diag(Tok.getLocation(), diag::err_expected) << tok::l_paren;
       return;
     }
     PP.Lex(Tok);
 
     // Lex the attribute tokens.
     SmallVector<Token, 16> AttributeTokens;

cfe/trunk/lib/Sema/SemaAttr.cpp

if (I.index())
       OS << (I.index() == Rules.size() - 1 ? ", and " : ", ");
     OS << "'" << attr::getSubjectMatchRuleSpelling(I.value()) << "'";
   }
   return OS.str();
 }
 
 } // end anonymous namespace
 
-void Sema::ActOnPragmaAttributePush(ParsedAttr &Attribute,
-                                    SourceLocation PragmaLoc,
-                                    attr::ParsedSubjectMatchRuleSet Rules) {
+void Sema::ActOnPragmaAttributeAttribute(
+    ParsedAttr &Attribute, SourceLocation PragmaLoc,
   SmallVector<attr::SubjectMatchRule, 4> SubjectMatchRules;
   // Gather the subject match rules that are supported by the attribute.
   SmallVector<std::pair<attr::SubjectMatchRule, bool>, 4>
       StrictSubjectMatchRuleSet;
   Attribute.getMatchRules(LangOpts, StrictSubjectMatchRuleSet);
 
   // Figure out which subject matching rules are valid.
   if (StrictSubjectMatchRuleSet.empty()) {
if (!Rules.empty()) {
     for (const auto &Rule : Rules) {
       ExtraRules.push_back(attr::SubjectMatchRule(Rule.first));
       Diagnostic << FixItHint::CreateRemoval(
           replacementRangeForListElement(*this, Rule.second));
     }
     Diagnostic << attrMatcherRuleListToString(ExtraRules);
   }
 
-  PragmaAttributeStack.push_back(
+  if (PragmaAttributeStack.empty()) {
+    Diag(PragmaLoc, diag::err_pragma_attr_attr_no_push);
+    return;
+  }
+
+  PragmaAttributeStack.back().Entries.push_back(
       {PragmaLoc, &Attribute, std::move(SubjectMatchRules), /*IsUsed=*/false});
 }
 
+void Sema::ActOnPragmaAttributeEmptyPush(SourceLocation PragmaLoc) {
+  PragmaAttributeStack.emplace_back();
+  PragmaAttributeStack.back().Loc = PragmaLoc;
+}
+
 void Sema::ActOnPragmaAttributePop(SourceLocation PragmaLoc) {
   if (PragmaAttributeStack.empty()) {
     Diag(PragmaLoc, diag::err_pragma_attribute_stack_mismatch);
     return;
   }
-  const PragmaAttributeEntry &Entry = PragmaAttributeStack.back();
+
+  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);
     }
+  }
+
   PragmaAttributeStack.pop_back();
 }
 
 void Sema::AddPragmaAttributes(Scope *S, Decl *D) {
   if (PragmaAttributeStack.empty())
     return;
-  for (auto &Entry : PragmaAttributeStack) {
+  for (auto &Group : PragmaAttributeStack) {
+    for (auto &Entry : Group.Entries) {
       ParsedAttr *Attribute = Entry.Attribute;
       assert(Attribute && "Expected an attribute");
 
       // Ensure that the attribute can be applied to the given declaration.
       bool Applies = false;
       for (const auto &Rule : Entry.MatchRules) {
         if (Attribute->appliesToDecl(D, Rule)) {
           Applies = true;
           break;
         }
       }
       if (!Applies)
         continue;
       Entry.IsUsed = true;
       PragmaAttributeCurrentTargetDecl = D;
       ParsedAttributesView Attrs;
       Attrs.addAtEnd(Attribute);
       ProcessDeclAttributeList(S, D, Attrs);
       PragmaAttributeCurrentTargetDecl = nullptr;
     }
   }
+}
 
 void Sema::PrintPragmaAttributeInstantiationPoint() {
   assert(PragmaAttributeCurrentTargetDecl && "Expected an active declaration");
   Diags.Report(PragmaAttributeCurrentTargetDecl->getBeginLoc(),
                diag::note_pragma_attribute_applied_decl_here);
 }
 
 void Sema::DiagnoseUnterminatedPragmaAttribute() {

cfe/trunk/test/Parser/pragma-attribute.cpp

 #pragma clang attribute push(__attribute__((annotate("test"))), apply_to = any( function, enum, function )) // expected-error {{duplicate attribute subject matcher 'function'}}
 #pragma clang attribute push(__attribute__((annotate("test"))), apply_to = any( enum, enum, function )) // expected-error {{duplicate attribute subject matcher 'enum'}}
 
 #pragma clang attribute push(__attribute__((annotate("test"))), apply_to = any( variable(is_global), variable(is_global) )) // expected-error {{duplicate attribute subject matcher 'variable(is_global)'}}
 #pragma clang attribute push(__attribute__((annotate("test"))), apply_to = any( variable(is_global), function, variable(is_global), variable(is_global) )) // expected-error 2 {{duplicate attribute subject matcher 'variable(is_global)'}}
 #pragma clang attribute push(__attribute__((annotate("test"))), apply_to = any( variable(unless(is_parameter)), variable(unless(is_parameter)) )) // expected-error {{duplicate attribute subject matcher 'variable(unless(is_parameter))'}}
 #pragma clang attribute push(__attribute__((annotate("test"))), apply_to = any( variable(unless(is_parameter)), variable(unless(is_parameter)), enum, variable(unless(is_parameter)) )) // expected-error 2 {{duplicate attribute subject matcher 'variable(unless(is_parameter))'}}
 
-#pragma clang attribute // expected-error {{expected 'push' or 'pop' after '#pragma clang attribute'}}
-#pragma clang attribute 42 // expected-error {{expected 'push' or 'pop' after '#pragma clang attribute'}}
+#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 push // expected-error {{expected '('}}
+#pragma clang attribute push
+#pragma clang attribute pop
 #pragma clang attribute push ( // expected-error {{expected an attribute after '('}}
 #pragma clang attribute push (__attribute__((annotate)) // expected-error {{expected ')'}}
 #pragma clang attribute push () // expected-error {{expected an attribute after '('}}
 
 #pragma clang attribute push (__attribute__((annotate("test"))), apply_to = function) () // expected-warning {{extra tokens at end of '#pragma clang attribute'}}
 // expected-error@-1 {{expected unqualified-id}}
 // expected-error@-2 {{unterminated '#pragma clang attribute push' at end of file}}
 

cfe/trunk/test/Sema/pragma-attribute.c

 
 void optnone2() { }
 
 __attribute__((always_inline)) void optnone3() { } // expected-warning {{'always_inline' attribute ignored}}
 // expected-note@-1 {{when applied to this declaration}}
 
 #pragma clang attribute pop
 
+#pragma clang attribute push (__attribute__((annotate())), apply_to = function) // expected-error{{'annotate' attribute takes one argument}}
+#pragma clang attribute (__attribute__((annotate())), apply_to = function) // expected-error{{'annotate' attribute takes one argument}}
+
+void fun(); // expected-note 2 {{when applied to this declaration}}
+
+#pragma clang attribute pop
+#pragma clang attribute pop // expected-error{{'#pragma clang attribute pop' with no matching '#pragma clang attribute push'}}
+
+
+#pragma clang attribute push
+#pragma clang attribute (__attribute__((annotate())), apply_to = function) // expected-error 2 {{'annotate' attribute takes one argument}}
+
+void fun2(); // expected-note {{when applied to this declaration}}
+
+#pragma clang attribute push (__attribute__((annotate())), apply_to = function) // expected-error{{'annotate' attribute takes one argument}}
+void fun3(); // expected-note 2 {{when applied to this declaration}}
+#pragma clang attribute pop
+
+#pragma clang attribute pop
+#pragma clang attribute pop // expected-error{{'#pragma clang attribute pop' with no matching '#pragma clang attribute push'}}
+
+#pragma clang attribute (__attribute__((annotate)), apply_to = function) // expected-error{{'#pragma clang attribute' attribute with no matching '#pragma clang attribute push}}
+
 #pragma clang attribute push ([[]], apply_to = function) // A noop
 
 #pragma clang attribute pop // expected-error {{'#pragma clang attribute pop' with no matching '#pragma clang attribute push'}}
 
 #pragma clang attribute push (__attribute__((annotate("func"))), apply_to = function) // expected-error {{unterminated '#pragma clang attribute push' at end of file}}
 
 void function();