Skip to content

Commit 555a817

Browse files
committed
[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<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
1 parent 3f1f08f commit 555a817

File tree

6 files changed

+31
-41
lines changed

6 files changed

+31
-41
lines changed

clang/include/clang/Frontend/CompilerInvocation.h

+5
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ class HeaderSearchOptions;
5050
class PreprocessorOptions;
5151
class TargetOptions;
5252

53+
// This lets us create the DiagnosticsEngine with a properly-filled-out
54+
// DiagnosticOptions instance.
55+
std::unique_ptr<DiagnosticOptions>
56+
CreateAndPopulateDiagOpts(ArrayRef<const char *> Argv);
57+
5358
/// Fill out Opts based on the options given in Args.
5459
///
5560
/// Args must have been created from the OptTable returned by

clang/lib/Frontend/CompilerInvocation.cpp

+13
Original file line numberDiff line numberDiff line change
@@ -2256,6 +2256,19 @@ void CompilerInvocation::GenerateDiagnosticArgs(
22562256
}
22572257
}
22582258

2259+
std::unique_ptr<DiagnosticOptions>
2260+
clang::CreateAndPopulateDiagOpts(ArrayRef<const char *> Argv) {
2261+
auto DiagOpts = std::make_unique<DiagnosticOptions>();
2262+
unsigned MissingArgIndex, MissingArgCount;
2263+
InputArgList Args = getDriverOptTable().ParseArgs(
2264+
Argv.slice(1), MissingArgIndex, MissingArgCount);
2265+
// We ignore MissingArgCount and the return value of ParseDiagnosticArgs.
2266+
// Any errors that would be diagnosed here will also be diagnosed later,
2267+
// when the DiagnosticsEngine actually exists.
2268+
(void)ParseDiagnosticArgs(*DiagOpts, Args);
2269+
return DiagOpts;
2270+
}
2271+
22592272
bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
22602273
DiagnosticsEngine *Diags,
22612274
bool DefaultDiagColor) {

clang/lib/Interpreter/Interpreter.cpp

+2-7
Original file line numberDiff line numberDiff line change
@@ -147,15 +147,10 @@ IncrementalCompilerBuilder::create(std::vector<const char *> &ClangArgv) {
147147
// Buffer diagnostics from argument parsing so that we can output them using a
148148
// well formed diagnostic object.
149149
IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
150-
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
150+
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts =
151+
CreateAndPopulateDiagOpts(ClangArgv);
151152
TextDiagnosticBuffer *DiagsBuffer = new TextDiagnosticBuffer;
152153
DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagsBuffer);
153-
unsigned MissingArgIndex, MissingArgCount;
154-
const llvm::opt::OptTable &Opts = driver::getDriverOptTable();
155-
llvm::opt::InputArgList ParsedArgs =
156-
Opts.ParseArgs(ArrayRef<const char *>(ClangArgv).slice(1),
157-
MissingArgIndex, MissingArgCount);
158-
ParseDiagnosticArgs(*DiagOpts, ParsedArgs, &Diags);
159154

160155
driver::Driver Driver(/*MainBinaryName=*/ClangArgv[0],
161156
llvm::sys::getProcessTriple(), Diags);

clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp

+2-6
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,8 @@ int main(int argc, const char **argv) {
9191
llvm::transform(Args, Argv.begin(),
9292
[](const std::string &Arg) { return Arg.c_str(); });
9393

94-
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
95-
unsigned MissingArgIndex, MissingArgCount;
96-
auto Opts = driver::getDriverOptTable();
97-
auto ParsedArgs = Opts.ParseArgs(llvm::makeArrayRef(Argv).slice(1),
98-
MissingArgIndex, MissingArgCount);
99-
ParseDiagnosticArgs(*DiagOpts, ParsedArgs);
94+
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts =
95+
CreateAndPopulateDiagOpts(Argv);
10096

10197
// Don't output diagnostics, because common scenarios such as
10298
// cross-compiling fail with diagnostics. This is not fatal, but

clang/lib/Tooling/Tooling.cpp

+2-5
Original file line numberDiff line numberDiff line change
@@ -343,11 +343,8 @@ bool ToolInvocation::run() {
343343
for (const std::string &Str : CommandLine)
344344
Argv.push_back(Str.c_str());
345345
const char *const BinaryName = Argv[0];
346-
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
347-
unsigned MissingArgIndex, MissingArgCount;
348-
llvm::opt::InputArgList ParsedArgs = driver::getDriverOptTable().ParseArgs(
349-
ArrayRef<const char *>(Argv).slice(1), MissingArgIndex, MissingArgCount);
350-
ParseDiagnosticArgs(*DiagOpts, ParsedArgs);
346+
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts =
347+
CreateAndPopulateDiagOpts(Argv);
351348
TextDiagnosticPrinter DiagnosticPrinter(
352349
llvm::errs(), &*DiagOpts);
353350
DiagnosticsEngine Diagnostics(

clang/tools/driver/driver.cpp

+7-23
Original file line numberDiff line numberDiff line change
@@ -278,27 +278,6 @@ static void FixupDiagPrefixExeName(TextDiagnosticPrinter *DiagClient,
278278
DiagClient->setPrefix(std::string(ExeBasename));
279279
}
280280

281-
// This lets us create the DiagnosticsEngine with a properly-filled-out
282-
// DiagnosticOptions instance.
283-
static DiagnosticOptions *
284-
CreateAndPopulateDiagOpts(ArrayRef<const char *> argv, bool &UseNewCC1Process) {
285-
auto *DiagOpts = new DiagnosticOptions;
286-
unsigned MissingArgIndex, MissingArgCount;
287-
InputArgList Args = getDriverOptTable().ParseArgs(
288-
argv.slice(1), MissingArgIndex, MissingArgCount);
289-
// We ignore MissingArgCount and the return value of ParseDiagnosticArgs.
290-
// Any errors that would be diagnosed here will also be diagnosed later,
291-
// when the DiagnosticsEngine actually exists.
292-
(void)ParseDiagnosticArgs(*DiagOpts, Args);
293-
294-
UseNewCC1Process =
295-
Args.hasFlag(clang::driver::options::OPT_fno_integrated_cc1,
296-
clang::driver::options::OPT_fintegrated_cc1,
297-
/*Default=*/CLANG_SPAWN_CC1);
298-
299-
return DiagOpts;
300-
}
301-
302281
static void SetInstallDir(SmallVectorImpl<const char *> &argv,
303282
Driver &TheDriver, bool CanonicalPrefixes) {
304283
// Attempt to find the original path used to invoke the driver, to determine
@@ -459,10 +438,15 @@ int main(int Argc, const char **Argv) {
459438
// should spawn a new clang subprocess (old behavior).
460439
// Not having an additional process saves some execution time of Windows,
461440
// and makes debugging and profiling easier.
462-
bool UseNewCC1Process;
441+
bool UseNewCC1Process = CLANG_SPAWN_CC1;
442+
for (const char *Arg : Args)
443+
UseNewCC1Process = llvm::StringSwitch<bool>(Arg)
444+
.Case("-fno-integrated-cc1", true)
445+
.Case("-fintegrated-cc1", false)
446+
.Default(UseNewCC1Process);
463447

464448
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts =
465-
CreateAndPopulateDiagOpts(Args, UseNewCC1Process);
449+
CreateAndPopulateDiagOpts(Args);
466450

467451
TextDiagnosticPrinter *DiagClient
468452
= new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);

0 commit comments

Comments
 (0)