[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.

Author: jansvoboda11

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.

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) {

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.

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();
 

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());

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) {

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(

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())

clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp

@@ -415,24 +415,25 @@ 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) {
     DepFS->resetBypassedPathPrefix();
     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

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);