Skip to content

Commit 47a6e47

Browse files
committed
[clang] Avoid reparsing VFS overlay files for module dep collector (llvm#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`. (cherry picked from commit 4957c47)
1 parent 5120a13 commit 47a6e47

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
@@ -276,19 +276,12 @@ static void collectIncludePCH(CompilerInstance &CI,
276276

277277
static void collectVFSEntries(CompilerInstance &CI,
278278
std::shared_ptr<ModuleDependencyCollector> MDC) {
279-
if (CI.getHeaderSearchOpts().VFSOverlayFiles.empty())
280-
return;
281-
282279
// Collect all VFS found.
283280
SmallVector<llvm::vfs::YAMLVFSEntry, 16> VFSEntries;
284-
for (const std::string &VFSFile : CI.getHeaderSearchOpts().VFSOverlayFiles) {
285-
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer =
286-
llvm::MemoryBuffer::getFile(VFSFile);
287-
if (!Buffer)
288-
return;
289-
llvm::vfs::collectVFSFromYAML(std::move(Buffer.get()),
290-
/*DiagHandler*/ nullptr, VFSFile, VFSEntries);
291-
}
281+
CI.getVirtualFileSystem().visit([&](llvm::vfs::FileSystem &VFS) {
282+
if (auto *RedirectingVFS = dyn_cast<llvm::vfs::RedirectingFileSystem>(&VFS))
283+
llvm::vfs::collectVFSEntries(*RedirectingVFS, VFSEntries);
284+
});
292285

293286
for (auto &E : VFSEntries)
294287
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
@@ -1178,14 +1178,10 @@ class LLVM_ABI RedirectingFileSystem
11781178
};
11791179

11801180
/// Collect all pairs of <virtual path, real path> entries from the
1181-
/// \p YAMLFilePath. This is used by the module dependency collector to forward
1181+
/// \p VFS. This is used by the module dependency collector to forward
11821182
/// the entries into the reproducer output VFS YAML file.
1183-
LLVM_ABI void collectVFSFromYAML(
1184-
std::unique_ptr<llvm::MemoryBuffer> Buffer,
1185-
llvm::SourceMgr::DiagHandlerTy DiagHandler, StringRef YAMLFilePath,
1186-
SmallVectorImpl<YAMLVFSEntry> &CollectedEntries,
1187-
void *DiagContext = nullptr,
1188-
IntrusiveRefCntPtr<FileSystem> ExternalFS = getRealFileSystem());
1183+
void collectVFSEntries(RedirectingFileSystem &VFS,
1184+
SmallVectorImpl<YAMLVFSEntry> &CollectedEntries);
11891185

11901186
class YAMLVFSWriter {
11911187
std::vector<YAMLVFSEntry> Mappings;

llvm/lib/Support/VirtualFileSystem.cpp

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

2729-
void vfs::collectVFSFromYAML(std::unique_ptr<MemoryBuffer> Buffer,
2730-
SourceMgr::DiagHandlerTy DiagHandler,
2731-
StringRef YAMLFilePath,
2732-
SmallVectorImpl<YAMLVFSEntry> &CollectedEntries,
2733-
void *DiagContext,
2734-
IntrusiveRefCntPtr<FileSystem> ExternalFS) {
2735-
std::unique_ptr<RedirectingFileSystem> VFS = RedirectingFileSystem::create(
2736-
std::move(Buffer), DiagHandler, YAMLFilePath, DiagContext,
2737-
std::move(ExternalFS));
2738-
if (!VFS)
2739-
return;
2740-
ErrorOr<RedirectingFileSystem::LookupResult> RootResult =
2741-
VFS->lookupPath("/");
2729+
void vfs::collectVFSEntries(RedirectingFileSystem &VFS,
2730+
SmallVectorImpl<YAMLVFSEntry> &CollectedEntries) {
2731+
ErrorOr<RedirectingFileSystem::LookupResult> RootResult = VFS.lookupPath("/");
27422732
if (!RootResult)
27432733
return;
27442734
SmallVector<StringRef, 8> Components;

0 commit comments

Comments
 (0)