Skip to content

Commit 866879f

Browse files
authored
[clang] Don't silently inherit the VFS from FileManager (#164323)
Since #158381 the `CompilerInstance` is aware of the VFS and co-owns it. To reduce scope of that PR, the VFS was being inherited from the `FileManager` during `setFileManager()` if it wasn't configured before. However, the implementation of that setter was buggy. This PR fixes the bug, and moves us closer to the long-term goal of `CompilerInstance` requiring the VFS to be configured explicitly and owned by the instance.
1 parent 47ea854 commit 866879f

File tree

14 files changed

+21
-11
lines changed

14 files changed

+21
-11
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ bool IncludeFixerActionFactory::runInvocation(
9090

9191
// Set up Clang.
9292
CompilerInstance Compiler(std::move(Invocation), std::move(PCHContainerOps));
93+
Compiler.setVirtualFileSystem(Files->getVirtualFileSystemPtr());
9394
Compiler.setFileManager(Files);
9495

9596
// Create the compiler's actual diagnostics engine. We want to drop all

clang/include/clang/Frontend/ASTUnit.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,11 @@ class ASTUnit {
499499
return *PPOpts;
500500
}
501501

502+
IntrusiveRefCntPtr<llvm::vfs::FileSystem> getVirtualFileSystemPtr() {
503+
// FIXME: Don't defer VFS ownership to the FileManager.
504+
return FileMgr->getVirtualFileSystemPtr();
505+
}
506+
502507
const FileManager &getFileManager() const { return *FileMgr; }
503508
FileManager &getFileManager() { return *FileMgr; }
504509
IntrusiveRefCntPtr<FileManager> getFileManagerPtr() { return FileMgr; }

clang/include/clang/Frontend/CompilerInstance.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ class CompilerInstance : public ModuleLoader {
460460
FileMgr.resetWithoutRelease();
461461
}
462462

463-
/// Replace the current file manager and virtual file system.
463+
/// Replace the current file manager.
464464
void setFileManager(IntrusiveRefCntPtr<FileManager> Value);
465465

466466
/// @}

clang/lib/Frontend/ASTUnit.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1651,6 +1651,7 @@ ASTUnit *ASTUnit::LoadFromCompilerInvocationAction(
16511651
AST->Reader = nullptr;
16521652

16531653
// Create a file manager object to provide access to and cache the filesystem.
1654+
Clang->setVirtualFileSystem(AST->getVirtualFileSystemPtr());
16541655
Clang->setFileManager(AST->getFileManagerPtr());
16551656

16561657
// Create the source manager.
@@ -2290,6 +2291,7 @@ void ASTUnit::CodeComplete(
22902291
"IR inputs not support here!");
22912292

22922293
// Use the source and file managers that we were given.
2294+
Clang->setVirtualFileSystem(FileMgr->getVirtualFileSystemPtr());
22932295
Clang->setFileManager(FileMgr);
22942296
Clang->setSourceManager(SourceMgr);
22952297

clang/lib/Frontend/CompilerInstance.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,6 @@ bool CompilerInstance::createTarget() {
160160
}
161161

162162
void CompilerInstance::setFileManager(IntrusiveRefCntPtr<FileManager> Value) {
163-
if (!hasVirtualFileSystem())
164-
setVirtualFileSystem(Value->getVirtualFileSystemPtr());
165163
assert(Value == nullptr ||
166164
getVirtualFileSystemPtr() == Value->getVirtualFileSystemPtr());
167165
FileMgr = std::move(Value);

clang/lib/Frontend/FrontendAction.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -945,6 +945,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
945945

946946
// Set the shared objects, these are reset when we finish processing the
947947
// file, otherwise the CompilerInstance will happily destroy them.
948+
CI.setVirtualFileSystem(AST->getVirtualFileSystemPtr());
948949
CI.setFileManager(AST->getFileManagerPtr());
949950
CI.setSourceManager(AST->getSourceManagerPtr());
950951
CI.setPreprocessor(AST->getPreprocessorPtr());

clang/lib/Frontend/PrecompiledPreamble.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -479,16 +479,12 @@ llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
479479
Diagnostics->Reset();
480480
ProcessWarningOptions(*Diagnostics, Clang->getDiagnosticOpts(), *VFS);
481481

482-
VFS = createVFSFromCompilerInvocation(Clang->getInvocation(), *Diagnostics,
483-
VFS);
484-
485482
// Create a file manager object to provide access to and cache the filesystem.
486-
Clang->setFileManager(
487-
llvm::makeIntrusiveRefCnt<FileManager>(Clang->getFileSystemOpts(), VFS));
483+
Clang->createVirtualFileSystem(VFS);
484+
Clang->createFileManager();
488485

489486
// Create the source manager.
490-
Clang->setSourceManager(llvm::makeIntrusiveRefCnt<SourceManager>(
491-
*Diagnostics, Clang->getFileManager()));
487+
Clang->createSourceManager();
492488

493489
auto PreambleDepCollector = std::make_shared<PreambleDependencyCollector>();
494490
Clang->addDependencyCollector(PreambleDepCollector);

clang/lib/StaticAnalyzer/Frontend/ModelInjector.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ void ModelInjector::onBodySynthesis(const NamedDecl *D) {
9393

9494
// The instance wants to take ownership, however DisableFree frontend option
9595
// is set to true to avoid double free issues
96+
Instance.setVirtualFileSystem(CI.getVirtualFileSystemPtr());
9697
Instance.setFileManager(CI.getFileManagerPtr());
9798
Instance.setSourceManager(SM);
9899
Instance.setPreprocessor(CI.getPreprocessorPtr());

clang/lib/Tooling/Tooling.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,7 @@ bool FrontendActionFactory::runInvocation(
446446
DiagnosticConsumer *DiagConsumer) {
447447
// Create a compiler instance to handle the actual work.
448448
CompilerInstance Compiler(std::move(Invocation), std::move(PCHContainerOps));
449+
Compiler.setVirtualFileSystem(Files->getVirtualFileSystemPtr());
449450
Compiler.setFileManager(Files);
450451

451452
// The FrontendAction can have lifetime requirements for Compiler or its

clang/tools/clang-installapi/ClangInstallAPI.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ static bool run(ArrayRef<const char *> Args, const char *ProgName) {
114114

115115
// Set up compilation.
116116
std::unique_ptr<CompilerInstance> CI(new CompilerInstance());
117+
CI->setVirtualFileSystem(FM->getVirtualFileSystemPtr());
117118
CI->setFileManager(FM);
118119
CI->createDiagnostics();
119120
if (!CI->hasDiagnostics())

0 commit comments

Comments
 (0)