[clang] NFC: Extract DiagnosticOptions parsing
authorJan Svoboda <jan_svoboda@apple.com>
Mon, 30 Aug 2021 13:41:53 +0000 (15:41 +0200)
committerJan Svoboda <jan_svoboda@apple.com>
Thu, 2 Sep 2021 12:37:14 +0000 (14:37 +0200)
The way we parse `DiagnosticOptions` is a bit involved.

`DiagnosticOptions` are parsed as part of the cc1-parsing function `CompilerInvocation::CreateFromArgs` which takes `DiagnosticsEngine` as an argument to be able to report errors in command-line arguments. But to create `DiagnosticsEngine`, `DiagnosticOptions` are needed. This is solved by exposing the `ParseDiagnosticArgs` to clients and making its `DiagnosticsEngine` argument optional, essentially breaking the dependency cycle.

The `ParseDiagnosticArgs` function takes `llvm::opt::ArgList &`, which each client needs to create from the command-line (typically represented as `std::vector<const char *>`). Creating this data structure in this context is somewhat particular. This code pattern is copy-pasted in some places across the upstream code base and also in downstream repos. To make things a bit more uniform, this patch extracts the code into a new reusable function: `CreateAndPopulateDiagOpts`.

Reviewed By: dexonsmith

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

clang/include/clang/Frontend/CompilerInvocation.h
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Interpreter/Interpreter.cpp
clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
clang/lib/Tooling/Tooling.cpp
clang/tools/driver/driver.cpp

index 2245439..922c84a 100644 (file)
@@ -50,6 +50,11 @@ class HeaderSearchOptions;
 class PreprocessorOptions;
 class TargetOptions;
 
+// This lets us create the DiagnosticsEngine with a properly-filled-out
+// DiagnosticOptions instance.
+std::unique_ptr<DiagnosticOptions>
+CreateAndPopulateDiagOpts(ArrayRef<const char *> Argv);
+
 /// Fill out Opts based on the options given in Args.
 ///
 /// Args must have been created from the OptTable returned by
index 64800b1..0da2cb3 100644 (file)
@@ -2256,6 +2256,19 @@ void CompilerInvocation::GenerateDiagnosticArgs(
   }
 }
 
