jansvoboda11 created this revision. jansvoboda11 added reviewers: Bigcheese, dexonsmith, arphaman. jansvoboda11 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
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, essentialy 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 function: `CreateAndPopulateDiagOpts`. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D108918 Files: 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: clang/tools/driver/driver.cpp =================================================================== --- clang/tools/driver/driver.cpp +++ clang/tools/driver/driver.cpp @@ -278,27 +278,6 @@ 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 @@ -462,7 +441,7 @@ bool UseNewCC1Process; IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = - CreateAndPopulateDiagOpts(Args, UseNewCC1Process); + CreateAndPopulateDiagOpts(Args, &UseNewCC1Process); TextDiagnosticPrinter *DiagClient = new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts); Index: clang/lib/Tooling/Tooling.cpp =================================================================== --- clang/lib/Tooling/Tooling.cpp +++ clang/lib/Tooling/Tooling.cpp @@ -343,11 +343,8 @@ 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: clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp =================================================================== --- clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp +++ clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp @@ -91,12 +91,8 @@ llvm::transform(Args, Argv.begin(), [](const std::string &Arg) { return Arg.c_str(); }); - IntrusiveRefCntPtr<DiagnosticOptions> 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<DiagnosticOptions> DiagOpts = + CreateAndPopulateDiagOpts(Argv); // Don't output diagnostics, because common scenarios such as // cross-compiling fail with diagnostics. This is not fatal, but Index: clang/lib/Interpreter/Interpreter.cpp =================================================================== --- clang/lib/Interpreter/Interpreter.cpp +++ clang/lib/Interpreter/Interpreter.cpp @@ -147,15 +147,10 @@ // 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: clang/lib/Frontend/CompilerInvocation.cpp =================================================================== --- clang/lib/Frontend/CompilerInvocation.cpp +++ clang/lib/Frontend/CompilerInvocation.cpp @@ -2256,6 +2256,26 @@ } } +DiagnosticOptions *clang::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); + + if (UseNewCC1Process) + *UseNewCC1Process = + Args.hasFlag(clang::driver::options::OPT_fno_integrated_cc1, + clang::driver::options::OPT_fintegrated_cc1, + /*Default=*/CLANG_SPAWN_CC1); + + return DiagOpts; +} + bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args, DiagnosticsEngine *Diags, bool DefaultDiagColor) { Index: clang/include/clang/Frontend/CompilerInvocation.h =================================================================== --- clang/include/clang/Frontend/CompilerInvocation.h +++ clang/include/clang/Frontend/CompilerInvocation.h @@ -50,6 +50,11 @@ class PreprocessorOptions; class TargetOptions; +// This lets us create the DiagnosticsEngine with a properly-filled-out +// DiagnosticOptions instance. +DiagnosticOptions *CreateAndPopulateDiagOpts(ArrayRef<const char *> Argv, + bool *UseNewCC1Process = nullptr); + /// Fill out Opts based on the options given in Args. /// /// Args must have been created from the OptTable returned by
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits