Skip to content

Commit 4957c47

Browse files
authored
[clang] Avoid reparsing VFS overlay files for module dep collector (#158372)
This PR uses the new-ish `llvm::vfs::FileSystem::visit()` interface to collect VFS overlay entries from an existing `FileSystem` instance rather than parsing the VFS YAML file anew. This prevents duplicate diagnostics as observed by `clang/test/VFS/broken-vfs-module-dep.c`.
1 parent 985dc69 commit 4957c47

File tree

4 files changed

+10
-32
lines changed

4 files changed

+10
-32
lines changed

clang/lib/Frontend/CompilerInstance.cpp

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -271,19 +271,12 @@ static void collectIncludePCH(CompilerInstance &CI,
271271

272272
static void collectVFSEntries(CompilerInstance &CI,
273273
std::shared_ptr<ModuleDependencyCollector> MDC) {
274-
if (CI.getHeaderSearchOpts().VFSOverlayFiles.empty())
275-
return;
276-
277274
// Collect all VFS found.
278275
SmallVector<llvm::vfs::YAMLVFSEntry, 16> VFSEntries;
279-
for (const std::string &VFSFile : CI.getHeaderSearchOpts().VFSOverlayFiles) {
280-
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer =
281-
llvm::MemoryBuffer::getFile(VFSFile);
282-
if (!Buffer)
283-
return;
284-
llvm::vfs::collectVFSFromYAML(std::move(Buffer.get()),
285-
/*DiagHandler*/ nullptr, VFSFile, VFSEntries);
286-
}
276+
CI.getVirtualFileSystem().visit([&](llvm::vfs::FileSystem &VFS) {
277+
if (auto *RedirectingVFS = dyn_cast<llvm::vfs::RedirectingFileSystem>(&VFS))
278+
llvm::vfs::collectVFSEntries(*RedirectingVFS, VFSEntries);
279+
});
287280

288281
for (auto &E : VFSEntries)
289282
MDC->addFile(E.VPath, E.RPath);

clang/test/VFS/broken-vfs-module-dep.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,5 @@
22
// RUN: mkdir -p %t
33
// RUN: not %clang_cc1 -module-dependency-dir %t -ivfsoverlay %S/Inputs/invalid-yaml.yaml %s 2>&1 | FileCheck %s
44

5-
// CHECK: error: Unexpected token
65
// CHECK: error: Unexpected token
76
// CHECK: 1 error generated

llvm/include/llvm/Support/VirtualFileSystem.h

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,14 +1114,10 @@ class LLVM_ABI RedirectingFileSystem
11141114
};
11151115

11161116
/// Collect all pairs of <virtual path, real path> entries from the
1117-
/// \p YAMLFilePath. This is used by the module dependency collector to forward
1117+
/// \p VFS. This is used by the module dependency collector to forward
11181118
/// the entries into the reproducer output VFS YAML file.
1119-
LLVM_ABI void collectVFSFromYAML(
1120-
std::unique_ptr<llvm::MemoryBuffer> Buffer,
1121-
llvm::SourceMgr::DiagHandlerTy DiagHandler, StringRef YAMLFilePath,
1122-
SmallVectorImpl<YAMLVFSEntry> &CollectedEntries,
1123-
void *DiagContext = nullptr,
1124-
IntrusiveRefCntPtr<FileSystem> ExternalFS = getRealFileSystem());
1119+
void collectVFSEntries(RedirectingFileSystem &VFS,
1120+
SmallVectorImpl<YAMLVFSEntry> &CollectedEntries);
11251121

11261122
class YAMLVFSWriter {
11271123
std::vector<YAMLVFSEntry> Mappings;

llvm/lib/Support/VirtualFileSystem.cpp

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2707,19 +2707,9 @@ static void getVFSEntries(RedirectingFileSystem::Entry *SrcE,
27072707
Entries.push_back(YAMLVFSEntry(VPath.c_str(), FE->getExternalContentsPath()));
27082708
}
27092709

2710-
void vfs::collectVFSFromYAML(std::unique_ptr<MemoryBuffer> Buffer,
2711-
SourceMgr::DiagHandlerTy DiagHandler,
2712-
StringRef YAMLFilePath,
2713-
SmallVectorImpl<YAMLVFSEntry> &CollectedEntries,
2714-
void *DiagContext,
2715-
IntrusiveRefCntPtr<FileSystem> ExternalFS) {
2716-
std::unique_ptr<RedirectingFileSystem> VFS = RedirectingFileSystem::create(
2717-
std::move(Buffer), DiagHandler, YAMLFilePath, DiagContext,
2718-
std::move(ExternalFS));
2719-
if (!VFS)
2720-
return;
2721-
ErrorOr<RedirectingFileSystem::LookupResult> RootResult =
2722-
VFS->lookupPath("/");
2710+
void vfs::collectVFSEntries(RedirectingFileSystem &VFS,
2711+
SmallVectorImpl<YAMLVFSEntry> &CollectedEntries) {
2712+
ErrorOr<RedirectingFileSystem::LookupResult> RootResult = VFS.lookupPath("/");
27232713
if (!RootResult)
27242714
return;
27252715
SmallVector<StringRef, 8> Components;

0 commit comments

Comments
 (0)