[clang] [clang-tools-extra] [lldb] [clang] NFCI: Clean up `CompilerInstance::create{File,Source}Manager()` (PR #160748)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 29 09:03:00 PDT 2025
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/160748
>From b59d98d44ae1e3edc3d71ee99c7ce919537ddc06 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 22 Sep 2025 16:58:46 -0700
Subject: [PATCH 1/3] [clang] Clean up
`CompilerInstance::create{File,Source}Manager()`
---
.../clang-include-fixer/IncludeFixer.cpp | 2 +-
.../include-cleaner/unittests/RecordTest.cpp | 7 ++++---
clang/include/clang/Frontend/CompilerInstance.h | 6 ++----
clang/lib/ExtractAPI/ExtractAPIConsumer.cpp | 3 +--
clang/lib/Frontend/ChainedIncludesSource.cpp | 2 +-
clang/lib/Frontend/CompilerInstance.cpp | 13 +++++++------
clang/lib/Frontend/FrontendAction.cpp | 11 ++++-------
clang/lib/Testing/TestAST.cpp | 2 +-
.../DependencyScanning/DependencyScannerImpl.cpp | 11 ++++++-----
clang/lib/Tooling/Tooling.cpp | 2 +-
clang/tools/clang-import-test/clang-import-test.cpp | 2 +-
clang/unittests/CodeGen/TestCompiler.h | 2 +-
.../Serialization/ForceCheckFileInputTest.cpp | 4 ++--
.../DependencyScanning/DependencyScannerTest.cpp | 2 +-
14 files changed, 33 insertions(+), 36 deletions(-)
diff --git a/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp b/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
index d2ae13c022b23..e825547ba0134 100644
--- a/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
+++ b/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
@@ -96,7 +96,7 @@ bool IncludeFixerActionFactory::runInvocation(
// diagnostics here.
Compiler.createDiagnostics(new clang::IgnoringDiagConsumer,
/*ShouldOwnClient=*/true);
- Compiler.createSourceManager(*Files);
+ Compiler.createSourceManager();
// We abort on fatal errors so don't let a large number of errors become
// fatal. A missing #include can cause thousands of errors.
diff --git a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
index 3fb49796039f2..cbf7bae23b365 100644
--- a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -649,11 +649,12 @@ TEST_F(PragmaIncludeTest, ExportInUnnamedBuffer) {
Clang->createVirtualFileSystem(VFS);
Clang->createDiagnostics();
- auto *FM = Clang->createFileManager();
+ Clang->createFileManager();
+ FileManager &FM = Clang->getFileManager();
ASSERT_TRUE(Clang->ExecuteAction(*Inputs.MakeAction()));
EXPECT_THAT(
- PI.getExporters(llvm::cantFail(FM->getFileRef("foo.h")), *FM),
- testing::ElementsAre(llvm::cantFail(FM->getFileRef("exporter.h"))));
+ PI.getExporters(llvm::cantFail(FM.getFileRef("foo.h")), FM),
+ testing::ElementsAre(llvm::cantFail(FM.getFileRef("exporter.h"))));
}
TEST_F(PragmaIncludeTest, OutlivesFMAndSM) {
diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h
index a6b6993b708d0..44fff69c217c5 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -712,12 +712,10 @@ class CompilerInstance : public ModuleLoader {
const CodeGenOptions *CodeGenOpts = nullptr);
/// Create the file manager and replace any existing one with it.
- ///
- /// \return The new file manager on success, or null on failure.
- FileManager *createFileManager();
+ void createFileManager();
/// Create the source manager and replace any existing one with it.
- void createSourceManager(FileManager &FileMgr);
+ void createSourceManager();
/// Create the preprocessor, using the invocation, file, and source managers,
/// and replace any existing one with it.
diff --git a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
index 1087eb3001856..6966d4097d64a 100644
--- a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -444,8 +444,7 @@ bool ExtractAPIAction::PrepareToExecuteAction(CompilerInstance &CI) {
return true;
if (!CI.hasFileManager())
- if (!CI.createFileManager())
- return false;
+ CI.createFileManager();
auto Kind = Inputs[0].getKind();
diff --git a/clang/lib/Frontend/ChainedIncludesSource.cpp b/clang/lib/Frontend/ChainedIncludesSource.cpp
index 82249f893a795..049277c2df7a9 100644
--- a/clang/lib/Frontend/ChainedIncludesSource.cpp
+++ b/clang/lib/Frontend/ChainedIncludesSource.cpp
@@ -129,7 +129,7 @@ clang::createChainedIncludesSource(CompilerInstance &CI,
Clang->setTarget(TargetInfo::CreateTargetInfo(
Clang->getDiagnostics(), Clang->getInvocation().getTargetOpts()));
Clang->createFileManager();
- Clang->createSourceManager(Clang->getFileManager());
+ Clang->createSourceManager();
Clang->createPreprocessor(TU_Prefix);
Clang->getDiagnosticClient().BeginSourceFile(Clang->getLangOpts(),
&Clang->getPreprocessor());
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index c989ad2e5155c..286eb19167472 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -382,17 +382,18 @@ IntrusiveRefCntPtr<DiagnosticsEngine> CompilerInstance::createDiagnostics(
// File Manager
-FileManager *CompilerInstance::createFileManager() {
+void CompilerInstance::createFileManager() {
assert(VFS && "CompilerInstance needs a VFS for creating FileManager");
FileMgr = llvm::makeIntrusiveRefCnt<FileManager>(getFileSystemOpts(), VFS);
- return FileMgr.get();
}
// Source Manager
-void CompilerInstance::createSourceManager(FileManager &FileMgr) {
- SourceMgr =
- llvm::makeIntrusiveRefCnt<SourceManager>(getDiagnostics(), FileMgr);
+void CompilerInstance::createSourceManager() {
+ assert(Diagnostics && "FileManager needed for creating SourceManager");
+ assert(FileMgr && "DiagnosticsEngine needed for creating SourceManager");
+ SourceMgr = llvm::makeIntrusiveRefCnt<SourceManager>(getDiagnostics(),
+ getFileManager());
}
// Initialize the remapping of files to alternative contents, e.g.,
@@ -1186,7 +1187,7 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompileImpl(
if (llvm::is_contained(DiagOpts.SystemHeaderWarningsModules, ModuleName))
Instance.getDiagnostics().setSuppressSystemWarnings(false);
- Instance.createSourceManager(Instance.getFileManager());
+ Instance.createSourceManager();
SourceManager &SourceMgr = Instance.getSourceManager();
if (ThreadSafeConfig) {
diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp
index 6cc3b65a16cb2..1b63c40a6efd7 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -879,7 +879,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
// file, otherwise the CompilerInstance will happily destroy them.
CI.setVirtualFileSystem(AST->getFileManager().getVirtualFileSystemPtr());
CI.setFileManager(AST->getFileManagerPtr());
- CI.createSourceManager(CI.getFileManager());
+ CI.createSourceManager();
CI.getSourceManager().initializeForReplay(AST->getSourceManager());
// Preload all the module files loaded transitively by the AST unit. Also
@@ -971,13 +971,10 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
// Set up the file system, file and source managers, if needed.
if (!CI.hasVirtualFileSystem())
CI.createVirtualFileSystem();
- if (!CI.hasFileManager()) {
- if (!CI.createFileManager()) {
- return false;
- }
- }
+ if (!CI.hasFileManager())
+ CI.createFileManager();
if (!CI.hasSourceManager()) {
- CI.createSourceManager(CI.getFileManager());
+ CI.createSourceManager();
if (CI.getDiagnosticOpts().getFormat() == DiagnosticOptions::SARIF) {
static_cast<SARIFDiagnosticPrinter *>(&CI.getDiagnosticClient())
->setSarifWriter(
diff --git a/clang/lib/Testing/TestAST.cpp b/clang/lib/Testing/TestAST.cpp
index 9ad0de95530fb..d3338956f3043 100644
--- a/clang/lib/Testing/TestAST.cpp
+++ b/clang/lib/Testing/TestAST.cpp
@@ -61,7 +61,7 @@ void createMissingComponents(CompilerInstance &Clang) {
if (!Clang.hasFileManager())
Clang.createFileManager();
if (!Clang.hasSourceManager())
- Clang.createSourceManager(Clang.getFileManager());
+ Clang.createSourceManager();
if (!Clang.hasTarget())
Clang.createTarget();
if (!Clang.hasPreprocessor())
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp
index d370bfd0dd10f..94e7eca7d8c68 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp
@@ -415,7 +415,7 @@ bool DependencyScanningAction::runInvocation(
any(Service.getOptimizeArgs() & ScanningOptimizations::VFS);
// Create a new FileManager to match the invocation's FileSystemOptions.
- auto *FileMgr = ScanInstance.createFileManager();
+ ScanInstance.createFileManager();
// Use the dependency scanning optimized file system if requested to do so.
if (DepFS) {
@@ -423,16 +423,17 @@ bool DependencyScanningAction::runInvocation(
if (!ScanInstance.getHeaderSearchOpts().ModuleCachePath.empty()) {
SmallString<256> ModulesCachePath;
normalizeModuleCachePath(
- *FileMgr, ScanInstance.getHeaderSearchOpts().ModuleCachePath,
- ModulesCachePath);
+ ScanInstance.getFileManager(),
+ ScanInstance.getHeaderSearchOpts().ModuleCachePath, ModulesCachePath);
DepFS->setBypassedPathPrefix(ModulesCachePath);
}
ScanInstance.setDependencyDirectivesGetter(
- std::make_unique<ScanningDependencyDirectivesGetter>(*FileMgr));
+ std::make_unique<ScanningDependencyDirectivesGetter>(
+ ScanInstance.getFileManager()));
}
- ScanInstance.createSourceManager(*FileMgr);
+ ScanInstance.createSourceManager();
// Create a collection of stable directories derived from the ScanInstance
// for determining whether module dependencies would fully resolve from
diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp
index 2d4790b205b1a..ea5a37216e959 100644
--- a/clang/lib/Tooling/Tooling.cpp
+++ b/clang/lib/Tooling/Tooling.cpp
@@ -458,7 +458,7 @@ bool FrontendActionFactory::runInvocation(
if (!Compiler.hasDiagnostics())
return false;
- Compiler.createSourceManager(*Files);
+ Compiler.createSourceManager();
const bool Success = Compiler.ExecuteAction(*ScopedToolAction);
diff --git a/clang/tools/clang-import-test/clang-import-test.cpp b/clang/tools/clang-import-test/clang-import-test.cpp
index 910e08ca4dffa..977cec1d53157 100644
--- a/clang/tools/clang-import-test/clang-import-test.cpp
+++ b/clang/tools/clang-import-test/clang-import-test.cpp
@@ -216,7 +216,7 @@ std::unique_ptr<CompilerInstance> BuildCompilerInstance() {
Ins->getTarget().adjust(Ins->getDiagnostics(), Ins->getLangOpts(),
/*AuxTarget=*/nullptr);
Ins->createFileManager();
- Ins->createSourceManager(Ins->getFileManager());
+ Ins->createSourceManager();
Ins->createPreprocessor(TU_Complete);
return Ins;
diff --git a/clang/unittests/CodeGen/TestCompiler.h b/clang/unittests/CodeGen/TestCompiler.h
index 57b5b079a2e30..9bd90609fcd29 100644
--- a/clang/unittests/CodeGen/TestCompiler.h
+++ b/clang/unittests/CodeGen/TestCompiler.h
@@ -52,7 +52,7 @@ struct TestCompiler {
PtrSize = TInfo.getPointerWidth(clang::LangAS::Default) / 8;
compiler.createFileManager();
- compiler.createSourceManager(compiler.getFileManager());
+ compiler.createSourceManager();
compiler.createPreprocessor(clang::TU_Prefix);
compiler.createASTContext();
diff --git a/clang/unittests/Serialization/ForceCheckFileInputTest.cpp b/clang/unittests/Serialization/ForceCheckFileInputTest.cpp
index 24e2fd65f3c0a..edf33ae04230b 100644
--- a/clang/unittests/Serialization/ForceCheckFileInputTest.cpp
+++ b/clang/unittests/Serialization/ForceCheckFileInputTest.cpp
@@ -122,8 +122,8 @@ export int aa = 43;
Clang.setDiagnostics(Diags);
Clang.createVirtualFileSystem(CIOpts.VFS);
- FileManager *FM = Clang.createFileManager();
- Clang.createSourceManager(*FM);
+ Clang.createFileManager();
+ Clang.createSourceManager();
EXPECT_TRUE(Clang.createTarget());
Clang.createPreprocessor(TU_Complete);
diff --git a/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp b/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp
index 80289efd374cf..aa32bb3d39f6d 100644
--- a/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp
+++ b/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp
@@ -65,7 +65,7 @@ class TestDependencyScanningAction : public tooling::ToolAction {
if (!Compiler.hasDiagnostics())
return false;
- Compiler.createSourceManager(*FileMgr);
+ Compiler.createSourceManager();
Compiler.addDependencyCollector(std::make_shared<TestFileCollector>(
Compiler.getInvocation().getDependencyOutputOpts(), Deps));
>From 6fe5cf3889e7b976c9d4dd1c1033f2d596701ae9 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Thu, 25 Sep 2025 11:00:46 -0700
Subject: [PATCH 2/3] Fix LLDB build
---
.../Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 924953cc43fa2..3c49c911108a3 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -792,7 +792,7 @@ ClangExpressionParser::ClangExpressionParser(
// 6. Set up the source management objects inside the compiler
m_compiler->createFileManager();
if (!m_compiler->hasSourceManager())
- m_compiler->createSourceManager(m_compiler->getFileManager());
+ m_compiler->createSourceManager();
m_compiler->createPreprocessor(TU_Complete);
switch (expr.Language().AsLanguageType()) {
>From 0a7d5e4cb7232558d48121d820761ce2b9a73151 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Fri, 26 Sep 2025 11:35:27 -0700
Subject: [PATCH 3/3] Swap asserts
---
clang/lib/Frontend/CompilerInstance.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 286eb19167472..00a0e93cb85c7 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -390,8 +390,8 @@ void CompilerInstance::createFileManager() {
// Source Manager
void CompilerInstance::createSourceManager() {
- assert(Diagnostics && "FileManager needed for creating SourceManager");
- assert(FileMgr && "DiagnosticsEngine needed for creating SourceManager");
+ assert(Diagnostics && "DiagnosticsEngine needed for creating SourceManager");
+ assert(FileMgr && "FileManager needed for creating SourceManager");
SourceMgr = llvm::makeIntrusiveRefCnt<SourceManager>(getDiagnostics(),
getFileManager());
}
More information about the cfe-commits
mailing list