[Frontend] give createInvocationFromCommandLine an options struct

It's accumulating way too many optional params (see D124970)

While here, improve the name and the documentation.

Differential Revision: https://reviews.llvm.org/D124971

Author: sam-mccall

clang-tools-extra/clangd/Compiler.cpp

@@ -91,12 +91,13 @@ buildCompilerInvocation(const ParseInputs &Inputs, clang::DiagnosticConsumer &D,
   for (const auto &S : Inputs.CompileCommand.CommandLine)
     ArgStrs.push_back(S.c_str());
 
-  auto VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory);
-  llvm::IntrusiveRefCntPtr<DiagnosticsEngine> CommandLineDiagsEngine =
+  CreateInvocationOptions CIOpts;
+  CIOpts.VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory);
+  CIOpts.CC1Args = CC1Args;
+  CIOpts.RecoverOnError = true;
+  CIOpts.Diags =
       CompilerInstance::createDiagnostics(new DiagnosticOptions, &D, false);
-  std::unique_ptr<CompilerInvocation> CI = createInvocationFromCommandLine(
-      ArgStrs, CommandLineDiagsEngine, std::move(VFS),
-      /*ShouldRecoverOnErrors=*/true, CC1Args);
+  std::unique_ptr<CompilerInvocation> CI = createInvocation(ArgStrs, CIOpts);
   if (!CI)
     return nullptr;
   // createInvocationFromCommandLine sets DisableFree.

clang/include/clang/Frontend/Utils.h

@@ -22,7 +22,6 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
-#include "llvm/Option/OptSpecifier.h"
 #include "llvm/Support/FileCollector.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include <cstdint>
@@ -190,18 +189,47 @@ IntrusiveRefCntPtr<ExternalSemaSource>
 createChainedIncludesSource(CompilerInstance &CI,
                             IntrusiveRefCntPtr<ExternalSemaSource> &Reader);
 
-/// createInvocationFromCommandLine - Construct a compiler invocation object for
-/// a command line argument vector.
+/// Optional inputs to createInvocation.
+struct CreateInvocationOptions {
+  /// Receives diagnostics encountered while parsing command-line flags.
+  /// If not provided, these are printed to stderr.
+  IntrusiveRefCntPtr<DiagnosticsEngine> Diags = nullptr;
+  /// Used e.g. to probe for system headers locations.
+  /// If not provided, the real filesystem is used.
+  /// FIXME: the driver does perform some non-virtualized IO.
+  IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr;
+  /// Whether to attempt to produce a non-null (possibly incorrect) invocation
+  /// if any errors were encountered.
+  /// By default, always return null on errors.
+  bool RecoverOnError = false;
+  /// If set, the target is populated with the cc1 args produced by the driver.
+  /// This may be populated even if createInvocation returns nullptr.
+  std::vector<std::string> *CC1Args = nullptr;
+};
+
+/// Interpret clang arguments in preparation to parse a file.
+///
+/// This simulates a number of steps Clang takes when its driver is invoked:
+/// - choosing actions (e.g compile + link) to run
+/// - probing the system for settings like standard library locations
+/// - spawning a cc1 subprocess to compile code, with more explicit arguments
+/// - in the cc1 process, assembling those arguments into a CompilerInvocation
+///   which is used to configure the parser
 ///
-/// \param ShouldRecoverOnErrors - whether we should attempt to return a
-/// non-null (and possibly incorrect) CompilerInvocation if any errors were
-/// encountered. When this flag is false, always return null on errors.
+/// This simulation is lossy, e.g. in some situations one driver run would
+/// result in multiple parses. (Multi-arch, CUDA, ...).
+/// This function tries to select a reasonable invocation that tools should use.
 ///
-/// \param CC1Args - if non-null, will be populated with the args to cc1
-/// expanded from \p Args. May be set even if nullptr is returned.
+/// Args[0] should be the driver name, such as "clang" or "/usr/bin/g++".
+/// Absolute path is preferred - this affects searching for system headers.
 ///
-/// \return A CompilerInvocation, or nullptr if none was built for the given
-/// argument vector.
+/// May return nullptr if an invocation could not be determined.
+/// See CreateInvocationOptions::ShouldRecoverOnErrors to try harder!
+std::unique_ptr<CompilerInvocation>
+createInvocation(ArrayRef<const char *> Args,
+                 CreateInvocationOptions Opts = {});
+
+/// Deprecated version of createInvocation with individual optional args.
 std::unique_ptr<CompilerInvocation> createInvocationFromCommandLine(
     ArrayRef<const char *> Args,
     IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
@@ -210,8 +238,6 @@ std::unique_ptr<CompilerInvocation> createInvocationFromCommandLine(
     bool ShouldRecoverOnErrors = false,
     std::vector<std::string> *CC1Args = nullptr);
 
-// Frontend timing utils
-
 } // namespace clang
 
 #endif // LLVM_CLANG_FRONTEND_UTILS_H

