From 555a817d1dac5d88fdb445cd7c93cf66b769bc37 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Mon, 30 Aug 2021 15:41:53 +0200 Subject: [PATCH] [clang] NFC: Extract DiagnosticOptions parsing 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`). 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 | 5 ++++ clang/lib/Frontend/CompilerInvocation.cpp | 13 ++++++++++ clang/lib/Interpreter/Interpreter.cpp | 9 ++----- clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp | 8 ++---- clang/lib/Tooling/Tooling.cpp | 7 ++---- clang/tools/driver/driver.cpp | 30 ++++++----------------- 6 files changed, 31 insertions(+), 41 deletions(-) diff --git a/clang/include/clang/Frontend/CompilerInvocation.h b/clang/include/clang/Frontend/CompilerInvocation.h index 2245439..922c84a 100644 --- a/clang/include/clang/Frontend/CompilerInvocation.h +++ b/clang/include/clang/Frontend/CompilerInvocation.h @@ -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 +CreateAndPopulateDiagOpts(ArrayRef Argv); + /// Fill out Opts based on the options given in Args. /// /// Args must have been created from the OptTable returned by diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 64800b1..0da2cb3 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -2256,6 +2256,19 @@ void CompilerInvocation::GenerateDiagnosticArgs( } } +std::unique_ptr +clang::CreateAndPopulateDiagOpts(ArrayRef Argv) { + auto DiagOpts = std::make_unique(); + 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) { diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp index 937504f..3e8d388 100644 --- a/clang/lib/Interpreter/Interpreter.cpp +++ b/clang/lib/Interpreter/Interpreter.cpp @@ -147,15 +147,10 @@ IncrementalCompilerBuilder::create(std::vector &ClangArgv) { // Buffer diagnostics from argument parsing so that we can output them using a // well formed diagnostic object. IntrusiveRefCntPtr DiagID(new DiagnosticIDs()); - IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions(); + IntrusiveRefCntPtr 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(ClangArgv).slice(1), - MissingArgIndex, MissingArgCount); - ParseDiagnosticArgs(*DiagOpts, ParsedArgs, &Diags); driver::Driver Driver(/*MainBinaryName=*/ClangArgv[0], llvm::sys::getProcessTriple(), Diags); diff --git a/clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp b/clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp index 8091a46..9c82542 100644 --- a/clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp +++ b/clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp @@ -91,12 +91,8 @@ int main(int argc, const char **argv) { llvm::transform(Args, Argv.begin(), [](const std::string &Arg) { return Arg.c_str(); }); - IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions(); - unsigned MissingArgIndex, MissingArgCount; - auto Opts = driver::getDriverOptTable(); - auto ParsedArgs = Opts.ParseArgs(llvm::makeArrayRef(Argv).slice(1), - MissingArgIndex, MissingArgCount); - ParseDiagnosticArgs(*DiagOpts, ParsedArgs); + IntrusiveRefCntPtr DiagOpts = + CreateAndPopulateDiagOpts(Argv); // Don't output diagnostics, because common scenarios such as // cross-compiling fail with diagnostics. This is not fatal, but diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp index 5242134..2a051c4 100644 --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -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 DiagOpts = new DiagnosticOptions(); - unsigned MissingArgIndex, MissingArgCount; - llvm::opt::InputArgList ParsedArgs = driver::getDriverOptTable().ParseArgs( - ArrayRef(Argv).slice(1), MissingArgIndex, MissingArgCount); - ParseDiagnosticArgs(*DiagOpts, ParsedArgs); + IntrusiveRefCntPtr DiagOpts = + CreateAndPopulateDiagOpts(Argv); TextDiagnosticPrinter DiagnosticPrinter( llvm::errs(), &*DiagOpts); DiagnosticsEngine Diagnostics( diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index 2113c3d..ad36bd6 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -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 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 &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(Arg) + .Case("-fno-integrated-cc1", true) + .Case("-fintegrated-cc1", false) + .Default(UseNewCC1Process); IntrusiveRefCntPtr DiagOpts = - CreateAndPopulateDiagOpts(Args, UseNewCC1Process); + CreateAndPopulateDiagOpts(Args); TextDiagnosticPrinter *DiagClient = new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts); -- 2.7.4