Skip to content

Commit b69dcb8

Browse files
jansvoboda11jansvoboda11
authored
[clang][frontend] Require invocation to construct CompilerInstance (#137668)
This PR makes it so that `CompilerInvocation` needs to be provided to `CompilerInstance` on construction. There are a couple of benefits in my view: * Making it impossible to mis-use some `CompilerInstance` APIs. For example there are cases, where `createDiagnostics()` was called before `setInvocation()`, causing the `DiagnosticEngine` to use the default-constructed `DiagnosticOptions` instead of the intended ones. * This shrinks `CompilerInstance`'s state space. * This makes it possible to access **the** invocation in `CompilerInstance`'s constructor (to be used in a follow-up).
1 parent 0009a17 commit b69dcb8

33 files changed

+105
-148
lines changed

‎clang-tools-extra/clang-include-fixer/IncludeFixer.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ bool IncludeFixerActionFactory::runInvocation(
8989
assert(Invocation->getFrontendOpts().Inputs.size() == 1);
9090

9191
// Set up Clang.
92-
clang::CompilerInstance Compiler(PCHContainerOps);
93-
Compiler.setInvocation(std::move(Invocation));
92+
CompilerInstance Compiler(std::move(Invocation), std::move(PCHContainerOps));
9493
Compiler.setFileManager(Files);
9594

9695
// Create the compiler's actual diagnostics engine. We want to drop all

‎clang-tools-extra/clangd/Compiler.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,7 @@ prepareCompilerInstance(std::unique_ptr<clang::CompilerInvocation> CI,
145145
CI->getFrontendOpts().Inputs[0].getFile(), Buffer.get());
146146
}
147147

148-
auto Clang = std::make_unique<CompilerInstance>(
149-
std::make_shared<PCHContainerOperations>());
150-
Clang->setInvocation(std::move(CI));
148+
auto Clang = std::make_unique<CompilerInstance>(std::move(CI));
151149
Clang->createDiagnostics(*VFS, &DiagsClient, false);
152150

153151
if (auto VFSWithRemapping = createVFSFromCompilerInvocation(

‎clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -618,14 +618,14 @@ TEST_F(PragmaIncludeTest, ExportInUnnamedBuffer) {
618618
llvm::MemoryBuffer::getMemBufferCopy(Extra.getValue(),
619619
/*BufferName=*/""));
620620

621-
auto Clang = std::make_unique<CompilerInstance>(
622-
std::make_shared<PCHContainerOperations>());
623-
Clang->createDiagnostics(*VFS);
621+
auto DiagOpts = llvm::makeIntrusiveRefCnt<DiagnosticOptions>();
622+
auto Diags = CompilerInstance::createDiagnostics(*VFS, DiagOpts.get());
623+
auto Invocation = std::make_unique<CompilerInvocation>();
624+
ASSERT_TRUE(CompilerInvocation::CreateFromArgs(*Invocation, {Filename.data()},
625+
*Diags, "clang"));
624626

625-
Clang->setInvocation(std::make_unique<CompilerInvocation>());
626-
ASSERT_TRUE(CompilerInvocation::CreateFromArgs(
627-
Clang->getInvocation(), {Filename.data()}, Clang->getDiagnostics(),
628-
"clang"));
627+
auto Clang = std::make_unique<CompilerInstance>(std::move(Invocation));
628+
Clang->createDiagnostics(*VFS);
629629

630630
auto *FM = Clang->createFileManager(VFS);
631631
ASSERT_TRUE(Clang->ExecuteAction(*Inputs.MakeAction()));

‎clang/include/clang/Frontend/CompilerInstance.h

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,8 @@ class CompilerInstance : public ModuleLoader {
204204
void operator=(const CompilerInstance &) = delete;
205205
public:
206206
explicit CompilerInstance(
207+
std::shared_ptr<CompilerInvocation> Invocation =
208+
std::make_shared<CompilerInvocation>(),
207209
std::shared_ptr<PCHContainerOperations> PCHContainerOps =
208210
std::make_shared<PCHContainerOperations>(),
209211
ModuleCache *ModCache = nullptr);
@@ -251,18 +253,10 @@ class CompilerInstance : public ModuleLoader {
251253
/// @name Compiler Invocation and Options
252254
/// @{
253255

254-
bool hasInvocation() const { return Invocation != nullptr; }
255-
256-
CompilerInvocation &getInvocation() {
257-
assert(Invocation && "Compiler instance has no invocation!");
258-
return *Invocation;
259-
}
256+
CompilerInvocation &getInvocation() { return *Invocation; }
260257

261258
std::shared_ptr<CompilerInvocation> getInvocationPtr() { return Invocation; }
262259

263-
/// setInvocation - Replace the current invocation.
264-
void setInvocation(std::shared_ptr<CompilerInvocation> Value);
265-
266260
/// Indicates whether we should (re)build the global module index.
267261
bool shouldBuildGlobalModuleIndex() const;
268262

‎clang/lib/Frontend/ASTUnit.cpp

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,9 +1162,8 @@ bool ASTUnit::Parse(std::shared_ptr<PCHContainerOperations> PCHContainerOps,
11621162
}
11631163

11641164
// Create the compiler instance to use for building the AST.
1165-
std::unique_ptr<CompilerInstance> Clang(
1166-
new CompilerInstance(std::move(PCHContainerOps)));
1167-
Clang->setInvocation(CCInvocation);
1165+
auto Clang = std::make_unique<CompilerInstance>(CCInvocation,
1166+
std::move(PCHContainerOps));
11681167

11691168
// Clean up on error, disengage it if the function returns successfully.
11701169
auto CleanOnError = llvm::make_scope_exit([&]() {
@@ -1487,7 +1486,6 @@ void ASTUnit::RealizeTopLevelDeclsFromPreamble() {
14871486
void ASTUnit::transferASTDataFromCompilerInstance(CompilerInstance &CI) {
14881487
// Steal the created target, context, and preprocessor if they have been
14891488
// created.
1490-
assert(CI.hasInvocation() && "missing invocation");
14911489
LangOpts = std::make_unique<LangOptions>(CI.getInvocation().getLangOpts());
14921490
TheSema = CI.takeSema();
14931491
Consumer = CI.takeASTConsumer();
@@ -1601,14 +1599,13 @@ ASTUnit *ASTUnit::LoadFromCompilerInvocationAction(
16011599
AST->getFileManager().getVirtualFileSystem());
16021600

16031601
// Create the compiler instance to use for building the AST.
1604-
std::unique_ptr<CompilerInstance> Clang(
1605-
new CompilerInstance(std::move(PCHContainerOps)));
1602+
auto Clang = std::make_unique<CompilerInstance>(std::move(CI),
1603+
std::move(PCHContainerOps));
16061604

16071605
// Recover resources if we crash before exiting this method.
16081606
llvm::CrashRecoveryContextCleanupRegistrar<CompilerInstance>
16091607
CICleanup(Clang.get());
16101608

1611-
Clang->setInvocation(std::move(CI));
16121609
AST->OriginalSourceFile =
16131610
std::string(Clang->getFrontendOpts().Inputs[0].getFile());
16141611

@@ -2232,15 +2229,14 @@ void ASTUnit::CodeComplete(
22322229
LangOpts.SpellChecking = false;
22332230
CCInvocation->getDiagnosticOpts().IgnoreWarnings = true;
22342231

2235-
std::unique_ptr<CompilerInstance> Clang(
2236-
new CompilerInstance(PCHContainerOps));
2232+
auto Clang = std::make_unique<CompilerInstance>(std::move(CCInvocation),
2233+
PCHContainerOps);
22372234

22382235
// Recover resources if we crash before exiting this method.
22392236
llvm::CrashRecoveryContextCleanupRegistrar<CompilerInstance>
22402237
CICleanup(Clang.get());
22412238

2242-
auto &Inv = *CCInvocation;
2243-
Clang->setInvocation(std::move(CCInvocation));
2239+
auto &Inv = Clang->getInvocation();
22442240
OriginalSourceFile =
22452241
std::string(Clang->getFrontendOpts().Inputs[0].getFile());
22462242

@@ -2254,7 +2250,6 @@ void ASTUnit::CodeComplete(
22542250

22552251
// Create the target instance.
22562252
if (!Clang->createTarget()) {
2257-
Clang->setInvocation(nullptr);
22582253
return;
22592254
}
22602255

‎clang/lib/Frontend/ChainedIncludesSource.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,8 @@ IntrusiveRefCntPtr<ExternalSemaSource> clang::createChainedIncludesSource(
122122
IntrusiveRefCntPtr<DiagnosticsEngine> Diags(
123123
new DiagnosticsEngine(DiagID, &CI.getDiagnosticOpts(), DiagClient));
124124

125-
std::unique_ptr<CompilerInstance> Clang(
126-
new CompilerInstance(CI.getPCHContainerOperations()));
127-
Clang->setInvocation(std::move(CInvok));
125+
auto Clang = std::make_unique<CompilerInstance>(
126+
std::move(CInvok), CI.getPCHContainerOperations());
128127
Clang->setDiagnostics(Diags.get());
129128
Clang->setTarget(TargetInfo::CreateTargetInfo(
130129
Clang->getDiagnostics(), Clang->getInvocation().getTargetOpts()));

‎clang/lib/Frontend/CompilerInstance.cpp

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,22 +67,20 @@
6767
using namespace clang;
6868

6969
CompilerInstance::CompilerInstance(
70+
std::shared_ptr<CompilerInvocation> Invocation,
7071
std::shared_ptr<PCHContainerOperations> PCHContainerOps,
7172
ModuleCache *ModCache)
7273
: ModuleLoader(/*BuildingModule=*/ModCache),
73-
Invocation(new CompilerInvocation()),
74+
Invocation(std::move(Invocation)),
7475
ModCache(ModCache ? ModCache : createCrossProcessModuleCache()),
75-
ThePCHContainerOperations(std::move(PCHContainerOps)) {}
76+
ThePCHContainerOperations(std::move(PCHContainerOps)) {
77+
assert(this->Invocation && "Invocation must not be null");
78+
}
7679

7780
CompilerInstance::~CompilerInstance() {
7881
assert(OutputFiles.empty() && "Still output files in flight?");
7982
}
8083

81-
void CompilerInstance::setInvocation(
82-
std::shared_ptr<CompilerInvocation> Value) {
83-
Invocation = std::move(Value);
84-
}
85-
8684
bool CompilerInstance::shouldBuildGlobalModuleIndex() const {
8785
return (BuildGlobalModuleIndex ||
8886
(TheASTReader && TheASTReader->isGlobalIndexUnavailable() &&
@@ -1210,11 +1208,10 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompileImpl(
12101208
// CompilerInstance::CompilerInstance is responsible for finalizing the
12111209
// buffers to prevent use-after-frees.
12121210
auto InstancePtr = std::make_unique<CompilerInstance>(
1213-
getPCHContainerOperations(), &getModuleCache());
1211+
std::move(Invocation), getPCHContainerOperations(), &getModuleCache());
12141212
auto &Instance = *InstancePtr;
12151213

1216-
auto &Inv = *Invocation;
1217-
Instance.setInvocation(std::move(Invocation));
1214+
auto &Inv = Instance.getInvocation();
12181215

12191216
if (ThreadSafeConfig) {
12201217
Instance.createFileManager(ThreadSafeConfig->getVFS());

‎clang/lib/Frontend/PrecompiledPreamble.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -454,14 +454,13 @@ llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
454454
PreprocessorOpts.GeneratePreamble = true;
455455

456456
// Create the compiler instance to use for building the precompiled preamble.
457-
std::unique_ptr<CompilerInstance> Clang(
458-
new CompilerInstance(std::move(PCHContainerOps)));
457+
auto Clang = std::make_unique<CompilerInstance>(std::move(PreambleInvocation),
458+
std::move(PCHContainerOps));
459459

460460
// Recover resources if we crash before exiting this method.
461461
llvm::CrashRecoveryContextCleanupRegistrar<CompilerInstance> CICleanup(
462462
Clang.get());
463463

464-
Clang->setInvocation(std::move(PreambleInvocation));
465464
Clang->setDiagnostics(&Diagnostics);
466465

467466
// Create the target instance.

‎clang/lib/Frontend/Rewrite/FrontendActions.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,9 @@ class RewriteIncludesAction::RewriteImportsListener : public ASTReaderListener {
242242
(*OS) << '\n';
243243

244244
// Rewrite the contents of the module in a separate compiler instance.
245-
CompilerInstance Instance(CI.getPCHContainerOperations(),
246-
&CI.getModuleCache());
247-
Instance.setInvocation(
248-
std::make_shared<CompilerInvocation>(CI.getInvocation()));
245+
CompilerInstance Instance(
246+
std::make_shared<CompilerInvocation>(CI.getInvocation()),
247+
CI.getPCHContainerOperations(), &CI.getModuleCache());
249248
Instance.createDiagnostics(
250249
CI.getVirtualFileSystem(),
251250
new ForwardingDiagnosticConsumer(CI.getDiagnosticClient()),

‎clang/lib/StaticAnalyzer/Frontend/ModelInjector.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,8 @@ void ModelInjector::onBodySynthesis(const NamedDecl *D) {
7575

7676
// Modules are parsed by a separate CompilerInstance, so this code mimics that
7777
// behavior for models
78-
CompilerInstance Instance(CI.getPCHContainerOperations());
79-
Instance.setInvocation(std::move(Invocation));
78+
CompilerInstance Instance(std::move(Invocation),
79+
CI.getPCHContainerOperations());
8080
Instance.createDiagnostics(
8181
CI.getVirtualFileSystem(),
8282
new ForwardingDiagnosticConsumer(CI.getDiagnosticClient()),

‎clang/lib/Testing/TestAST.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,7 @@ void createMissingComponents(CompilerInstance &Clang) {
7575
} // namespace
7676

7777
TestAST::TestAST(const TestInputs &In) {
78-
Clang = std::make_unique<CompilerInstance>(
79-
std::make_shared<PCHContainerOperations>());
78+
Clang = std::make_unique<CompilerInstance>();
8079
// If we don't manage to finish parsing, create CompilerInstance components
8180
// anyway so that the test will see an empty AST instead of crashing.
8281
auto RecoverFromEarlyExit =
@@ -109,7 +108,6 @@ TestAST::TestAST(const TestInputs &In) {
109108
for (const auto &S : In.ExtraArgs)
110109
Argv.push_back(S.c_str());
111110
Argv.push_back(Filename.c_str());
112-
Clang->setInvocation(std::make_unique<CompilerInvocation>());
113111
if (!CompilerInvocation::CreateFromArgs(Clang->getInvocation(), Argv,
114112
Clang->getDiagnostics(), "clang")) {
115113
ADD_FAILURE() << "Failed to create invocation";

‎clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -412,9 +412,9 @@ class DependencyScanningAction : public tooling::ToolAction {
412412

413413
// Create a compiler instance to handle the actual work.
414414
auto ModCache = makeInProcessModuleCache(Service.getModuleCacheMutexes());
415-
ScanInstanceStorage.emplace(std::move(PCHContainerOps), ModCache.get());
415+
ScanInstanceStorage.emplace(std::move(Invocation),
416+
std::move(PCHContainerOps), ModCache.get());
416417
CompilerInstance &ScanInstance = *ScanInstanceStorage;
417-
ScanInstance.setInvocation(std::move(Invocation));
418418
ScanInstance.setBuildingModule(false);
419419

420420
// Create the compiler's actual diagnostics engine.

‎clang/lib/Tooling/Tooling.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -447,8 +447,7 @@ bool FrontendActionFactory::runInvocation(
447447
std::shared_ptr<PCHContainerOperations> PCHContainerOps,
448448
DiagnosticConsumer *DiagConsumer) {
449449
// Create a compiler instance to handle the actual work.
450-
CompilerInstance Compiler(std::move(PCHContainerOps));
451-
Compiler.setInvocation(std::move(Invocation));
450+
CompilerInstance Compiler(std::move(Invocation), std::move(PCHContainerOps));
452451
Compiler.setFileManager(Files);
453452

454453
// The FrontendAction can have lifetime requirements for Compiler or its

‎clang/tools/clang-import-test/clang-import-test.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -162,18 +162,18 @@ class TestDiagnosticConsumer : public DiagnosticConsumer {
162162
};
163163

164164
std::unique_ptr<CompilerInstance> BuildCompilerInstance() {
165-
auto Ins = std::make_unique<CompilerInstance>();
165+
auto DiagOpts = llvm::makeIntrusiveRefCnt<DiagnosticOptions>();
166166
auto DC = std::make_unique<TestDiagnosticConsumer>();
167-
const bool ShouldOwnClient = true;
168-
Ins->createDiagnostics(*llvm::vfs::getRealFileSystem(), DC.release(),
169-
ShouldOwnClient);
167+
auto Diags = CompilerInstance::createDiagnostics(
168+
*llvm::vfs::getRealFileSystem(), DiagOpts.get(), DC.get(),
169+
/*ShouldOwnClient=*/false);
170170

171171
auto Inv = std::make_unique<CompilerInvocation>();
172172

173173
std::vector<const char *> ClangArgv(ClangArgs.size());
174174
std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
175175
[](const std::string &s) -> const char * { return s.data(); });
176-
CompilerInvocation::CreateFromArgs(*Inv, ClangArgv, Ins->getDiagnostics());
176+
CompilerInvocation::CreateFromArgs(*Inv, ClangArgv, *Diags);
177177

178178
{
179179
using namespace driver::types;
@@ -205,7 +205,10 @@ std::unique_ptr<CompilerInstance> BuildCompilerInstance() {
205205
Inv->getCodeGenOpts().setDebugInfo(llvm::codegenoptions::FullDebugInfo);
206206
Inv->getTargetOpts().Triple = llvm::sys::getDefaultTargetTriple();
207207

208-
Ins->setInvocation(std::move(Inv));
208+
auto Ins = std::make_unique<CompilerInstance>(std::move(Inv));
209+
210+
Ins->createDiagnostics(*llvm::vfs::getRealFileSystem(), DC.release(),
211+
/*ShouldOwnClient=*/true);
209212

210213
TargetInfo *TI = TargetInfo::CreateTargetInfo(
211214
Ins->getDiagnostics(), Ins->getInvocation().getTargetOpts());

‎clang/tools/driver/cc1_main.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,11 +217,10 @@ static int PrintEnabledExtensions(const TargetOptions& TargetOpts) {
217217
int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
218218
ensureSufficientStack();
219219

220-
std::unique_ptr<CompilerInstance> Clang(new CompilerInstance());
221220
IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
222221

223222
// Register the support for object-file-wrapped Clang modules.
224-
auto PCHOps = Clang->getPCHContainerOperations();
223+
auto PCHOps = std::make_shared<PCHContainerOperations>();
225224
PCHOps->registerWriter(std::make_unique<ObjectFilePCHContainerWriter>());
226225
PCHOps->registerReader(std::make_unique<ObjectFilePCHContainerReader>());
227226

@@ -242,8 +241,12 @@ int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
242241
Diags.setSeverity(diag::remark_cc1_round_trip_generated,
243242
diag::Severity::Remark, {});
244243

245-
bool Success = CompilerInvocation::CreateFromArgs(Clang->getInvocation(),
246-
Argv, Diags, Argv0);
244+
auto Invocation = std::make_shared<CompilerInvocation>();
245+
bool Success =
246+
CompilerInvocation::CreateFromArgs(*Invocation, Argv, Diags, Argv0);
247+
248+
auto Clang = std::make_unique<CompilerInstance>(std::move(Invocation),
249+
std::move(PCHOps));
247250

248251
if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
249252
llvm::timeTraceProfilerInitialize(

‎clang/unittests/AST/ExternalASTSourceTest.cpp

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,24 +44,25 @@ class TestFrontendAction : public ASTFrontendAction {
4444
IntrusiveRefCntPtr<ExternalASTSource> Source;
4545
};
4646

47-
bool testExternalASTSource(ExternalASTSource *Source,
48-
StringRef FileContents) {
49-
CompilerInstance Compiler;
50-
Compiler.createDiagnostics(*llvm::vfs::getRealFileSystem());
47+
bool testExternalASTSource(ExternalASTSource *Source, StringRef FileContents) {
5148

5249
auto Invocation = std::make_shared<CompilerInvocation>();
5350
Invocation->getPreprocessorOpts().addRemappedFile(
5451
"test.cc", MemoryBuffer::getMemBuffer(FileContents).release());
5552
const char *Args[] = { "test.cc" };
56-
CompilerInvocation::CreateFromArgs(*Invocation, Args,
57-
Compiler.getDiagnostics());
58-
Compiler.setInvocation(std::move(Invocation));
53+
54+
auto InvocationDiagOpts = llvm::makeIntrusiveRefCnt<DiagnosticOptions>();
55+
auto InvocationDiags = CompilerInstance::createDiagnostics(
56+
*llvm::vfs::getRealFileSystem(), InvocationDiagOpts.get());
57+
CompilerInvocation::CreateFromArgs(*Invocation, Args, *InvocationDiags);
58+
59+
CompilerInstance Compiler(std::move(Invocation));
60+
Compiler.createDiagnostics(*llvm::vfs::getRealFileSystem());
5961

6062
TestFrontendAction Action(Source);
6163
return Compiler.ExecuteAction(Action);
6264
}
6365

64-
6566
// Ensure that a failed name lookup into an external source only occurs once.
6667
TEST(ExternalASTSourceTest, FailedLookupOccursOnce) {
6768
struct TestSource : ExternalASTSource {

0 commit comments

Comments
 (0)