Author: Jan Svoboda Date: 2025-10-02T11:01:27-07:00 New Revision: b86ddae1da651f921125a9864b45a5b11bc3b1c0
URL: https://github.com/llvm/llvm-project/commit/b86ddae1da651f921125a9864b45a5b11bc3b1c0 DIFF: https://github.com/llvm/llvm-project/commit/b86ddae1da651f921125a9864b45a5b11bc3b1c0.diff LOG: [clang] NFCI: Clean up `CompilerInstance::create{File,Source}Manager()` (#160748) The `CompilerInstance::createSourceManager()` function currently accepts the `FileManager` to be used. However, all clients call `CompilerInstance::createFileManager()` prior to creating the `SourceManager`, and it never makes sense to use a `FileManager` in the `SourceManager` that's different from the rest of the compiler. Passing the `FileManager` explicitly is redundant, error-prone, and deviates from the style of other `CompilerInstance` initialization APIs. This PR therefore removes the `FileManager` parameter from `createSourceManager()` and also stops returning the `FileManager` pointer from `createFileManager()`, since that was its primary use. Now, `createSourceManager()` internally calls `getFileManager()` instead. Added: Modified: clang-tools-extra/clang-include-fixer/IncludeFixer.cpp clang-tools-extra/include-cleaner/unittests/RecordTest.cpp clang/include/clang/Frontend/CompilerInstance.h clang/lib/ExtractAPI/ExtractAPIConsumer.cpp clang/lib/Frontend/ChainedIncludesSource.cpp clang/lib/Frontend/CompilerInstance.cpp clang/lib/Frontend/FrontendAction.cpp clang/lib/Testing/TestAST.cpp clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp clang/lib/Tooling/Tooling.cpp clang/tools/clang-import-test/clang-import-test.cpp clang/unittests/CodeGen/TestCompiler.h clang/unittests/Serialization/ForceCheckFileInputTest.cpp clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp Removed: ################################################################################ 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 b1fb905e3602f..584436665622d 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 && "DiagnosticsEngine needed for creating SourceManager"); + assert(FileMgr && "FileManager 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 66cf2688e0a13..010380dee3617 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)); 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()) { _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
