[clang][cli] Don't emit manufactured warnings

[jansvoboda11]

Summary

The switch from per-member round-trip to whole-CompilerInvocation round-trip in D96280 means we'll be running the extra code present in CompilerInvocation::CreateFromArgs. This code conditionally manufactures extra warnings based on the target and input. This patch ensures we don't generate arguments for such manufactured warnings. Without this patch, each call to CompilerInvocation::generateCC1CommandLine during whole-CompilerInvocation round-trip could generate more arguments than the previous one.

[dexonsmith]

clang/lib/Frontend/CompilerInvocation.cpp

2347–2353

It seems reasonable to skip generating them if they're implied by other command-line options, but I'm not sure "manufactured" is the right word to use as a distinguishing characteristic. The entire CompilerInvocation could have been created programmatically. I suggest instead saying the warning flag is implied by the other command-line options.

Also, note that when created programmatically, one could have pushed stdlibcxx-not-found *after* pushing no-stdlibcxx-not-found. Since that's impossible to recreate, maybe there should be an assertion to catch this? Alternatively, should this kind of imply-diagnostic-options logic be moved to the driver?

Relatedly, unlike most command-line options, GenerateDiagnosticArgs is not canonicalizing the options. For example, if -Wabc implies -Wdef, it'd be nice to generate just -Wabc from initial command-lines of either -Wabc -Wdef or -Wdef -Wabc / to drop the first of -Wno-abc -Wabc / etc.

IMO, something akin to an initial DiagnosticsEngine::DiagState (likely renamed) could be stored in DiagnosticOptions (effectively, the resulting state from calling ProcessWarningOptions). Parsing could translate command-line options to this initial state. The state could be modified programmatically; it'd also be used to initialize DiagnosticsEngine. Generating command-line options would emit a canonical set of options that would recreate the state. But that's a pretty big refactoring, and I think it's okay to make progress without that.

As an initial fix, this is probably fine, but I think the comments and/or FIXMEs should acknowledge that it's a bit fragile and point in a more sound / less fragile direction (doesn't have to be my suggestion).

[jansvoboda11]

Abandoning this in favor of D97041 & D97042.

clang/lib/Frontend/CompilerInvocation.cpp

2347–2353

Using "implied" would be fitting as well. I guess I wanted to distinguish this from other implied options that don't need any explicit special-casing.

You're right the way of handling warnings is fragile. Normalizing the arguments into DiagState would be more robust, but as you say, it's a bit more involved. I'm putting that on my back-burner.

In the end, I looked into the history of -Wno-stdlibcxx-not-found and -Wspir-compat more closely and found ways to remove them from CompilerInvocation entirely: D97041 & D97042.

clang/include/clang/Frontend/CompilerInvocation.h

