https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/190843
>From e8e0dd36ea86a04f178c6128a69d9dfeeeb47088 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Thu, 2 Apr 2026 15:19:45 -0700 Subject: [PATCH 1/2] [clang][deps] Simplify scanner VFS --- clang/include/clang/Basic/Module.h | 51 ++++++++++++------- .../DependencyScanningFilesystem.h | 13 ----- clang/lib/Basic/Module.cpp | 9 ++-- .../DependencyScannerImpl.cpp | 11 +--- .../DependencyScanningFilesystem.cpp | 13 ----- .../DependencyScanningFilesystemTest.cpp | 11 ---- 6 files changed, 39 insertions(+), 69 deletions(-) diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index 70668860dadc2..7ada9efef1ba4 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -56,37 +56,42 @@ 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 +/// For implicitly-built modules, this is the \c sys::fs::UniqueID 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. +/// 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. - 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; + struct Implicit { + /// The unique ID of the module cache directory. + llvm::sys::fs::UniqueID ModuleCacheUID; + /// The module file path relative to the module cache. + std::string ImplicitModulePathSuffix; + + bool operator==(const Implicit &Other) const { + return ModuleCacheUID == Other.ModuleCacheUID && + ImplicitModulePathSuffix == Other.ImplicitModulePathSuffix; + } + }; + + std::variant<const FileEntry *, Implicit> Key; friend class ModuleFileName; friend llvm::DenseMapInfo<ModuleFileKey>; - ModuleFileKey(const void *Ptr) : Ptr(Ptr) {} - - ModuleFileKey(const FileEntry *ModuleFile) : Ptr(ModuleFile) {} + ModuleFileKey(const FileEntry *ModuleFile) : Key(ModuleFile) {} - ModuleFileKey(const DirectoryEntry *ModuleCacheDir, StringRef PathSuffix) - : Ptr(ModuleCacheDir), ImplicitModulePathSuffix(PathSuffix) {} + ModuleFileKey(llvm::sys::fs::UniqueID ModuleCacheUID, StringRef PathSuffix) + : Key(Implicit{ModuleCacheUID, PathSuffix.str()}) {} public: bool operator==(const ModuleFileKey &Other) const { - return Ptr == Other.Ptr && - ImplicitModulePathSuffix == Other.ImplicitModulePathSuffix; + return Key == Other.Key; } bool operator!=(const ModuleFileKey &Other) const { @@ -1042,15 +1047,23 @@ class VisibleModuleSet { template <> struct llvm::DenseMapInfo<clang::ModuleFileKey> { static clang::ModuleFileKey getEmptyKey() { - return DenseMapInfo<const void *>::getEmptyKey(); + return DenseMapInfo<const clang::FileEntry *>::getEmptyKey(); } static clang::ModuleFileKey getTombstoneKey() { - return DenseMapInfo<const void *>::getTombstoneKey(); + return DenseMapInfo<const clang::FileEntry *>::getTombstoneKey(); } static unsigned getHashValue(const clang::ModuleFileKey &Val) { - return hash_combine(Val.Ptr, Val.ImplicitModulePathSuffix); + return std::visit( + makeVisitor( + [](const clang::FileEntry *FE) { return hash_combine(0, FE); }, + [](const clang::ModuleFileKey::Implicit &I) { + return hash_combine(1, I.ModuleCacheUID.getDevice(), + I.ModuleCacheUID.getFile(), + I.ImplicitModulePathSuffix); + }), + Val.Key); } static bool isEqual(const clang::ModuleFileKey &LHS, 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/Basic/Module.cpp b/clang/lib/Basic/Module.cpp index 81e28e46d36ca..cd8fed598f558 100644 --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -24,6 +24,7 @@ #include "llvm/ADT/StringSwitch.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" +#include "llvm/Support/IOSandbox.h" #include "llvm/Support/raw_ostream.h" #include <cassert> #include <functional> @@ -40,9 +41,11 @@ ModuleFileName::makeKey(FileManager &FileMgr) const { StringRef(Path).drop_back(ImplicitModuleSuffixLength); StringRef ImplicitModuleSuffix = StringRef(Path).take_back(ImplicitModuleSuffixLength); - if (auto ModuleCache = FileMgr.getOptionalDirectoryRef( - ModuleCachePath, /*CacheFailure=*/false)) - return ModuleFileKey(*ModuleCache, ImplicitModuleSuffix); + // 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(ModuleCachePath, Status) == std::error_code{}) + return ModuleFileKey(Status.getUniqueID(), ImplicitModuleSuffix); } else { if (auto ModuleFile = FileMgr.getOptionalFileRef(Path, /*OpenFile=*/true, /*CacheFailure=*/false)) diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp index 1940d2f275c73..a2272e0b35435 100644 --- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp @@ -424,19 +424,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) { >From 622c61bea2ab9a720942e34cab2d4df76153732c Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Tue, 7 Apr 2026 13:30:29 -0700 Subject: [PATCH 2/2] git-clang-format --- clang/include/clang/Basic/Module.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index 7ada9efef1ba4..09d94fca0eccf 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -90,9 +90,7 @@ class ModuleFileKey { : Key(Implicit{ModuleCacheUID, PathSuffix.str()}) {} public: - bool operator==(const ModuleFileKey &Other) const { - return Key == Other.Key; - } + bool operator==(const ModuleFileKey &Other) const { return Key == Other.Key; } bool operator!=(const ModuleFileKey &Other) const { return !operator==(Other); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
