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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 20 05:17:24 PST 2018


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)) {
+        PP.Diag(Tok.getLocation(), diag::err_pragma_attribute_expected_period)
+            << II;
----------------
erik.pilkington wrote:
> aaron.ballman wrote:
> > Can you reuse `diag::err_expected_after` instead of making a new diagnostic?
> I was kinda concerned about how we would diagnose a case like this:
> ```
> #pragma clang attribute add (...) // add isn't a thing!
> ```
> A generic diagnostic about the missing `.` would be pretty unhelpful. The custom diagnostic reads as "expected '.' after pragma attribute namespace 'add'", which makes how the parser interpreted the code a lot more clear.
Okay, that's a good reason to go with a separate diagnostic, thanks!


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

https://reviews.llvm.org/D55628





More information about the cfe-commits mailing list