[PATCH] D126796: [clang][driver] adds `-print-diagnostics`

Christopher Di Bella via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 8 09:50:06 PDT 2022


cjdb added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:829
 def Xoffload_linker : JoinedAndSeparate<["-"], "Xoffload-linker">,
-  HelpText<"Pass <arg> to the offload linkers or the ones idenfied by -<triple>">, 
+  HelpText<"Pass <arg> to the offload linkers or the ones idenfied by -<triple>">,
   MetaVarName<"<triple> <arg>">, Group<Link_Group>;
----------------
aaron.ballman wrote:
> Unintended whitespace change?
My editor trims whitespace. I've added it back, but trimmed lines are nice!


================
Comment at: clang/lib/Basic/DiagnosticIDs.cpp:656
 std::vector<std::string> DiagnosticIDs::getDiagnosticFlags() {
-  std::vector<std::string> Res;
+  std::vector<std::string> Res{"-W", "-Wno-"};
   for (size_t I = 1; DiagGroupNames[I] != '\0';) {
----------------
aaron.ballman wrote:
> I presume this change doesn't have any negative effects in `Driver::HandleAutocompletions()`? (That's currently the only consumer of this API.)
All tests pass, so I hope not?


================
Comment at: clang/lib/Driver/Driver.cpp:2010
+  if (C.getArgs().hasArg(options::OPT_print_diagnostic_options)) {
+    std::vector<std::string> Flags = DiagnosticIDs::getDiagnosticFlags();
+    for (std::size_t I = 0; I != Flags.size(); I += 2)
----------------
aaron.ballman wrote:
> Declare the type as a const reference?
> 
> (Btw, when uploading patches, you should give them plenty of patch context -- that's what allows reviewers to suggest changes directly in Phab, but it also gives more information to reviewers so they don't have to leave the review to go to a code editor. FWIW, I usually use `-U999` to add context.)
> Declare the type as a const reference?

I'm not seeing the advantage of doing this here. Happy to make it const, but the reference doesn't add a benefit and potentially introduces confusion.

> (Btw, when uploading patches, you should give them plenty of patch context -- that's what allows reviewers to suggest changes directly in Phab, but it also gives more information to reviewers so they don't have to leave the review to go to a code editor. FWIW, I usually use -U999 to add context.)

Sorry about that. I usually use `arc diff` to upload patches, but have been having problems with it recently and did a rushed manual upload instead.


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

https://reviews.llvm.org/D126796



More information about the cfe-commits mailing list