[PATCH] D55628: Add support for "labels" on push/pop directives in #pragma clang attribute

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.


================
Comment at: clang/docs/LanguageExtensions.rst:2667
+Because multiple push directives can be nested, if you're writing a macro that
+expands to ``_Pragma("clang attribute")`` it's good hygiene to add a label to
+your push/pop directives. A pop directive with a label will pop the innermost
----------------
aaron.ballman wrote:
> The documentation reads like the label is optional, but the examples make it look required (because all examples have the label present). Can you clarify what happens when the label is omitted and keep an example that shows it?
Sure, I moved this to the end of the section, added an example, and left the manual `#pragma clang attribute` below intact to make it more clear.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:840-842
+def err_pragma_attribute_stack_mismatch_label : Error<
+  "'#pragma clang attribute pop %0' with no matching "
+  "'#pragma clang attribute push %0'">;
----------------
aaron.ballman wrote:
> Can this be combined with the diagnostic above using `%select` (given that the wording is basically identical)?
Sure, good point.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55628/new/

https://reviews.llvm.org/D55628