+std::unique_ptr<DiagnosticOptions>
+clang::CreateAndPopulateDiagOpts(ArrayRef<const char *> Argv) {
+  auto DiagOpts = std::make_unique<DiagnosticOptions>();
+  unsigned MissingArgIndex, MissingArgCount;
+  InputArgList Args = getDriverOptTable().ParseArgs(
+      Argv.slice(1), MissingArgIndex, MissingArgCount);
+  // We ignore MissingArgCount and the return value of ParseDiagnosticArgs.
+  // Any errors that would be diagnosed here will also be diagnosed later,
+  // when the DiagnosticsEngine actually exists.
+  (void)ParseDiagnosticArgs(*DiagOpts, Args);
+  return DiagOpts;
+}
+
 bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
                                 DiagnosticsEngine *Diags,
                                 bool DefaultDiagColor) {
index 937504f..3e8d388 100644 (file)
@@ -147,15 +147,10 @@ IncrementalCompilerBuilder::create(std::vector<const char *> &ClangArgv) {
   // Buffer diagnostics from argument parsing so that we can output them using a
   // well formed diagnostic object.
   IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
-  IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
+  IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts =
+      CreateAndPopulateDiagOpts(ClangArgv);
   TextDiagnosticBuffer *DiagsBuffer = new TextDiagnosticBuffer;
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagsBuffer);
-  unsigned MissingArgIndex, MissingArgCount;
-  const llvm::opt::OptTable &Opts = driver::getDriverOptTable();
-  llvm::opt::InputArgList ParsedArgs =
-      Opts.ParseArgs(ArrayRef<const char *>(ClangArgv).slice(1),
-                     MissingArgIndex, MissingArgCount);
-  ParseDiagnosticArgs(*DiagOpts, ParsedArgs, &Diags);
 
   driver::Driver Driver(/*MainBinaryName=*/ClangArgv[0],
                         llvm::sys::getProcessTriple(), Diags);
index 8091a46..9c82542 100644 (file)
@@ -91,12 +91,8 @@ int main(int argc, const char **argv) {
   llvm::transform(Args, Argv.begin(),\r
                   [](const std::string &Arg) { return Arg.c_str(); });\r
 \r
-  IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();\r
-  unsigned MissingArgIndex, MissingArgCount;\r
-  auto Opts = driver::getDriverOptTable();\r
-  auto ParsedArgs = Opts.ParseArgs(llvm::makeArrayRef(Argv).slice(1),\r
-                                   MissingArgIndex, MissingArgCount);\r
-  ParseDiagnosticArgs(*DiagOpts, ParsedArgs);\r
+  IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts =\r
+      CreateAndPopulateDiagOpts(Argv);\r
 \r
   // Don't output diagnostics, because common scenarios such as\r
   // cross-compiling fail with diagnostics.  This is not fatal, but\r
index 5242134..2a051c4 100644 (file)
@@ -343,11 +343,8 @@ bool ToolInvocation::run() {
   for (const std::string &Str : CommandLine)
     Argv.push_back(Str.c_str());
   const char *const BinaryName = Argv[0];
-  IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
-  unsigned MissingArgIndex, MissingArgCount;
-  llvm::opt::InputArgList ParsedArgs = driver::getDriverOptTable().ParseArgs(
-      ArrayRef<const char *>(Argv).slice(1), MissingArgIndex, MissingArgCount);
-  ParseDiagnosticArgs(*DiagOpts, ParsedArgs);
+  IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts =
+      CreateAndPopulateDiagOpts(Argv);
   TextDiagnosticPrinter DiagnosticPrinter(
       llvm::errs(), &*DiagOpts);
   DiagnosticsEngine Diagnostics(
index 2113c3d..ad36bd6 100644 (file)
@@ -278,27 +278,6 @@ static void FixupDiagPrefixExeName(TextDiagnosticPrinter *DiagClient,
   DiagClient->setPrefix(std::string(ExeBasename));
 }
 
-// This lets us create the DiagnosticsEngine with a properly-filled-out
-// DiagnosticOptions instance.
-static DiagnosticOptions *
-CreateAndPopulateDiagOpts(ArrayRef<const char *> argv, bool &UseNewCC1Process) {
-  auto *DiagOpts = new DiagnosticOptions;
-  unsigned MissingArgIndex, MissingArgCount;
-  InputArgList Args = getDriverOptTable().ParseArgs(
-      argv.slice(1), MissingArgIndex, MissingArgCount);
-  // We ignore MissingArgCount and the return value of ParseDiagnosticArgs.
-  // Any errors that would be diagnosed here will also be diagnosed later,
-  // when the DiagnosticsEngine actually exists.
-  (void)ParseDiagnosticArgs(*DiagOpts, Args);
-
-  UseNewCC1Process =
-      Args.hasFlag(clang::driver::options::OPT_fno_integrated_cc1,
-                   clang::driver::options::OPT_fintegrated_cc1,
-                   /*Default=*/CLANG_SPAWN_CC1);
-
-  return DiagOpts;
-}
-
 static void SetInstallDir(SmallVectorImpl<const char *> &argv,
                           Driver &TheDriver, bool CanonicalPrefixes) {
   // Attempt to find the original path used to invoke the driver, to determine
@@ -459,10 +438,15 @@ int main(int Argc, const char **Argv) {
   // should spawn a new clang subprocess (old behavior).
   // Not having an additional process saves some execution time of Windows,
   // and makes debugging and profiling easier.
-  bool UseNewCC1Process;
+  bool UseNewCC1Process = CLANG_SPAWN_CC1;
+  for (const char *Arg : Args)
+    UseNewCC1Process = llvm::StringSwitch<bool>(Arg)
+                           .Case("-fno-integrated-cc1", true)
+                           .Case("-fintegrated-cc1", false)
+                           .Default(UseNewCC1Process);
 
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts =
-      CreateAndPopulateDiagOpts(Args, UseNewCC1Process);
+      CreateAndPopulateDiagOpts(Args);
 
   TextDiagnosticPrinter *DiagClient
     = new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);