Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

[dexonsmith]

Summary

Remove CompilerInstance::VirtualFileSystem and
CompilerInstance::setVirtualFileSystem, instead relying on the VFS in
the FileManager. CompilerInstance and its clients already went to some
trouble to make these match. Now they are guaranteed to match.

As part of this, I added a VFS parameter (defaults to nullptr) to
CompilerInstance::createFileManager to avoid repeating construction
logic in clients that just want to customize the VFS.

[dexonsmith]

I slightly simplified the code for CompilerInstance::createFileManager (replaced with an assert), since I noticed that FileManager always constructs its own VFS if it isn't passed one.

[jkorous]

In D59377#1430002, @dexonsmith wrote:

... since I noticed that FileManager ...

This kind of implies that we should move the comment from FileManager constructor implementation to the header thus making it an explicit part of the interface contract.

// If the caller doesn't provide a virtual file system, just grab the real
// file system.

Maybe https://reviews.llvm.org/D59388 would be the right place to do it.

[dexonsmith]

Document the default VFS used by the FileManager.

[dexonsmith]

In D59377#1431203, @jkorous wrote:

In D59377#1430002, @dexonsmith wrote:

... since I noticed that FileManager ...

This kind of implies that we should move the comment from FileManager constructor implementation to the header thus making it an explicit part of the interface contract.

// If the caller doesn't provide a virtual file system, just grab the real
// file system.

Maybe https://reviews.llvm.org/D59388 would be the right place to do it.

Good call; since I'm relying on it in this patch I've updated this patch to document it.

[jkorous]

LGTM

[dexonsmith]

Committed r357037.

clang-tools-extra/clangd/Compiler.cpp

prepareCompilerInstance(std::unique_ptr<clang::CompilerInvocation> CI,
 
   auto Clang = llvm::make_unique<CompilerInstance>(PCHs);
   Clang->setInvocation(std::move(CI));
   Clang->createDiagnostics(&DiagsClient, false);
 
   if (auto VFSWithRemapping = createVFSFromCompilerInvocation(
           Clang->getInvocation(), Clang->getDiagnostics(), VFS))
     VFS = VFSWithRemapping;
-  Clang->setVirtualFileSystem(VFS);
+  Clang->createFileManager(VFS);
 
   Clang->setTarget(TargetInfo::CreateTargetInfo(
       Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
   if (!Clang->hasTarget())
     return nullptr;
 
   // RemappedFileBuffers will handle the lifetime of the Buffer pointer,
   // release it.
   Buffer.release();
   return Clang;
 }
 
 } // namespace clangd
 } // namespace clang

clang/include/clang/Basic/FileManager.h

class FileManager : public RefCountedBase<FileManager> {
   /// Add all ancestors of the given path (pointing to either a file
   /// or a directory) as virtual directories.
   void addAncestorsAsVirtualDirs(StringRef Path);
 
   /// Fills the RealPathName in file entry.
   void fillRealPathName(FileEntry *UFE, llvm::StringRef FileName);
 
 public:
+  /// Construct a file manager, optionally with a custom VFS.
+  ///
+  /// \param FS if non-null, the VFS to use.  Otherwise uses
+  /// llvm::vfs::getRealFileSystem().
   FileManager(const FileSystemOptions &FileSystemOpts,
               IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = nullptr);
   ~FileManager();
 
   /// Installs the provided FileSystemStatCache object within
   /// the FileManager.
   ///
   /// Ownership of this object is transferred to the FileManager.

clang/include/clang/Frontend/CompilerInstance.h

class CompilerInstance : public ModuleLoader {
   IntrusiveRefCntPtr<DiagnosticsEngine> Diagnostics;
 
   /// The target being compiled for.
   IntrusiveRefCntPtr<TargetInfo> Target;
 
   /// Auxiliary Target info.
   IntrusiveRefCntPtr<TargetInfo> AuxTarget;
 
-  /// The virtual file system.
-  IntrusiveRefCntPtr<llvm::vfs::FileSystem> VirtualFileSystem;
-
   /// The file manager.
   IntrusiveRefCntPtr<FileManager> FileMgr;
 
   /// The source manager.
   IntrusiveRefCntPtr<SourceManager> SourceMgr;
 
   /// The cache of PCM files.
   IntrusiveRefCntPtr<InMemoryModuleCache> ModuleCache;
public:
 
   /// Replace the current AuxTarget.
   void setAuxTarget(TargetInfo *Value);
 
   /// }
   /// @name Virtual File System
   /// {
 
-  bool hasVirtualFileSystem() const { return VirtualFileSystem != nullptr; }
-
   llvm::vfs::FileSystem &getVirtualFileSystem() const {
-    assert(hasVirtualFileSystem() &&
-           "Compiler instance has no virtual file system");
-    return *VirtualFileSystem;
-  }
-
-  /// Replace the current virtual file system.
-  ///
-  /// \note Most clients should use setFileManager, which will implicitly reset
-  /// the virtual file system to the one contained in the file manager.
-  void setVirtualFileSystem(IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS) {
-    VirtualFileSystem = std::move(FS);
+    return *getFileManager().getVirtualFileSystem();
   }
 