clang/lib/Frontend/CreateInvocationFromCommandLine.cpp

@@ -26,16 +26,13 @@
 using namespace clang;
 using namespace llvm::opt;
 
-std::unique_ptr<CompilerInvocation> clang::createInvocationFromCommandLine(
-    ArrayRef<const char *> ArgList, IntrusiveRefCntPtr<DiagnosticsEngine> Diags,
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS, bool ShouldRecoverOnErorrs,
-    std::vector<std::string> *CC1Args) {
+std::unique_ptr<CompilerInvocation>
+clang::createInvocation(ArrayRef<const char *> ArgList,
+                        CreateInvocationOptions Opts) {
   assert(!ArgList.empty());
-  if (!Diags.get()) {
-    // No diagnostics engine was provided, so create our own diagnostics object
-    // with the default options.
-    Diags = CompilerInstance::createDiagnostics(new DiagnosticOptions);
-  }
+  auto Diags = Opts.Diags
+                   ? std::move(Opts.Diags)
+                   : CompilerInstance::createDiagnostics(new DiagnosticOptions);
 
   SmallVector<const char *, 16> Args(ArgList.begin(), ArgList.end());
 
@@ -47,7 +44,7 @@ std::unique_ptr<CompilerInvocation> clang::createInvocationFromCommandLine(
 
   // FIXME: We shouldn't have to pass in the path info.
   driver::Driver TheDriver(Args[0], llvm::sys::getDefaultTargetTriple(), *Diags,
-                           "clang LLVM compiler", VFS);
+                           "clang LLVM compiler", Opts.VFS);
 
   // Don't check that inputs exist, they may have been remapped.
   TheDriver.setCheckInputsExist(false);
@@ -81,7 +78,7 @@ std::unique_ptr<CompilerInvocation> clang::createInvocationFromCommandLine(
     }
   }
 
-  bool PickFirstOfMany = OffloadCompilation || ShouldRecoverOnErorrs;
+  bool PickFirstOfMany = OffloadCompilation || Opts.RecoverOnError;
   if (Jobs.size() == 0 || (Jobs.size() > 1 && !PickFirstOfMany)) {
     SmallString<256> Msg;
     llvm::raw_svector_ostream OS(Msg);
@@ -98,11 +95,20 @@ std::unique_ptr<CompilerInvocation> clang::createInvocationFromCommandLine(
   }
 
   const ArgStringList &CCArgs = Cmd->getArguments();
-  if (CC1Args)
-    *CC1Args = {CCArgs.begin(), CCArgs.end()};
+  if (Opts.CC1Args)
+    *Opts.CC1Args = {CCArgs.begin(), CCArgs.end()};
   auto CI = std::make_unique<CompilerInvocation>();
   if (!CompilerInvocation::CreateFromArgs(*CI, CCArgs, *Diags, Args[0]) &&
-      !ShouldRecoverOnErorrs)
+      !Opts.RecoverOnError)
     return nullptr;
   return CI;
 }
+
+std::unique_ptr<CompilerInvocation> clang::createInvocationFromCommandLine(
+    ArrayRef<const char *> Args, IntrusiveRefCntPtr<DiagnosticsEngine> Diags,
+    IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS, bool ShouldRecoverOnErrors,
+    std::vector<std::string> *CC1Args) {
+  return createInvocation(
+      Args,
+      CreateInvocationOptions{Diags, VFS, ShouldRecoverOnErrors, CC1Args});
+}

clang/unittests/Frontend/UtilsTest.cpp

@@ -23,12 +23,12 @@ TEST(BuildCompilerInvocationTest, RecoverMultipleJobs) {
   std::vector<const char *> Args = {"clang", "--target=macho", "-arch",  "i386",
                                     "-arch", "x86_64",         "foo.cpp"};
   clang::IgnoringDiagConsumer D;
-  llvm::IntrusiveRefCntPtr<DiagnosticsEngine> CommandLineDiagsEngine =
-      clang::CompilerInstance::createDiagnostics(new DiagnosticOptions, &D,
-                                                 false);
-  std::unique_ptr<CompilerInvocation> CI = createInvocationFromCommandLine(
-      Args, CommandLineDiagsEngine, new llvm::vfs::InMemoryFileSystem(),
-      /*ShouldRecoverOnErrors=*/true);
+  CreateInvocationOptions Opts;
+  Opts.RecoverOnError = true;
+  Opts.Diags = clang::CompilerInstance::createDiagnostics(new DiagnosticOptions,
+                                                          &D, false);
+  Opts.VFS = new llvm::vfs::InMemoryFileSystem();
+  std::unique_ptr<CompilerInvocation> CI = createInvocation(Args, Opts);
   ASSERT_TRUE(CI);
   EXPECT_THAT(CI->TargetOpts->Triple, testing::StartsWith("i386-"));
 }