[clang][driver] adds `-print-diagnostics`

[cjdb]

Summary

Prints a list of all the warnings that Clang offers.

[cjdb]

I haven't added a test because I'm not sure how we can reasonably test this without keeping a file that holds all the warnings and using FileCheck. I'm okay with that, but want to make sure it's the best thing to do.

[MaskRay]

FYI diagtool tree dumps the diagnostics. You can even provide -W options and observe the differences.

[cjdb]

In D126796#3550848, @MaskRay wrote:

FYI diagtool tree dumps the diagnostics. You can even provide -W options and observe the differences.

Is diagtool tree something that users widely know about?

[MaskRay]

In D126796#3551261, @cjdb wrote:

In D126796#3550848, @MaskRay wrote:

FYI diagtool tree dumps the diagnostics. You can even provide -W options and observe the differences.

Is diagtool tree something that users widely know about?

I think no. But any random new option to clang will have the same discoverability problem.

[aaron.ballman]

In D126796#3551261, @cjdb wrote:

In D126796#3550848, @MaskRay wrote:

FYI diagtool tree dumps the diagnostics. You can even provide -W options and observe the differences.

Is diagtool tree something that users widely know about?

Doubtful?

I think I may have been vaguely aware of it, but I thought it was a testing utility. Is diagtool part of the default install of Clang?

[aaron.ballman]

In D126796#3551280, @MaskRay wrote:

In D126796#3551261, @cjdb wrote:

In D126796#3550848, @MaskRay wrote:

FYI diagtool tree dumps the diagnostics. You can even provide -W options and observe the differences.

Is diagtool tree something that users widely know about?

I think no. But any random new option to clang will have the same discoverability problem.

Yes, but I think there's a pretty wide discoverability gap between "documented command line option that shows up in Clang" and "random utility that doesn't have any obvious relationship to Clang specifically".

[tschuett]

Would a JSON dump help you for your tools?

[cjdb]

In D126796#3551312, @tschuett wrote:

Would a JSON dump help you for your tools?

Do you think a JSON dump would be more appropriate? I was just going for something not unlike --help here.

[tschuett]

In D126796#3551781, @cjdb wrote:

In D126796#3551312, @tschuett wrote:

Would a JSON dump help you for your tools?

Do you think a JSON dump would be more appropriate? I was just going for something not unlike --help here.

I thought that you need this for your diagnostics campaign.

[cjdb]

In D126796#3551834, @tschuett wrote:

In D126796#3551781, @cjdb wrote:

In D126796#3551312, @tschuett wrote:

Would a JSON dump help you for your tools?

Do you think a JSON dump would be more appropriate? I was just going for something not unlike --help here.

I thought that you need this for your diagnostics campaign.

Ah, gotcha. The idea for this feature came about while @aaron.ballman was reviewing a draft of that RFC, but it's orthogonal to that. I can't immediately see how users would benefit from this outputting JSON, but open to ideas.

[aaron.ballman]

In D126796#3551996, @cjdb wrote:

In D126796#3551834, @tschuett wrote:

In D126796#3551781, @cjdb wrote:

In D126796#3551312, @tschuett wrote:

Would a JSON dump help you for your tools?

Do you think a JSON dump would be more appropriate? I was just going for something not unlike --help here.

I thought that you need this for your diagnostics campaign.

Ah, gotcha. The idea for this feature came about while @aaron.ballman was reviewing a draft of that RFC, but it's orthogonal to that. I can't immediately see how users would benefit from this outputting JSON, but open to ideas.

+1 -- I think JSON output would be a better feature for diagtool. My goal with proposing the idea @cjdb has graciously worked on is to make warnings more discoverable for users in Clang itself, so I don't think JSON output helps there.

[cjdb]

In D126796#3551288, @aaron.ballman wrote:

In D126796#3551280, @MaskRay wrote:

In D126796#3551261, @cjdb wrote:

In D126796#3550848, @MaskRay wrote:

FYI diagtool tree dumps the diagnostics. You can even provide -W options and observe the differences.

Is diagtool tree something that users widely know about?

I think no. But any random new option to clang will have the same discoverability problem.

Yes, but I think there's a pretty wide discoverability gap between "documented command line option that shows up in Clang" and "random utility that doesn't have any obvious relationship to Clang specifically".

Agreed. I think a combination of Clang-next's release notes, plus this appearing in --help and web docs will be enough.

On discoverability, I'm thinking --print-all-warning-options might be a better name, since 'diagnostic' is a fairly technical term (and this is also not printing all diagnostics).

[MaskRay]

If you still decide to add an option to clang, I think "diagnostics" is better than "warnings". There are many diagnostics which are errors by default.

[aaron.ballman]

In D126796#3553708, @MaskRay wrote:

If you still decide to add an option to clang, I think "diagnostics" is better than "warnings". There are many diagnostics which are errors by default.