   /// }
   /// @name File Manager
   /// {
 
   bool hasFileManager() const { return FileMgr != nullptr; }
 
public:
   createDiagnostics(DiagnosticOptions *Opts,
                     DiagnosticConsumer *Client = nullptr,
                     bool ShouldOwnClient = true,
                     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();
+  FileManager *
+  createFileManager(IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr);
 
   /// Create the source manager and replace any existing one with it.
   void createSourceManager(FileManager &FileMgr);
 
   /// Create the preprocessor, using the invocation, file, and source managers,
   /// and replace any existing one with it.
   void createPreprocessor(TranslationUnitKind TUKind);
 

clang/lib/Frontend/ASTUnit.cpp

 /// \returns True if a failure occurred that causes the ASTUnit not to
 /// contain any translation-unit information, false otherwise.
 bool ASTUnit::Parse(std::shared_ptr<PCHContainerOperations> PCHContainerOps,
                     std::unique_ptr<llvm::MemoryBuffer> OverrideMainBuffer,
                     IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
   if (!Invocation)
     return true;
 
+  if (VFS && FileMgr)
+    assert(VFS == FileMgr->getVirtualFileSystem() &&
+           "VFS passed to Parse and VFS in FileMgr are different");
+
   auto CCInvocation = std::make_shared<CompilerInvocation>(*Invocation);
   if (OverrideMainBuffer) {
     assert(Preamble &&
            "No preamble was built, but OverrideMainBuffer is not null");
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem> OldVFS = VFS;
     Preamble->AddImplicitPreamble(*CCInvocation, VFS, OverrideMainBuffer.get());
-    if (OldVFS != VFS && FileMgr) {
-      assert(OldVFS == FileMgr->getVirtualFileSystem() &&
-             "VFS passed to Parse and VFS in FileMgr are different");
-      FileMgr = new FileManager(FileMgr->getFileSystemOpts(), VFS);
-    }
+    // VFS may have changed...
   }
 
   // Create the compiler instance to use for building the AST.
   std::unique_ptr<CompilerInstance> Clang(
       new CompilerInstance(std::move(PCHContainerOps)));
-  if (FileMgr && VFS) {
-    assert(VFS == FileMgr->getVirtualFileSystem() &&
-           "VFS passed to Parse and VFS in FileMgr are different");
-  } else if (VFS) {
-    Clang->setVirtualFileSystem(VFS);
-  }
+
+  // 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)
+    Clang->setFileManager(&*FileMgr);
+  else
+    FileMgr = Clang->createFileManager(std::move(VFS));
 
   // Recover resources if we crash before exiting this method.
   llvm::CrashRecoveryContextCleanupRegistrar<CompilerInstance>
     CICleanup(Clang.get());
 
   Clang->setInvocation(CCInvocation);
   OriginalSourceFile = Clang->getFrontendOpts().Inputs[0].getFile();
 
assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
          "FIXME: AST inputs not yet supported here!");
   assert(Clang->getFrontendOpts().Inputs[0].getKind().getLanguage() !=
              InputKind::LLVM_IR &&
          "IR inputs not support here!");
 
   // Configure the various subsystems.
   LangOpts = Clang->getInvocation().LangOpts;
   FileSystemOpts = Clang->getFileSystemOpts();
-  if (!FileMgr) {
-    Clang->createFileManager();
-    FileMgr = &Clang->getFileManager();
-  }
 
   ResetForParse();
 
   SourceMgr = new SourceManager(getDiagnostics(), *FileMgr,
                                 UserFilesAreVolatile);
   if (!OverrideMainBuffer) {
     checkAndRemoveNonDriverDiags(StoredDiagnostics);
     TopLevelDeclsInPreamble.clear();

clang/lib/Frontend/CompilerInstance.cpp

void CompilerInstance::setDiagnostics(DiagnosticsEngine *Value) {
   Diagnostics = Value;
 }
 
 void CompilerInstance::setTarget(TargetInfo *Value) { Target = Value; }
 void CompilerInstance::setAuxTarget(TargetInfo *Value) { AuxTarget = Value; }
 
 void CompilerInstance::setFileManager(FileManager *Value) {
   FileMgr = Value;
-  if (Value)
-    VirtualFileSystem = Value->getVirtualFileSystem();
-  else
-    VirtualFileSystem.reset();
 }
 
 void CompilerInstance::setSourceManager(SourceManager *Value) {
   SourceMgr = Value;
 }
 
 void CompilerInstance::setPreprocessor(std::shared_ptr<Preprocessor> Value) {
   PP = std::move(Value);
CompilerInstance::createDiagnostics(DiagnosticOptions *Opts,
   // Configure our handling of diagnostics.
   ProcessWarningOptions(*Diags, *Opts);
 
   return Diags;
 }
 
 // File Manager
 
-FileManager *CompilerInstance::createFileManager() {
-  if (!hasVirtualFileSystem()) {
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS =
-        createVFSFromCompilerInvocation(getInvocation(), getDiagnostics());
-    setVirtualFileSystem(VFS);
-  }
-  FileMgr = new FileManager(getFileSystemOpts(), VirtualFileSystem);
+FileManager *CompilerInstance::createFileManager(
+    IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
+  if (!VFS)
+    VFS = FileMgr ? FileMgr->getVirtualFileSystem()
+                  : createVFSFromCompilerInvocation(getInvocation(),
+                                                    getDiagnostics());
+  assert(VFS && "FileManager has no VFS?");
+  FileMgr = new FileManager(getFileSystemOpts(), std::move(VFS));
   return FileMgr.get();
 }
 
 // Source Manager
 
 void CompilerInstance::createSourceManager(FileManager &FileMgr) {
   SourceMgr = new SourceManager(getDiagnostics(), FileMgr);
 }
CompilerInstance Instance(ImportingInstance.getPCHContainerOperations(),
                             &ImportingInstance.getModuleCache());
   auto &Inv = *Invocation;
   Instance.setInvocation(std::move(Invocation));
 
   Instance.createDiagnostics(new ForwardingDiagnosticConsumer(
                                    ImportingInstance.getDiagnosticClient()),
                              /*ShouldOwnClient=*/true);
 
-  Instance.setVirtualFileSystem(&ImportingInstance.getVirtualFileSystem());
-
   // Note that this module is part of the module build stack, so that we
   // can detect cycles in the module graph.
   Instance.setFileManager(&ImportingInstance.getFileManager());
   Instance.createSourceManager(Instance.getFileManager());
   SourceManager &SourceMgr = Instance.getSourceManager();
   SourceMgr.setModuleBuildStack(
     ImportingInstance.getSourceManager().getModuleBuildStack());
   SourceMgr.pushModuleBuildStack(ModuleName,

clang/lib/StaticAnalyzer/Frontend/ModelInjector.cpp

void ModelInjector::onBodySynthesis(const NamedDecl *D) {
   CompilerInstance Instance(CI.getPCHContainerOperations());
   Instance.setInvocation(std::move(Invocation));
   Instance.createDiagnostics(
       new ForwardingDiagnosticConsumer(CI.getDiagnosticClient()),
       /*ShouldOwnClient=*/true);
 
   Instance.getDiagnostics().setSourceManager(&SM);
 
-  Instance.setVirtualFileSystem(&CI.getVirtualFileSystem());
-
   // The instance wants to take ownership, however DisableFree frontend option
   // is set to true to avoid double free issues
   Instance.setFileManager(&CI.getFileManager());
   Instance.setSourceManager(&SM);
   Instance.setPreprocessor(CI.getPreprocessorPtr());
   Instance.setASTContext(&CI.getASTContext());
 
   Instance.getPreprocessor().InitializeForModelFile();

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

m_compiler->setModuleDepCollector(
         std::make_shared<ModuleDependencyCollectorAdaptor>(
             fp.GetFileCollector()));
     DependencyOutputOptions &opts = m_compiler->getDependencyOutputOpts();
     opts.IncludeSystemHeaders = true;
     opts.IncludeModuleFiles = true;
   }
 
   // Make sure clang uses the same VFS as LLDB.
-  m_compiler->setVirtualFileSystem(
-      FileSystem::Instance().GetVirtualFileSystem());
+  m_compiler->createFileManager(FileSystem::Instance().GetVirtualFileSystem());
 
   lldb::LanguageType frame_lang =
       expr.Language(); // defaults to lldb::eLanguageTypeUnknown
   bool overridden_target_opts = false;
   lldb_private::LanguageRuntime *lang_rt = nullptr;
 
   std::string abi;
   ArchSpec target_arch;

lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

instance->setModuleDepCollector(
         std::make_shared<ModuleDependencyCollectorAdaptor>(
             fp.GetFileCollector()));
     clang::DependencyOutputOptions &opts = instance->getDependencyOutputOpts();
     opts.IncludeSystemHeaders = true;
     opts.IncludeModuleFiles = true;
   }
 
   // Make sure clang uses the same VFS as LLDB.
-  instance->setVirtualFileSystem(FileSystem::Instance().GetVirtualFileSystem());
+  instance->createFileManager(FileSystem::Instance().GetVirtualFileSystem());
   instance->setDiagnostics(diagnostics_engine.get());
   instance->setInvocation(invocation);
 
   std::unique_ptr<clang::FrontendAction> action(new clang::SyntaxOnlyAction);
 
   instance->setTarget(clang::TargetInfo::CreateTargetInfo(
       *diagnostics_engine, instance->getInvocation().TargetOpts));