From 1da7eac87c15662f808f69e260e3ec47923c28f6 Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Tue, 26 Mar 2019 22:18:52 +0000 Subject: [PATCH] Frontend: Remove CompilerInstance::VirtualFileSystem, NFC 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 wanted to customize the VFS. https://reviews.llvm.org/D59377 llvm-svn: 357037 --- clang-tools-extra/clangd/Compiler.cpp | 2 +- clang/include/clang/Basic/FileManager.h | 4 +++ clang/include/clang/Frontend/CompilerInstance.h | 20 +++------------ clang/lib/Frontend/ASTUnit.cpp | 29 ++++++++++------------ clang/lib/Frontend/CompilerInstance.cpp | 21 ++++++---------- .../lib/StaticAnalyzer/Frontend/ModelInjector.cpp | 2 -- .../Clang/ClangExpressionParser.cpp | 3 +-- .../Clang/ClangModulesDeclVendor.cpp | 2 +- 8 files changed, 31 insertions(+), 52 deletions(-) diff --git a/clang-tools-extra/clangd/Compiler.cpp b/clang-tools-extra/clangd/Compiler.cpp index 1eecce9..e31e030 100644 --- a/clang-tools-extra/clangd/Compiler.cpp +++ b/clang-tools-extra/clangd/Compiler.cpp @@ -95,7 +95,7 @@ prepareCompilerInstance(std::unique_ptr CI, 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)); diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h index 6547f8a..b834826 100644 --- a/clang/include/clang/Basic/FileManager.h +++ b/clang/include/clang/Basic/FileManager.h @@ -175,6 +175,10 @@ class FileManager : public RefCountedBase { 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 FS = nullptr); ~FileManager(); diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index 2fc6157..f63ff48 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -82,9 +82,6 @@ class CompilerInstance : public ModuleLoader { /// Auxiliary Target info. IntrusiveRefCntPtr AuxTarget; - /// The virtual file system. - IntrusiveRefCntPtr VirtualFileSystem; - /// The file manager. IntrusiveRefCntPtr FileMgr; @@ -382,20 +379,8 @@ public: /// @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 FS) { - VirtualFileSystem = std::move(FS); + return *getFileManager().getVirtualFileSystem(); } /// } @@ -645,7 +630,8 @@ public: /// 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 VFS = nullptr); /// Create the source manager and replace any existing one with it. void createSourceManager(FileManager &FileMgr); diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index 0b2995b..7bc3010 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -1078,28 +1078,29 @@ bool ASTUnit::Parse(std::shared_ptr PCHContainerOps, 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(*Invocation); if (OverrideMainBuffer) { assert(Preamble && "No preamble was built, but OverrideMainBuffer is not null"); - IntrusiveRefCntPtr 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 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 @@ -1136,10 +1137,6 @@ bool ASTUnit::Parse(std::shared_ptr PCHContainerOps, // Configure the various subsystems. LangOpts = Clang->getInvocation().LangOpts; FileSystemOpts = Clang->getFileSystemOpts(); - if (!FileMgr) { - Clang->createFileManager(); - FileMgr = &Clang->getFileManager(); - } ResetForParse(); diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index e738596..6c82a6e 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -89,10 +89,6 @@ 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) { @@ -297,13 +293,14 @@ CompilerInstance::createDiagnostics(DiagnosticOptions *Opts, // File Manager -FileManager *CompilerInstance::createFileManager() { - if (!hasVirtualFileSystem()) { - IntrusiveRefCntPtr VFS = - createVFSFromCompilerInvocation(getInvocation(), getDiagnostics()); - setVirtualFileSystem(VFS); - } - FileMgr = new FileManager(getFileSystemOpts(), VirtualFileSystem); +FileManager *CompilerInstance::createFileManager( + IntrusiveRefCntPtr 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(); } @@ -1101,8 +1098,6 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, 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()); diff --git a/clang/lib/StaticAnalyzer/Frontend/ModelInjector.cpp b/clang/lib/StaticAnalyzer/Frontend/ModelInjector.cpp index e4bbfba..fe5f590 100644 --- a/clang/lib/StaticAnalyzer/Frontend/ModelInjector.cpp +++ b/clang/lib/StaticAnalyzer/Frontend/ModelInjector.cpp @@ -82,8 +82,6 @@ void ModelInjector::onBodySynthesis(const NamedDecl *D) { 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()); diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index 7589c37..f9ddb75 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -309,8 +309,7 @@ ClangExpressionParser::ClangExpressionParser( } // 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 diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp index 0529492..993843f 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp @@ -679,7 +679,7 @@ ClangModulesDeclVendor::Create(Target &target) { } // 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); -- 2.7.4