[clang] [clang][deps] Simplify scanner VFS (PR #190843)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 20 13:49:29 PDT 2026
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/190843
>From a249245d35470731c49df4828ab4e183dfe27056 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 20 Apr 2026 12:58:08 -0700
Subject: [PATCH 1/2] [clang] Get the directory identity from `ModuleCache`
instead of `FileManager`
---
clang/include/clang/Basic/Module.h | 27 +++++++------------
.../include/clang/Serialization/ModuleCache.h | 20 ++++++++++++++
.../clang/Serialization/ModuleManager.h | 6 +++++
clang/lib/Basic/Module.cpp | 20 --------------
clang/lib/Frontend/CompilerInstance.cpp | 4 ++-
clang/lib/Serialization/ModuleCache.cpp | 17 ++++++++++++
clang/lib/Serialization/ModuleManager.cpp | 21 +++++++++++++--
7 files changed, 74 insertions(+), 41 deletions(-)
diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index 99f79625488ac..1d9953af057ad 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -56,34 +56,31 @@ using ModuleId = SmallVector<std::pair<std::string, SourceLocation>, 2>;
/// Deduplication key for a loaded module file in \c ModuleManager.
///
-/// For implicitly-built modules, this is the \c DirectoryEntry of the module
-/// cache and the module file name with the (optional) context hash.
-/// This enables using \c FileManager's inode-based canonicalization of the
-/// user-provided module cache path without hitting issues on file systems that
-/// recycle inodes for recompiled module files.
+/// For implicitly-built modules, this is a pointer representing the module
+/// cache directory and the module file name with the (optional) context hash.
+/// This enables using inode-based canonicalization of the user-provided module
+/// cache path without hitting issues on file systems that recycle inodes for
+/// recompiled module files.
///
/// For explicitly-built modules, this is \c FileEntry.
/// This uses \c FileManager's inode-based canonicalization of the user-provided
/// module file path. Because input explicitly-built modules do not change
/// during the lifetime of the compiler, inode recycling is not of concern here.
class ModuleFileKey {
- /// The FileManager entity used for deduplication.
+ /// The entity used for deduplication.
const void *Ptr;
/// The path relative to the module cache path for implicit module file, empty
/// for other kinds of module files.
std::string ImplicitModulePathSuffix;
- friend class ModuleFileName;
friend llvm::DenseMapInfo<ModuleFileKey>;
- ModuleFileKey(const void *Ptr) : Ptr(Ptr) {}
-
- ModuleFileKey(const FileEntry *ModuleFile) : Ptr(ModuleFile) {}
+public:
+ ModuleFileKey(const void *ModuleFile) : Ptr(ModuleFile) {}
- ModuleFileKey(const DirectoryEntry *ModuleCacheDir, StringRef PathSuffix)
+ ModuleFileKey(const void *ModuleCacheDir, StringRef PathSuffix)
: Ptr(ModuleCacheDir), ImplicitModulePathSuffix(PathSuffix) {}
-public:
bool operator==(const ModuleFileKey &Other) const {
return Ptr == Other.Ptr &&
ImplicitModulePathSuffix == Other.ImplicitModulePathSuffix;
@@ -148,12 +145,6 @@ class ModuleFileName {
/// Checks whether the module file name is empty.
bool empty() const { return Path.empty(); }
-
- /// Creates the deduplication key for use in \c ModuleManager.
- /// Returns an empty optional if:
- /// * the module cache does not exist for an implicit module name,
- /// * the module file does not exist for an explicit module name.
- std::optional<ModuleFileKey> makeKey(FileManager &FileMgr) const;
};
/// The signature of a module, which is a hash of the AST content.
diff --git a/clang/include/clang/Serialization/ModuleCache.h b/clang/include/clang/Serialization/ModuleCache.h
index 857f414ff0bec..0dae5091f3d91 100644
--- a/clang/include/clang/Serialization/ModuleCache.h
+++ b/clang/include/clang/Serialization/ModuleCache.h
@@ -10,6 +10,9 @@
#define LLVM_CLANG_SERIALIZATION_MODULECACHE_H
#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/FileSystem/UniqueID.h"
#include <ctime>
#include <memory>
@@ -25,10 +28,27 @@ class MemoryBufferRef;
namespace clang {
class InMemoryModuleCache;
+/// The address of an instance of this class represents the identity of a module
+/// cache directory.
+class ModuleCacheDirectory {};
+
/// The module cache used for compiling modules implicitly. This centralizes the
/// operations the compiler might want to perform on the cache.
class ModuleCache {
+ /// Mapping from a path to the module cache directory identity.
+ llvm::StringMap<const ModuleCacheDirectory *> ByName;
+
+ /// Mapping from the filesystem entity to the module cache directory identity.
+ llvm::DenseMap<llvm::sys::fs::UniqueID, std::unique_ptr<ModuleCacheDirectory>>
+ ByUID;
+
public:
+ /// Returns an opaque pointer representing the module cache directory. This
+ /// returns the same pointer regardless of the path spelling, as long as it
+ /// resolves to the same file system entity. This also resolves links in the
+ /// path. This may return nullptr if the module cache does not exist.
+ virtual const ModuleCacheDirectory *getDirectoryPtr(StringRef Path);
+
/// Returns lock for the given module file. The lock is initially unlocked.
virtual std::unique_ptr<llvm::AdvisoryLock>
getLock(StringRef ModuleFilename) = 0;
diff --git a/clang/include/clang/Serialization/ModuleManager.h b/clang/include/clang/Serialization/ModuleManager.h
index d3765ce743877..6f7f657dfc23f 100644
--- a/clang/include/clang/Serialization/ModuleManager.h
+++ b/clang/include/clang/Serialization/ModuleManager.h
@@ -328,6 +328,12 @@ class ModuleManager {
/// View the graphviz representation of the module graph.
void viewGraph();
+ /// Creates the deduplication key for use in \c ModuleManager.
+ /// Returns an empty optional if:
+ /// * the module cache does not exist for an implicit module name,
+ /// * the module file does not exist for an explicit module name.
+ std::optional<ModuleFileKey> makeKey(const ModuleFileName &Name) const;
+
ModuleCache &getModuleCache() const { return ModCache; }
};
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 869d0a5b0c3cf..97f742d292224 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -33,26 +33,6 @@
using namespace clang;
-std::optional<ModuleFileKey>
-ModuleFileName::makeKey(FileManager &FileMgr) const {
- if (ImplicitModuleSuffixLength) {
- StringRef ModuleCachePath =
- StringRef(Path).drop_back(ImplicitModuleSuffixLength);
- StringRef ImplicitModuleSuffix =
- StringRef(Path).take_back(ImplicitModuleSuffixLength);
- if (auto ModuleCache = FileMgr.getOptionalDirectoryRef(
- ModuleCachePath, /*CacheFailure=*/false))
- return ModuleFileKey(*ModuleCache, ImplicitModuleSuffix);
- } else {
- if (auto ModuleFile = FileMgr.getOptionalFileRef(Path, /*OpenFile=*/true,
- /*CacheFailure=*/false,
- /*IsText=*/false))
- return ModuleFileKey(*ModuleFile);
- }
-
- return std::nullopt;
-}
-
Module::Module(ModuleConstructorTag, StringRef Name,
SourceLocation DefinitionLoc, Module *Parent, bool IsFramework,
bool IsExplicit, unsigned VisibilityID)
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index be097ba867407..f868e1e449325 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -41,6 +41,7 @@
#include "clang/Serialization/GlobalModuleIndex.h"
#include "clang/Serialization/InMemoryModuleCache.h"
#include "clang/Serialization/ModuleCache.h"
+#include "clang/Serialization/ModuleManager.h"
#include "clang/Serialization/SerializationDiagnostic.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/STLExtras.h"
@@ -1945,7 +1946,8 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
// Check whether M refers to the file in the prebuilt module path.
if (M && M->getASTFileKey() &&
- *M->getASTFileKey() == ModuleFilename.makeKey(*FileMgr))
+ *M->getASTFileKey() ==
+ getASTReader()->getModuleManager().makeKey(ModuleFilename))
return M;
getDiagnostics().Report(ModuleNameLoc, diag::err_module_prebuilt)
diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp
index cc70a775d1411..419e421fae0d9 100644
--- a/clang/lib/Serialization/ModuleCache.cpp
+++ b/clang/lib/Serialization/ModuleCache.cpp
@@ -19,6 +19,23 @@
using namespace clang;
+const ModuleCacheDirectory *ModuleCache::getDirectoryPtr(StringRef Path) {
+ auto [ByNameIt, ByNameInserted] = ByName.insert({Path, nullptr});
+ if (!ByNameIt->second) {
+ // This is a compiler-internal input/output, let's bypass the sandbox.
+ auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
+ llvm::sys::fs::file_status Status;
+ if (llvm::sys::fs::status(Path, Status))
+ return nullptr;
+ llvm::sys::fs::UniqueID UID = Status.getUniqueID();
+ auto [ByUIDIt, ByUIDInserted] = ByUID.insert({UID, nullptr});
+ if (!ByUIDIt->second)
+ ByUIDIt->second = std::make_unique<ModuleCacheDirectory>();
+ ByNameIt->second = ByUIDIt->second.get();
+ }
+ return ByNameIt->second;
+}
+
/// Write a new timestamp file with the given path.
static void writeTimestampFile(StringRef TimestampFile) {
std::error_code EC;
diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp
index f0a29607d81a2..b1ec886704fe4 100644
--- a/clang/lib/Serialization/ModuleManager.cpp
+++ b/clang/lib/Serialization/ModuleManager.cpp
@@ -41,6 +41,23 @@
using namespace clang;
using namespace serialization;
+std::optional<ModuleFileKey>
+ModuleManager::makeKey(const ModuleFileName &Name) const {
+ if (unsigned SuffixLen = Name.getImplicitModuleSuffixLength()) {
+ StringRef ModuleCachePath = StringRef(Name).drop_back(SuffixLen);
+ StringRef ImplicitModuleSuffix = StringRef(Name).take_back(SuffixLen);
+ if (auto *ModuleCacheDir = ModCache.getDirectoryPtr(ModuleCachePath))
+ return ModuleFileKey(ModuleCacheDir, ImplicitModuleSuffix);
+ } else {
+ if (auto ModuleFile = FileMgr.getOptionalFileRef(Name, /*OpenFile=*/true,
+ /*CacheFailure=*/false,
+ /*IsText=*/false))
+ return ModuleFileKey(*ModuleFile);
+ }
+
+ return std::nullopt;
+}
+
ModuleFile *ModuleManager::lookupByModuleName(StringRef Name) const {
if (const Module *Mod = HeaderSearchInfo.getModuleMap().findModule(Name))
if (const ModuleFileName *FileName = Mod->getASTFileName())
@@ -50,7 +67,7 @@ ModuleFile *ModuleManager::lookupByModuleName(StringRef Name) const {
}
ModuleFile *ModuleManager::lookupByFileName(ModuleFileName Name) const {
- std::optional<ModuleFileKey> Key = Name.makeKey(FileMgr);
+ std::optional<ModuleFileKey> Key = makeKey(Name);
return Key ? lookup(*Key) : nullptr;
}
@@ -134,7 +151,7 @@ AddModuleResult ModuleManager::addModule(
ExpectedModTime = 0;
}
- std::optional<ModuleFileKey> FileKey = FileName.makeKey(FileMgr);
+ std::optional<ModuleFileKey> FileKey = makeKey(FileName);
if (!FileKey) {
Result.K = AddModuleResult::Missing;
return Result;
>From 976a438e8805cb08f8a8ebc9e316c7537f671fed Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Thu, 2 Apr 2026 15:19:45 -0700
Subject: [PATCH 2/2] [clang][deps] Simplify scanner VFS
---
.../DependencyScanningFilesystem.h | 13 -------------
.../DependencyScanning/DependencyScannerImpl.cpp | 11 +----------
.../DependencyScanningFilesystem.cpp | 13 -------------
.../DependencyScanningFilesystemTest.cpp | 11 -----------
4 files changed, 1 insertion(+), 47 deletions(-)
diff --git a/clang/include/clang/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/DependencyScanning/DependencyScanningFilesystem.h
index 2162222a66643..f50332c964ced 100644
--- a/clang/include/clang/DependencyScanning/DependencyScanningFilesystem.h
+++ b/clang/include/clang/DependencyScanning/DependencyScanningFilesystem.h
@@ -382,12 +382,6 @@ class DependencyScanningWorkerFilesystem
std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
- /// Make it so that no paths bypass this VFS.
- void resetBypassedPathPrefix() { BypassedPathPrefix.reset(); }
- /// Set the prefix for paths that should bypass this VFS and go straight to
- /// the underlying VFS.
- void setBypassedPathPrefix(StringRef Prefix) { BypassedPathPrefix = Prefix; }
-
/// Returns entry for the given filename.
///
/// Attempts to use the local and shared caches first, then falls back to
@@ -495,19 +489,12 @@ class DependencyScanningWorkerFilesystem
getUnderlyingFS().print(OS, Type, IndentLevel + 1);
}
- /// Whether this path should bypass this VFS and go straight to the underlying
- /// VFS.
- bool shouldBypass(StringRef Path) const;
-
/// The global cache shared between worker threads.
DependencyScanningFilesystemSharedCache &SharedCache;
/// The local cache is used by the worker thread to cache file system queries
/// locally instead of querying the global cache every time.
DependencyScanningFilesystemLocalCache LocalCache;
- /// Prefix of paths that should go straight to the underlying VFS.
- std::optional<std::string> BypassedPathPrefix;
-
/// The working directory to use for making relative paths absolute before
/// using them for cache lookups.
llvm::ErrorOr<std::string> WorkingDirForCacheLookup;
diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
index c752f76e53712..40a7d1b908a6c 100644
--- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
+++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
@@ -426,19 +426,10 @@ void dependencies::initializeScanCompilerInstance(
ScanInstance.createSourceManager();
// Use DepFS for getting the dependency directives if requested to do so.
- if (Service.getOpts().Mode == ScanningMode::DependencyDirectivesScan) {
- DepFS->resetBypassedPathPrefix();
- SmallString<256> ModulesCachePath;
- normalizeModuleCachePath(ScanInstance.getFileManager(),
- ScanInstance.getHeaderSearchOpts().ModuleCachePath,
- ModulesCachePath);
- if (!ModulesCachePath.empty())
- DepFS->setBypassedPathPrefix(ModulesCachePath);
-
+ if (Service.getOpts().Mode == ScanningMode::DependencyDirectivesScan)
ScanInstance.setDependencyDirectivesGetter(
std::make_unique<ScanningDependencyDirectivesGetter>(
ScanInstance.getFileManager()));
- }
}
std::shared_ptr<CompilerInvocation> dependencies::createScanCompilerInvocation(
diff --git a/clang/lib/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/DependencyScanning/DependencyScanningFilesystem.cpp
index 24a794e4a6a22..49dad3758cf57 100644
--- a/clang/lib/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -243,10 +243,6 @@ const CachedRealPath &DependencyScanningFilesystemSharedCache::CacheShard::
return *StoredRealPath;
}
-bool DependencyScanningWorkerFilesystem::shouldBypass(StringRef Path) const {
- return BypassedPathPrefix && Path.starts_with(*BypassedPathPrefix);
-}
-
DependencyScanningWorkerFilesystem::DependencyScanningWorkerFilesystem(
DependencyScanningFilesystemSharedCache &SharedCache,
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
@@ -328,9 +324,6 @@ DependencyScanningWorkerFilesystem::status(const Twine &Path) {
SmallString<256> OwnedFilename;
StringRef Filename = Path.toStringRef(OwnedFilename);
- if (shouldBypass(Filename))
- return getUnderlyingFS().status(Path);
-
llvm::ErrorOr<EntryRef> Result = getOrCreateFileSystemEntry(Filename);
if (!Result)
return Result.getError();
@@ -400,9 +393,6 @@ DependencyScanningWorkerFilesystem::openFileForRead(const Twine &Path) {
SmallString<256> OwnedFilename;
StringRef Filename = Path.toStringRef(OwnedFilename);
- if (shouldBypass(Filename))
- return getUnderlyingFS().openFileForRead(Path);
-
llvm::ErrorOr<EntryRef> Result = getOrCreateFileSystemEntry(Filename);
if (!Result)
return Result.getError();
@@ -415,9 +405,6 @@ DependencyScanningWorkerFilesystem::getRealPath(const Twine &Path,
SmallString<256> OwnedFilename;
StringRef OriginalFilename = Path.toStringRef(OwnedFilename);
- if (shouldBypass(OriginalFilename))
- return getUnderlyingFS().getRealPath(Path, Output);
-
SmallString<256> PathBuf;
auto FilenameForLookup = tryGetFilenameForLookup(OriginalFilename, PathBuf);
if (!FilenameForLookup)
diff --git a/clang/unittests/DependencyScanning/DependencyScanningFilesystemTest.cpp b/clang/unittests/DependencyScanning/DependencyScanningFilesystemTest.cpp
index 0e195411915aa..d9489a9bf27ca 100644
--- a/clang/unittests/DependencyScanning/DependencyScanningFilesystemTest.cpp
+++ b/clang/unittests/DependencyScanning/DependencyScanningFilesystemTest.cpp
@@ -199,17 +199,6 @@ TEST(DependencyScanningFilesystem, CacheStatFailures) {
DepFS.status("/dir/vector");
DepFS.status("/dir/vector");
EXPECT_EQ(InstrumentingFS->NumStatusCalls, 2u);
-
- DepFS.setBypassedPathPrefix("/cache");
- DepFS.exists("/cache/a.pcm");
- EXPECT_EQ(InstrumentingFS->NumStatusCalls, 3u);
- DepFS.exists("/cache/a.pcm");
- EXPECT_EQ(InstrumentingFS->NumStatusCalls, 4u);
-
- DepFS.resetBypassedPathPrefix();
- DepFS.exists("/cache/a.pcm");
- DepFS.exists("/cache/a.pcm");
- EXPECT_EQ(InstrumentingFS->NumStatusCalls, 5u);
}
TEST(DependencyScanningFilesystem, DiagnoseStaleStatFailures) {
More information about the cfe-commits
mailing list