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

This revision was automatically updated to reflect the committed changes. Closed by commit rC349845: Add support for namespaces on #pragma clang attribute (authored by epilk, committed by ). Changed prior to commit: https://reviews.llvm.org/D55628?vs=178952=179160#toc Repository: rC Clang

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

aaron.ballman accepted this revision. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/lib/Parse/ParsePragma.cpp:3161 + if (!Tok.is(tok::period)) { +

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

erik.pilkington added inline comments. Comment at: clang/lib/Parse/ParsePragma.cpp:3161 + if (!Tok.is(tok::period)) { +PP.Diag(Tok.getLocation(), diag::err_pragma_attribute_expected_period) +<< II; aaron.ballman wrote: > Can you reuse

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

erik.pilkington updated this revision to Diff 178952. erik.pilkington marked 4 inline comments as done. erik.pilkington added a comment. Add Aaron's testcase, improve the documentation a bit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55628/new/ https://reviews.llvm.org/D55628

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

aaron.ballman added a comment. In D55628#1335841 , @erik.pilkington wrote: > After looking through some users of `#pragma clang attribute`, I don't think > that the begin/end solution is what we want here. Some users of this > attribute write macros

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

erik.pilkington added a comment. Just to be clear, this is just a syntactic change from what I originally posted. The old `#pragma clang attribute push NoReturn (...)` is semantically equivalent to the new `#pragma clang attribute NoReturn.push (...)` CHANGES SINCE LAST ACTION

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

erik.pilkington updated this revision to Diff 178837. erik.pilkington added a comment. After looking through some users of `#pragma clang attribute`, I don't think that the begin/end solution is what we want here. Some users of this attribute write macros that can expand to different attributes

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

dexonsmith added inline comments. Comment at: clang/test/Sema/pragma-attribute-label.c:15 +// Out of order! +#pragma clang attribute pop MY_LABEL + aaron.ballman wrote: > dexonsmith wrote: > > erik.pilkington wrote: > > > aaron.ballman wrote: > > > > dexonsmith

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

aaron.ballman added inline comments. Comment at: clang/test/Sema/pragma-attribute-label.c:15 +// Out of order! +#pragma clang attribute pop MY_LABEL + dexonsmith wrote: > erik.pilkington wrote: > > aaron.ballman wrote: > > > dexonsmith wrote: > > > >

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

dexonsmith added inline comments. Comment at: clang/test/Sema/pragma-attribute-label.c:7 + +#pragma clang attribute pop // expected-error{{'#pragma clang attribute pop' with no matching '#pragma clang attribute push'}} +#pragma clang attribute pop NOT_MY_LABEL //

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

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

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

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:

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

erik.pilkington marked an inline comment as done. erik.pilkington added inline comments. Comment at: clang/test/Sema/pragma-attribute-label.c:7 + +#pragma clang attribute pop // expected-error{{'#pragma clang attribute pop' with no matching '#pragma clang attribute push'}}

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

aaron.ballman added inline comments. Comment at: clang/test/Sema/pragma-attribute-label.c:7 + +#pragma clang attribute pop // expected-error{{'#pragma clang attribute pop' with no matching '#pragma clang attribute push'}} +#pragma clang attribute pop NOT_MY_LABEL //

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

dexonsmith added inline comments. Comment at: clang/test/Sema/pragma-attribute-label.c:7 + +#pragma clang attribute pop // expected-error{{'#pragma clang attribute pop' with no matching '#pragma clang attribute push'}} +#pragma clang attribute pop NOT_MY_LABEL //

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

aaron.ballman 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

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

erik.pilkington created this revision. erik.pilkington added reviewers: arphaman, aaron.ballman. Herald added subscribers: dexonsmith, jkorous. One problem with defining macros that expand to _Pragma("clang attribute")` is that they don't nest very well: // In some header... #define