static bool ParseDiagnosticArgsRoundTrip(CompilerInvocation &Res,
                                            DiagnosticOptions &Opts,
                                            llvm::opt::ArgList &Args,
                                            DiagnosticsEngine *Diags = nullptr,
                                            bool DefaultDiagColor = true);
 
   /// Generate command line options from DiagnosticOptions.
   static void GenerateDiagnosticArgs(const DiagnosticOptions &Opts,
                                      SmallVectorImpl<const char *> &Args,
-                                     StringAllocator SA, bool DefaultDiagColor);
+                                     StringAllocator SA, bool DefaultDiagColor,
+                                     const llvm::Triple &T, InputKind DashX);
 
   /// Parse command line options that map to LangOptions.
   static bool ParseLangArgsImpl(LangOptions &Opts, llvm::opt::ArgList &Args,
                                 InputKind IK, const llvm::Triple &T,
                                 std::vector<std::string> &Includes,
                                 DiagnosticsEngine &Diags);
 
   static bool ParseLangArgs(CompilerInvocation &Res, LangOptions &Opts,

clang/lib/Frontend/CompilerInvocation.cpp

 #include "clang/Driver/Options.inc"
 #undef MIGRATOR_OPTION_WITH_MARSHALLING
 
   return Success;
 }
 
 void CompilerInvocation::GenerateDiagnosticArgs(
     const DiagnosticOptions &Opts, SmallVectorImpl<const char *> &Args,
-    StringAllocator SA, bool DefaultDiagColor) {
+    StringAllocator SA, bool DefaultDiagColor, const llvm::Triple &T,
+    InputKind DashX) {
   const DiagnosticOptions *DiagnosticOpts = &Opts;
 #define DIAG_OPTION_WITH_MARSHALLING(                                          \
     PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,        \
     HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,   \
     DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER,     \
     MERGER, EXTRACTOR, TABLE_INDEX)                                            \
   GENERATE_OPTION_WITH_MARSHALLING(                                            \
       Args, SA, KIND, FLAGS, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,    \
if (VIU == DiagnosticLevelMask::None) {
     if (static_cast<unsigned>(VIU & DiagnosticLevelMask::Error) != 0)
       GenerateArg(Args, OPT_verify_ignore_unexpected_EQ, "error", SA);
   }
 
   for (const auto &Warning : Opts.Warnings) {
     // This option is automatically generated from UndefPrefixes.
     if (Warning == "undef-prefix")
       continue;
+    // This warning was manufactured, don't put it on the command line.
+    if (Warning == "no-stdlibcxx-not-found" && T.isOSDarwin() &&
+        DashX.isPreprocessed())
+      continue;
+    // This warning was manufactured, don't put it on the command line.
+    if (Warning == "spir-compat" && T.isSPIR())
+      continue;
     Args.push_back(SA(StringRef("-W") + Warning));
   }
 
   for (const auto &Remark : Opts.Remarks) {
     // These arguments are generated from OptimizationRemark fields of
     // CodeGenOptions.
     StringRef IgnoredRemarks[] = {"pass",          "no-pass",
                                   "pass-analysis", "no-pass-analysis",
if (Arg *A =
           Args.getLastArg(OPT_diagnostic_serialized_file, OPT__serialize_diags))
     Opts.DiagnosticSerializationFile = A->getValue();
   Opts.ShowColors = parseShowColorsArgs(Args, DefaultDiagColor);
 
   Opts.VerifyDiagnostics = Args.hasArg(OPT_verify) || Args.hasArg(OPT_verify_EQ);
   Opts.VerifyPrefixes = Args.getAllArgValues(OPT_verify_EQ);
   if (Args.hasArg(OPT_verify))
     Opts.VerifyPrefixes.push_back("expected");
   // Keep VerifyPrefixes in its original order for the sake of diagnostics, and
   // then sort it to prepare for fast lookup using std::binary_search.
   if (!checkVerifyPrefixes(Opts.VerifyPrefixes, *Diags)) {
     Opts.VerifyDiagnostics = false;
     Success = false;
   }
   else
     llvm::sort(Opts.VerifyPrefixes);
   DiagnosticLevelMask DiagMask = DiagnosticLevelMask::None;
if (!SanHash.empty())
     code = hash_combine(code, SanHash.Mask);
 
   return llvm::APInt(64, code).toString(36, /*Signed=*/false);
 }
 
 void CompilerInvocation::generateCC1CommandLine(
     SmallVectorImpl<const char *> &Args, StringAllocator SA) const {
   llvm::Triple T(TargetOpts->Triple);
+  InputKind DashX = FrontendOpts.DashX;
 
   GenerateFileSystemArgs(FileSystemOpts, Args, SA);
   GenerateMigratorArgs(MigratorOpts, Args, SA);
   GenerateAnalyzerArgs(*AnalyzerOpts, Args, SA);
-  GenerateDiagnosticArgs(*DiagnosticOpts, Args, SA, false);
+  GenerateDiagnosticArgs(*DiagnosticOpts, Args, SA, false, T, DashX);
   GenerateFrontendArgs(FrontendOpts, Args, SA, LangOpts->IsHeaderFile);
   GenerateTargetArgs(*TargetOpts, Args, SA);
   GenerateHeaderSearchArgs(*HeaderSearchOpts, Args, SA);
   GenerateLangArgs(*LangOpts, Args, SA, T);
   GenerateCodeGenArgs(CodeGenOpts, Args, SA, T, FrontendOpts.OutputFile,
                       &*LangOpts);
   GeneratePreprocessorArgs(*PreprocessorOpts, Args, SA, *LangOpts, FrontendOpts,
                            CodeGenOpts);