[Clang][Driver] Clean up Clang::ConstructJob a little bit, NFC

[etherzhhb]

Summary

Currently Clang::ConstructJob has more than 2000 lines of code and it is a little bit hard to follow ...

This change tries to cleanup Clang::ConstructJob by introducing 'AddAnalyzeArgs' and move the related code there

[ahatanak]

I think this patch is an improvement, but Clang::ConstructJob is still one giant function.

Do you have ideas to improve readability of this function or plans to further reduce its size?

[etherzhhb]

In D28050#660715, @ahatanak wrote:

I think this patch is an improvement, but Clang::ConstructJob is still one giant function.

Do you have ideas to improve readability of this function or plans to further reduce its size?

I am trying to identify the self-contained chunks and extra them

lib/Driver/Tools.h

private:
   mutable std::unique_ptr<visualstudio::Compiler> CLFallback;
 
   mutable std::unique_ptr<llvm::raw_fd_ostream> CompilationDatabase = nullptr;
   void DumpCompilationDatabase(Compilation &C, StringRef Filename,
                                StringRef Target,
                                const InputInfo &Output, const InputInfo &Input,
                                const llvm::opt::ArgList &Args) const;
 
+  void AddAnalyzeArgs(const llvm::opt::ArgList &Args, const InputInfo &Input,
+                      llvm::opt::ArgStringList &CmdArgs, bool IsWindowsMSVC,
+                      bool IsPS4CPU) const;
+
 public:
   // CAUTION! The first constructor argument ("clang") is not arbitrary,
   // as it is for other tools. Some operations on a Tool actually test
   // whether that tool is Clang based on the Tool's Name as a string.
   Clang(const ToolChain &TC) : Tool("clang", "clang frontend", TC, RF_Full) {}
 
   bool hasGoodDiagnostics() const override { return true; }
   bool hasIntegratedAssembler() const override { return true; }

lib/Driver/Tools.cpp

     for (auto &it: ASL)
       CDB << ", \"" << escape(it) << "\"";
   }
   Buf = "--target=";
   Buf += Target;
   CDB << ", \"" << escape(Buf) << "\"]},\n";
 }
 
