[clang] Initialize the file system explicitly (llvm#158381)

This PR is a part of the effort to make the VFS used in the compiler
more explicit and consistent.

Instead of creating the VFS deep within the compiler (in
`CompilerInstance::createFileManager()`), clients are now required to
explicitly call `CompilerInstance::createVirtualFileSystem()` and
provide the base VFS from the outside.

This PR also helps in breaking up the dependency cycle where creating a
properly configured `DiagnosticsEngine` requires a properly configured
VFS, but creating properly configuring a VFS requires the
`DiagnosticsEngine`.

Both `CompilerInstance::create{FileManager,Diagnostics}()` now just use
the VFS already in `CompilerInstance` instead of taking one as a
parameter, making the VFS consistent across the instance sub-object.

Author: jansvoboda11

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

@@ -94,8 +94,7 @@ bool IncludeFixerActionFactory::runInvocation(
 
   // Create the compiler's actual diagnostics engine. We want to drop all
   // diagnostics here.
-  Compiler.createDiagnostics(Files->getVirtualFileSystem(),
-                             new clang::IgnoringDiagConsumer,
+  Compiler.createDiagnostics(new clang::IgnoringDiagConsumer,
                              /*ShouldOwnClient=*/true);
   Compiler.createSourceManager(*Files);
 

clang-tools-extra/clangd/Compiler.cpp

@@ -147,13 +147,9 @@ prepareCompilerInstance(std::unique_ptr<clang::CompilerInvocation> CI,
   }
 
   auto Clang = std::make_unique<CompilerInstance>(std::move(CI));
-  Clang->createDiagnostics(*VFS, &DiagsClient, false);
-
-  if (auto VFSWithRemapping = createVFSFromCompilerInvocation(
-          Clang->getInvocation(), Clang->getDiagnostics(), VFS))
-    VFS = VFSWithRemapping;
-  Clang->createFileManager(VFS);
-
+  Clang->createVirtualFileSystem(VFS, &DiagsClient);
+  Clang->createDiagnostics(&DiagsClient, false);
+  Clang->createFileManager();
   if (!Clang->createTarget())
     return nullptr;
 

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

@@ -646,9 +646,10 @@ TEST_F(PragmaIncludeTest, ExportInUnnamedBuffer) {
                                                  *Diags, "clang"));
 
   auto Clang = std::make_unique<CompilerInstance>(std::move(Invocation));
-  Clang->createDiagnostics(*VFS);
+  Clang->createVirtualFileSystem(VFS);
+  Clang->createDiagnostics();
 
-  auto *FM = Clang->createFileManager(VFS);
+  auto *FM = Clang->createFileManager();
   ASSERT_TRUE(Clang->ExecuteAction(*Inputs.MakeAction()));
   EXPECT_THAT(
       PI.getExporters(llvm::cantFail(FM->getFileRef("foo.h")), *FM),

clang/include/clang/Frontend/CompilerInstance.h

@@ -83,6 +83,9 @@ class CompilerInstance : public ModuleLoader {
   /// The options used in this compiler instance.
   std::shared_ptr<CompilerInvocation> Invocation;
 
+  /// The virtual file system instance.
+  IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS;
+
   /// The diagnostics engine instance.
   IntrusiveRefCntPtr<DiagnosticsEngine> Diagnostics;
 
@@ -409,9 +412,31 @@ class CompilerInstance : public ModuleLoader {
   /// @name Virtual File System
   /// @{
 
-  llvm::vfs::FileSystem &getVirtualFileSystem() const;
-  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-  getVirtualFileSystemPtr() const;
+  bool hasVirtualFileSystem() const { return VFS != nullptr; }
+
+  /// Create a virtual file system instance based on the invocation.
+  ///
+  /// @param BaseFS The file system that may be used when configuring the final
+  ///               file system, and act as the underlying file system. Must not
+  ///               be NULL.
+  /// @param DC If non-NULL, the diagnostic consumer to be used in case
+  ///           configuring the file system emits diagnostics. Note that the
+  ///           DiagnosticsEngine using the consumer won't obey the
+  ///           --warning-suppression-mappings= flag.
+  void createVirtualFileSystem(IntrusiveRefCntPtr<llvm::vfs::FileSystem>
+                                   BaseFS = llvm::vfs::getRealFileSystem(),
+                               DiagnosticConsumer *DC = nullptr);
+
+  /// Use the given file system.
+  void setVirtualFileSystem(IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS) {
+    VFS = std::move(FS);
+  }
+
+  llvm::vfs::FileSystem &getVirtualFileSystem() const { return *VFS; }
+
+  IntrusiveRefCntPtr<llvm::vfs::FileSystem> getVirtualFileSystemPtr() const {
+    return VFS;
+  }
 
   /// @}
   /// @name File Manager
@@ -650,32 +675,31 @@ class CompilerInstance : public ModuleLoader {
   /// Note that this routine also replaces the diagnostic client,
   /// allocating one if one is not provided.
   ///
-  /// \param VFS is used for any IO needed when creating DiagnosticsEngine. It
-  /// doesn't replace VFS in the CompilerInstance (if any).
-  ///
   /// \param Client If non-NULL, a diagnostic client that will be
   /// attached to (and, then, owned by) the DiagnosticsEngine inside this AST
   /// unit.
   ///
   /// \param ShouldOwnClient If Client is non-NULL, specifies whether
   /// the diagnostic object should take ownership of the client.
-  void createDiagnostics(llvm::vfs::FileSystem &VFS,
-                         DiagnosticConsumer *Client = nullptr,
+  void createDiagnostics(DiagnosticConsumer *Client = nullptr,
                          bool ShouldOwnClient = true);
 
-  /// Create a DiagnosticsEngine object with a the TextDiagnosticPrinter.
+  /// Create a DiagnosticsEngine object.
   ///
   /// If no diagnostic client is provided, this creates a
   /// DiagnosticConsumer that is owned by the returned diagnostic
   /// object, if using directly the caller is responsible for
   /// releasing the returned DiagnosticsEngine's client eventually.
   ///
+  /// \param VFS The file system used to load the suppression mappings file.
+  ///
   /// \param Opts - The diagnostic options; note that the created text
   /// diagnostic object contains a reference to these options.
   ///
   /// \param Client If non-NULL, a diagnostic client that will be
   /// attached to (and, then, owned by) the returned DiagnosticsEngine
-  /// object.
+  /// object. If NULL, the returned DiagnosticsEngine will own a newly-created
+  /// client.
   ///
   /// \param CodeGenOpts If non-NULL, the code gen options in use, which may be
   /// used by some diagnostics printers (for logging purposes only).
@@ -690,8 +714,7 @@ class CompilerInstance : public ModuleLoader {
   /// Create the file manager and replace any existing one with it.
   ///
   /// \return The new file manager on success, or null on failure.
-  FileManager *
-  createFileManager(IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr);
+  FileManager *createFileManager();
 
   /// Create the source manager and replace any existing one with it.
   void createSourceManager(FileManager &FileMgr);

clang/lib/Frontend/ASTUnit.cpp

@@ -1189,10 +1189,12 @@ bool ASTUnit::Parse(std::shared_ptr<PCHContainerOperations> PCHContainerOps,
   // Ensure that Clang has a FileManager with the right VFS, which may have
   // changed above in AddImplicitPreamble.  If VFS is nullptr, rely on
   // createFileManager to create one.
-  if (VFS && FileMgr && &FileMgr->getVirtualFileSystem() == VFS)
+  if (VFS && FileMgr && &FileMgr->getVirtualFileSystem() == VFS) {
+    Clang->setVirtualFileSystem(std::move(VFS));
     Clang->setFileManager(FileMgr);
-  else {
-    Clang->createFileManager(std::move(VFS));
+  } else {
+    Clang->setVirtualFileSystem(std::move(VFS));
+    Clang->createFileManager();
     FileMgr = Clang->getFileManagerPtr();
   }
 

clang/lib/Frontend/ChainedIncludesSource.cpp

@@ -124,6 +124,7 @@ clang::createChainedIncludesSource(CompilerInstance &CI,
 
     auto Clang = std::make_unique<CompilerInstance>(
         std::move(CInvok), CI.getPCHContainerOperations());
+    Clang->createVirtualFileSystem();
     Clang->setDiagnostics(Diags);
     Clang->setTarget(TargetInfo::CreateTargetInfo(
         Clang->getDiagnostics(), Clang->getInvocation().getTargetOpts()));

clang/lib/Frontend/CompilerInstance.cpp

@@ -159,17 +159,11 @@ bool CompilerInstance::createTarget() {
   return true;
 }
 
-llvm::vfs::FileSystem &CompilerInstance::getVirtualFileSystem() const {
-  return getFileManager().getVirtualFileSystem();
-}
-
-llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-CompilerInstance::getVirtualFileSystemPtr() const {
-  return getFileManager().getVirtualFileSystemPtr();
-}
-
-void CompilerInstance::setFileManager(
-    llvm::IntrusiveRefCntPtr<FileManager> Value) {
+void CompilerInstance::setFileManager(IntrusiveRefCntPtr<FileManager> Value) {
+  if (!hasVirtualFileSystem())
+    setVirtualFileSystem(Value->getVirtualFileSystemPtr());
+  assert(Value == nullptr ||
+         getVirtualFileSystemPtr() == Value->getVirtualFileSystemPtr());
   FileMgr = std::move(Value);
 }
 
@@ -282,6 +276,20 @@ static void collectVFSEntries(CompilerInstance &CI,
     MDC->addFile(E.VPath, E.RPath);
 }
 
+void CompilerInstance::createVirtualFileSystem(
+    IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS, DiagnosticConsumer *DC) {
+  DiagnosticOptions DiagOpts;
+  DiagnosticsEngine Diags(DiagnosticIDs::create(), DiagOpts, DC,
+                          /*ShouldOwnClient=*/false);
+
+  VFS = createVFSFromCompilerInvocation(getInvocation(), Diags,
+                                        std::move(BaseFS));
+  // FIXME: Should this go into createVFSFromCompilerInvocation?
+  if (getFrontendOpts().ShowStats)
+    VFS =
+        llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(std::move(VFS));
+}
+
 // Diagnostics
 static void SetUpDiagnosticLog(DiagnosticOptions &DiagOpts,
                                const CodeGenOptions *CodeGenOpts,
@@ -333,11 +341,10 @@ static void SetupSerializedDiagnostics(DiagnosticOptions &DiagOpts,
   }
 }
 
-void CompilerInstance::createDiagnostics(llvm::vfs::FileSystem &VFS,
-                                         DiagnosticConsumer *Client,
+void CompilerInstance::createDiagnostics(DiagnosticConsumer *Client,
                                          bool ShouldOwnClient) {
-  Diagnostics = createDiagnostics(VFS, getDiagnosticOpts(), Client,
-                                  ShouldOwnClient, &getCodeGenOpts());
+  Diagnostics = createDiagnostics(getVirtualFileSystem(), getDiagnosticOpts(),
+                                  Client, ShouldOwnClient, &getCodeGenOpts());
 }
 
 IntrusiveRefCntPtr<DiagnosticsEngine> CompilerInstance::createDiagnostics(
@@ -375,18 +382,9 @@ IntrusiveRefCntPtr<DiagnosticsEngine> CompilerInstance::createDiagnostics(
 
 // File Manager
 
-FileManager *CompilerInstance::createFileManager(
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
-  if (!VFS)
-    VFS = FileMgr ? FileMgr->getVirtualFileSystemPtr()
-                  : createVFSFromCompilerInvocation(getInvocation(),
-                                                    getDiagnostics());
-  assert(VFS && "FileManager has no VFS?");
-  if (getFrontendOpts().ShowStats)
-    VFS =
-        llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(std::move(VFS));
-  FileMgr = llvm::makeIntrusiveRefCnt<FileManager>(getFileSystemOpts(),
-                                                   std::move(VFS));
+FileManager *CompilerInstance::createFileManager() {
+  assert(VFS && "CompilerInstance needs a VFS for creating FileManager");
+  FileMgr = llvm::makeIntrusiveRefCnt<FileManager>(getFileSystemOpts(), VFS);
   return FileMgr.get();
 }
 
@@ -1167,20 +1165,21 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompileImpl(
   auto &Inv = Instance.getInvocation();
 
   if (ThreadSafeConfig) {
-    Instance.createFileManager(ThreadSafeConfig->getVFS());
+    Instance.setVirtualFileSystem(ThreadSafeConfig->getVFS());
+    Instance.createFileManager();
   } else if (FrontendOpts.ModulesShareFileManager) {
+    Instance.setVirtualFileSystem(getVirtualFileSystemPtr());
     Instance.setFileManager(getFileManagerPtr());
   } else {
-    Instance.createFileManager(getVirtualFileSystemPtr());
+    Instance.setVirtualFileSystem(getVirtualFileSystemPtr());
+    Instance.createFileManager();
   }
 
   if (ThreadSafeConfig) {
-    Instance.createDiagnostics(Instance.getVirtualFileSystem(),
-                               &ThreadSafeConfig->getDiagConsumer(),
+    Instance.createDiagnostics(&ThreadSafeConfig->getDiagConsumer(),
                                /*ShouldOwnClient=*/false);
   } else {
     Instance.createDiagnostics(
-        Instance.getVirtualFileSystem(),
         new ForwardingDiagnosticConsumer(getDiagnosticClient()),
         /*ShouldOwnClient=*/true);
   }

clang/lib/Frontend/FrontendAction.cpp

@@ -876,6 +876,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
 
     // Set the shared objects, these are reset when we finish processing the
     // file, otherwise the CompilerInstance will happily destroy them.
+    CI.setVirtualFileSystem(AST->getFileManager().getVirtualFileSystemPtr());
     CI.setFileManager(AST->getFileManagerPtr());
     CI.createSourceManager(CI.getFileManager());
     CI.getSourceManager().initializeForReplay(AST->getSourceManager());
@@ -966,7 +967,9 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
     return true;
   }
 
-  // Set up the file and source managers, if needed.
+  // Set up the file system, file and source managers, if needed.
+  if (!CI.hasVirtualFileSystem())
+    CI.createVirtualFileSystem();
   if (!CI.hasFileManager()) {
     if (!CI.createFileManager()) {
       return false;

clang/lib/Frontend/Rewrite/FrontendActions.cpp

@@ -245,8 +245,8 @@ class RewriteIncludesAction::RewriteImportsListener : public ASTReaderListener {
     CompilerInstance Instance(
         std::make_shared<CompilerInvocation>(CI.getInvocation()),
         CI.getPCHContainerOperations(), &CI.getModuleCache());
+    Instance.setVirtualFileSystem(CI.getVirtualFileSystemPtr());
     Instance.createDiagnostics(
-        CI.getVirtualFileSystem(),
         new ForwardingDiagnosticConsumer(CI.getDiagnosticClient()),
         /*ShouldOwnClient=*/true);
     Instance.getFrontendOpts().DisableFree = false;

clang/lib/Interpreter/Interpreter.cpp

@@ -107,8 +107,10 @@ CreateCI(const llvm::opt::ArgStringList &Argv) {
     Clang->getHeaderSearchOpts().ResourceDir =
         CompilerInvocation::GetResourcesPath(Argv[0], nullptr);
 
+  Clang->createVirtualFileSystem();
+
   // Create the actual diagnostics engine.
-  Clang->createDiagnostics(*llvm::vfs::getRealFileSystem());
+  Clang->createDiagnostics();
   if (!Clang->hasDiagnostics())
     return llvm::createStringError(llvm::errc::not_supported,
                                    "Initialization failed. "
@@ -475,7 +477,8 @@ Interpreter::createWithCUDA(std::unique_ptr<CompilerInstance> CI,
       std::make_unique<llvm::vfs::OverlayFileSystem>(
           llvm::vfs::getRealFileSystem());
   OverlayVFS->pushOverlay(IMVFS);
-  CI->createFileManager(OverlayVFS);
+  CI->createVirtualFileSystem(OverlayVFS);
+  CI->createFileManager();
 
   llvm::Expected<std::unique_ptr<Interpreter>> InterpOrErr =
       Interpreter::create(std::move(CI));