Good point about warnings which default to errors; those will also be part of the output. I think "diagnostic" is the best term I can think of that's still accurate.

[cjdb]

• renames flag to -print-diagnostic-options
• adds test
• adds logic to get as many options to be visible as possible

I'm very unhappy with the last point: getDiagnosticFlags doesn't seem to return *all* diagnostic flags (-W is missing with R0 of this patch), and even now, both -Wgnu-statement-expression-from-macro-expansion and -Wqualified-void-return-type are missing from the list despite being in the webpage.

Doing clang -print-diagnostic-options | wc reveals that there are also some options that I'm seeing as warnings, while the docs consider these remarks:

1. -Wmodule-build
2. -Wmodule-import
3. -Wmodule-lock
4. -Wpass
5. -Wpass-analysis
6. -Wpass-missed
7. -Wremark-backend-plugin
8. -Wround-trip-cc1-args
9. -Wsanitize-address
10. -Wsearch-path-usage

If Clang promoting these to warning flags is fine, that's okay. Regardless, it reveals a flaw in testing because these ten options aren't being flagged as missing when they probably should be. Is FileCheck capable of checking that all lines correspond to a file, or should I move to using diff?

[cjdb]

Okay, I understand why -W isn't being printed now, but the missing ones are still a bit baffling.

[cjdb]

• simplifies code
• minimises test to only a handful of cases

[aaron.ballman]

Thanks for this! Just some fairly minor nits from me. FWIW, the changes should also come with a release note so users know about the new command line option.

clang/include/clang/Driver/Options.td

829

Unintended whitespace change?

clang/lib/Basic/DiagnosticIDs.cpp

656

I presume this change doesn't have any negative effects in Driver::HandleAutocompletions()? (That's currently the only consumer of this API.)

clang/lib/Driver/Driver.cpp

2010

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.)

clang/test/Driver/print-diagnostic-options.c

1

"All" warning groups? ;-) Might want to update the comment somewhat.

[cjdb]

clang/include/clang/Driver/Options.td

829

My editor trims whitespace. I've added it back, but trimmed lines are nice!

clang/lib/Basic/DiagnosticIDs.cpp

656

All tests pass, so I hope not?

clang/lib/Driver/Driver.cpp

2010

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.

[aaron.ballman]

LGTM!

clang/lib/Basic/DiagnosticIDs.cpp

656

Sure, we can go with that. I also spot-checked and I don't think this should have a negative impact there.

clang/lib/Driver/Driver.cpp

2010

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.

I still reflexively assume that some older compilers aren't as good at move operations as they should be, and this would be inducing a copy of a pretty large vector. Using a const reference means the result of the call is lifetime extended and there's no chance of a big copy. However, I don't insist as I think we should be able to rely on move operations these days?

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.

Ah, no worries! I just mentioned it in case you weren't aware. :-)

clang/include/clang/Driver/Options.td

   HelpText<"Pass <arg> to the linker">, MetaVarName<"<arg>">,
   Group<Link_Group>;
 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>;
 def Xpreprocessor : Separate<["-"], "Xpreprocessor">, Group<Preprocessor_Group>,
   HelpText<"Pass <arg> to the preprocessor">, MetaVarName<"<arg>">;

   HelpText<"Print the paths used for finding ROCm installation">;
 def print_runtime_dir : Flag<["-", "--"], "print-runtime-dir">,
   HelpText<"Print the directory pathname containing clangs runtime libraries">;
+def print_diagnostic_options : Flag<["-", "--"], "print-diagnostic-options">,
+  HelpText<"Print all of Clang's warning options">;
 def private__bundle : Flag<["-"], "private_bundle">;
 def pthreads : Flag<["-"], "pthreads">;
 defm pthread : BoolOption<"", "pthread",

clang/lib/Basic/DiagnosticIDs.cpp

 }
 
 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';) {
     std::string Diag(DiagGroupNames + I + 1, DiagGroupNames[I]);
     I += DiagGroupNames[I] + 1;

clang/lib/Driver/Driver.cpp

     return false;
   }
 
+  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)
+      llvm::outs() << "  " << Flags[I] << "\n  " << Flags[I + 1] << "\n\n";
+    return false;
+  }
+
   // FIXME: The following handlers should use a callback mechanism, we don't
   // know what the client would like to do.
   if (Arg *A = C.getArgs().getLastArg(options::OPT_print_file_name_EQ)) {

clang/test/Driver/print-diagnostic-options.c

+// Test that -print-diagnostic-options prints all warning groups
+
+// RUN: %clang -print-diagnostic-options | FileCheck %s
+
+// CHECK:  -W
+// CHECK:  -Wno-
+// CHECK:  -W#pragma-messages
+// CHECK:  -Wno-#pragma-messages
+// CHECK:  -W#warnings
+// CHECK:  -Wabi
+// CHECK:  -Wno-abi
+// CHECK:  -Wall
+// CHECK:  -Wno-all