logan-5 added inline comments.

================
Comment at: 
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:27-34
+static const char NonReservedMessage[] =
+    "declaration uses identifier '%0', which is not a reserved "
+    "identifier";
+static const char GlobalUnderscoreMessage[] =
+    "declaration uses identifier '%0', which is reserved in the global "
+    "namespace; this causes undefined behavior";
+static const char DefaultMessage[] = "declaration uses reserved identifier "
----------------
aaron.ballman wrote:
> I think you can reasonably combine these into a single diagnostic using 
> `%select`. e.g.,
> `"declaration uses identifier %0, which is %select{a reserved identifier|not 
> a reserved identifier|reserved in the global namespace}1"`
> 
> I took out the bits about causing undefined behavior because that doesn't 
> really add much to the diagnostic (the "is a reserved identifier" bit should 
> be sufficient). Also, I removed the manual quoting around the `%0` because it 
> shouldn't be needed if you pass in a `NamedDecl*` as opposed to a string 
> (which you should prefer doing because it automatically formats the 
> identifier properly).
I don't have access to a `NamedDecl*` for a couple reasons, one being that the 
same diagnostic is used for both declarations and macro definitions. The 
%select change sounds like a good one--I'll do that (or I already did it, 
depending when this comment shows up).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72378



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to