+void Clang::AddAnalyzeArgs(const ArgList &Args, const InputInfo &Input,
+                           ArgStringList &CmdArgs, bool IsWindowsMSVC,
+                           bool IsPS4CPU) const {
+  // Enable region store model by default.
+  CmdArgs.push_back("-analyzer-store=region");
+
+  // Treat blocks as analysis entry points.
+  CmdArgs.push_back("-analyzer-opt-analyze-nested-blocks");
+
+  CmdArgs.push_back("-analyzer-eagerly-assume");
+
+  // Add default argument set.
+  if (!Args.hasArg(options::OPT__analyzer_no_default_checks)) {
+    CmdArgs.push_back("-analyzer-checker=core");
+    CmdArgs.push_back("-analyzer-checker=apiModeling");
+
+    if (!IsWindowsMSVC) {
+      CmdArgs.push_back("-analyzer-checker=unix");
+    } else {
+      // Enable "unix" checkers that also work on Windows.
+      CmdArgs.push_back("-analyzer-checker=unix.API");
+      CmdArgs.push_back("-analyzer-checker=unix.Malloc");
+      CmdArgs.push_back("-analyzer-checker=unix.MallocSizeof");
+      CmdArgs.push_back("-analyzer-checker=unix.MismatchedDeallocator");
+      CmdArgs.push_back("-analyzer-checker=unix.cstring.BadSizeArg");
+      CmdArgs.push_back("-analyzer-checker=unix.cstring.NullArg");
+    }
+
+    // Disable some unix checkers for PS4.
+    if (IsPS4CPU) {
+      CmdArgs.push_back("-analyzer-disable-checker=unix.API");
+      CmdArgs.push_back("-analyzer-disable-checker=unix.Vfork");
+    }
+
+    if (getToolChain().getTriple().getVendor() == llvm::Triple::Apple)
+      CmdArgs.push_back("-analyzer-checker=osx");
+
+    CmdArgs.push_back("-analyzer-checker=deadcode");
+
+    if (types::isCXX(Input.getType()))
+      CmdArgs.push_back("-analyzer-checker=cplusplus");
+
+    if (!IsPS4CPU) {
+      CmdArgs.push_back(
+          "-analyzer-checker=security.insecureAPI.UncheckedReturn");
+      CmdArgs.push_back("-analyzer-checker=security.insecureAPI.getpw");
+      CmdArgs.push_back("-analyzer-checker=security.insecureAPI.gets");
+      CmdArgs.push_back("-analyzer-checker=security.insecureAPI.mktemp");
+      CmdArgs.push_back("-analyzer-checker=security.insecureAPI.mkstemp");
+      CmdArgs.push_back("-analyzer-checker=security.insecureAPI.vfork");
+    }
+
+    // Default nullability checks.
+    CmdArgs.push_back("-analyzer-checker=nullability.NullPassedToNonnull");
+    CmdArgs.push_back("-analyzer-checker=nullability.NullReturnedFromNonnull");
+  }
+
+  // Set the output format. The default is plist, for (lame) historical
+  // reasons.
+  CmdArgs.push_back("-analyzer-output");
+  if (Arg *A = Args.getLastArg(options::OPT__analyzer_output))
+    CmdArgs.push_back(A->getValue());
+  else
+    CmdArgs.push_back("plist");
+
+  // Disable the presentation of standard compiler warnings when
+  // using --analyze.  We only want to show static analyzer diagnostics
+  // or frontend errors.
+  CmdArgs.push_back("-w");
+
+  // Add -Xanalyzer arguments when running as analyzer.
+  Args.AddAllArgValues(CmdArgs, options::OPT_Xanalyzer);
+}
+
 void Clang::ConstructJob(Compilation &C, const JobAction &JA,
                          const InputInfo &Output, const InputInfoList &Inputs,
                          const ArgList &Args, const char *LinkingOutput) const {
   const llvm::Triple &Triple = getToolChain().getEffectiveTriple();
   const std::string &TripleStr = Triple.getTriple();
 
   bool KernelOrKext =
       Args.hasArg(options::OPT_mkernel, options::OPT_fapple_kext);

   CmdArgs.push_back("-main-file-name");
   CmdArgs.push_back(getBaseInputName(Args, Input));
 
   // Some flags which affect the language (via preprocessor
   // defines).
   if (Args.hasArg(options::OPT_static))
     CmdArgs.push_back("-static-define");
 
-  if (isa<AnalyzeJobAction>(JA)) {
-    // Enable region store model by default.
-    CmdArgs.push_back("-analyzer-store=region");
-
-    // Treat blocks as analysis entry points.
-    CmdArgs.push_back("-analyzer-opt-analyze-nested-blocks");
-
-    CmdArgs.push_back("-analyzer-eagerly-assume");
-
-    // Add default argument set.
-    if (!Args.hasArg(options::OPT__analyzer_no_default_checks)) {
-      CmdArgs.push_back("-analyzer-checker=core");
-      CmdArgs.push_back("-analyzer-checker=apiModeling");
-
-    if (!IsWindowsMSVC) {
-      CmdArgs.push_back("-analyzer-checker=unix");
-    } else {
-      // Enable "unix" checkers that also work on Windows.
-      CmdArgs.push_back("-analyzer-checker=unix.API");
-      CmdArgs.push_back("-analyzer-checker=unix.Malloc");
-      CmdArgs.push_back("-analyzer-checker=unix.MallocSizeof");
-      CmdArgs.push_back("-analyzer-checker=unix.MismatchedDeallocator");
-      CmdArgs.push_back("-analyzer-checker=unix.cstring.BadSizeArg");
-      CmdArgs.push_back("-analyzer-checker=unix.cstring.NullArg");
-    }
-
-      // Disable some unix checkers for PS4.
-      if (IsPS4CPU) {
-        CmdArgs.push_back("-analyzer-disable-checker=unix.API");
-        CmdArgs.push_back("-analyzer-disable-checker=unix.Vfork");
-      }
-
-      if (getToolChain().getTriple().getVendor() == llvm::Triple::Apple)
-        CmdArgs.push_back("-analyzer-checker=osx");
-
-      CmdArgs.push_back("-analyzer-checker=deadcode");
-
-      if (types::isCXX(Input.getType()))
-        CmdArgs.push_back("-analyzer-checker=cplusplus");
-
-      if (!IsPS4CPU) {
-        CmdArgs.push_back(
-            "-analyzer-checker=security.insecureAPI.UncheckedReturn");
-        CmdArgs.push_back("-analyzer-checker=security.insecureAPI.getpw");
-        CmdArgs.push_back("-analyzer-checker=security.insecureAPI.gets");
-        CmdArgs.push_back("-analyzer-checker=security.insecureAPI.mktemp");
-        CmdArgs.push_back("-analyzer-checker=security.insecureAPI.mkstemp");
-        CmdArgs.push_back("-analyzer-checker=security.insecureAPI.vfork");
-      }
-
-      // Default nullability checks.
-      CmdArgs.push_back("-analyzer-checker=nullability.NullPassedToNonnull");
-      CmdArgs.push_back(
-          "-analyzer-checker=nullability.NullReturnedFromNonnull");
-    }
-
-    // Set the output format. The default is plist, for (lame) historical
-    // reasons.
-    CmdArgs.push_back("-analyzer-output");
-    if (Arg *A = Args.getLastArg(options::OPT__analyzer_output))
-      CmdArgs.push_back(A->getValue());
-    else
-      CmdArgs.push_back("plist");
-
-    // Disable the presentation of standard compiler warnings when
-    // using --analyze.  We only want to show static analyzer diagnostics
-    // or frontend errors.
-    CmdArgs.push_back("-w");
-
-    // Add -Xanalyzer arguments when running as analyzer.
-    Args.AddAllArgValues(CmdArgs, options::OPT_Xanalyzer);
-  }
+  if (isa<AnalyzeJobAction>(JA))
+    AddAnalyzeArgs(Args, Input, CmdArgs, IsWindowsMSVC, IsPS4CPU);
 
   CheckCodeGenerationOptions(D, Args);
 
   llvm::Reloc::Model RelocationModel;
   unsigned PICLevel;
   bool IsPIE;
   std::tie(RelocationModel, PICLevel, IsPIE) =
       ParsePICArgs(getToolChain(), Triple, Args);