[Driver][Diagnostics] Make 'show option names' default for driver warnings

[bruno]

Summary

Currently, driver level warnings do not show option names (e.g. warning: complain about foo [-Woption-name]) in a diagnostic unless -fdiagnostics-show-option is explictly specified. OTOH, the driver by default turn this option on for CC1. Change the logic to show option names by default in the driver as well.

[bruno]

Ping!

[bruno]

Ping

[bruno]

Ping!

[bruno]

Ping!!

[vsk]

lib/Frontend/CompilerInvocation.cpp

965

Does it make sense to use: hasFlag (OptSpecifier Pos, OptSpecifier Neg, bool Default=DefaultShowOpt) ? This might take care of an edge case where both show_option and no_show_option are specified -- the last flag passed should win out.

test/Driver/show-option-names.c

4

Can you split this into two checks? One to check that the warning appear, and another to check that the option isn't printed.

[bruno]

Update after Vedant's review!

[vsk]

Lgtm, with one potential nit.

test/Driver/show-option-names.c

4

Argh, I suppose there's no "CHECK-SAME-NOT" functionality in FileCheck. If you want to shave off a compiler invocation, maybe you could write:

CHECK: ... {{$}}

[bruno]

Committed r283827

test/Driver/show-option-names.c

4

The "-NOT" functionality is there, but doesn't help in this case. The {{$}} worked well enough though! Thanks

include/clang/Frontend/CompilerInvocation.h

 ///
 /// Args must have been created from the OptTable returned by
 /// createCC1OptTable().
 ///
 /// When errors are encountered, return false and, if Diags is non-null,
 /// report the error(s).
 bool ParseDiagnosticArgs(DiagnosticOptions &Opts, llvm::opt::ArgList &Args,
                          DiagnosticsEngine *Diags = nullptr,
-                         bool DefaultDiagColor = true);
+                         bool DefaultDiagColor = true,
+                         bool DefaultShowOpt = true);
 
 class CompilerInvocationBase : public RefCountedBase<CompilerInvocation> {
   void operator=(const CompilerInvocationBase &) = delete;
 
 public:
   /// Options controlling the language variant.
   std::shared_ptr<LangOptions> LangOpts;
 

lib/Frontend/CompilerInvocation.cpp

static bool parseShowColorsArgs(const ArgList &Args, bool DefaultColor) {
   if (ShowColors == Colors_On ||
       (ShowColors == Colors_Auto && llvm::sys::Process::StandardErrHasColors()))
     return true;
   return false;
 }
 
 bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
                                 DiagnosticsEngine *Diags,
-                                bool DefaultDiagColor) {
+                                bool DefaultDiagColor, bool DefaultShowOpt) {
   using namespace options;
   bool Success = true;
 
   Opts.DiagnosticLogFile = Args.getLastArgValue(OPT_diagnostic_log_file);
   if (Arg *A =
           Args.getLastArg(OPT_diagnostic_serialized_file, OPT__serialize_diags))
     Opts.DiagnosticSerializationFile = A->getValue();
   Opts.IgnoreWarnings = Args.hasArg(OPT_w);
   Opts.NoRewriteMacros = Args.hasArg(OPT_Wno_rewrite_macros);
   Opts.Pedantic = Args.hasArg(OPT_pedantic);
   Opts.PedanticErrors = Args.hasArg(OPT_pedantic_errors);
   Opts.ShowCarets = !Args.hasArg(OPT_fno_caret_diagnostics);
   Opts.ShowColors = parseShowColorsArgs(Args, DefaultDiagColor);
   Opts.ShowColumn = Args.hasFlag(OPT_fshow_column,
                                  OPT_fno_show_column,
                                  /*Default=*/true);
   Opts.ShowFixits = !Args.hasArg(OPT_fno_diagnostics_fixit_info);
   Opts.ShowLocation = !Args.hasArg(OPT_fno_show_source_location);
   Opts.AbsolutePath = Args.hasArg(OPT_fdiagnostics_absolute_paths);
-  Opts.ShowOptionNames = Args.hasArg(OPT_fdiagnostics_show_option);
+  Opts.ShowOptionNames = Args.hasArg(OPT_fdiagnostics_show_option) ||
+      (DefaultShowOpt && !Args.hasArg(OPT_fno_diagnostics_show_option));
 
   llvm::sys::Process::UseANSIEscapeCodes(Args.hasArg(OPT_fansi_escape_codes));
 
   // Default behavior is to not to show note include stacks.
   Opts.ShowNoteIncludeStack = false;
   if (Arg *A = Args.getLastArg(OPT_fdiagnostics_show_note_include_stack,
                                OPT_fno_diagnostics_show_note_include_stack))
     if (A->getOption().matches(OPT_fdiagnostics_show_note_include_stack))
bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res,
   for (const Arg *A : Args.filtered(OPT_UNKNOWN)) {
     Diags.Report(diag::err_drv_unknown_argument) << A->getAsString(Args);
     Success = false;
   }
 
   Success &= ParseAnalyzerArgs(*Res.getAnalyzerOpts(), Args, Diags);
   Success &= ParseMigratorArgs(Res.getMigratorOpts(), Args);
   ParseDependencyOutputArgs(Res.getDependencyOutputOpts(), Args);
-  Success &= ParseDiagnosticArgs(Res.getDiagnosticOpts(), Args, &Diags,
-                                 false /*DefaultDiagColor*/);
+  Success &=
+      ParseDiagnosticArgs(Res.getDiagnosticOpts(), Args, &Diags,
+                          false /*DefaultDiagColor*/, false /*DefaultShowOpt*/);
   ParseCommentArgs(LangOpts.CommentOpts, Args);
   ParseFileSystemArgs(Res.getFileSystemOpts(), Args);
   // FIXME: We shouldn't have to pass the DashX option around here
   InputKind DashX = ParseFrontendArgs(Res.getFrontendOpts(), Args, Diags);
   ParseTargetArgs(Res.getTargetOpts(), Args, Diags);
   Success &= ParseCodeGenArgs(Res.getCodeGenOpts(), Args, DashX, Diags,
                               Res.getTargetOpts());
   ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args);

test/Driver/show-option-names.c

+// RUN: %clang -c -target i386-apple-darwin10 -isysroot /FOO %s 2>&1 | FileCheck --check-prefix=CHECK-SHOW-OPTION-NAMES %s
+// CHECK-SHOW-OPTION-NAMES: warning: no such sysroot directory: '{{([A-Za-z]:.*)?}}/FOO' [-Wmissing-sysroot]
+// RUN: %clang -c -target i386-apple-darwin10 -fno-diagnostics-show-option -isysroot /FOO %s 2>&1 | FileCheck --check-prefix=CHECK-NO-SHOW-OPTION-NAMES %s
+// CHECK-NO-SHOW-OPTION-NAMES-NOT: warning: no such sysroot directory: '{{([A-Za-z]:.*)?}}/FOO' [-Wmissing-sysroot]