https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/198411
>From 45301fc6c166cd4ed1fd10e1f51779063200d55b Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Mon, 18 May 2026 15:28:01 -0700 Subject: [PATCH 1/3] [clang] Remove `FileSystemStatCache` This mechanism is not used anywhere, and can be easily replaced by a VFS. --- clang/include/clang/Basic/FileManager.h | 26 -- .../include/clang/Basic/FileSystemStatCache.h | 89 ----- clang/lib/Basic/CMakeLists.txt | 1 - clang/lib/Basic/FileManager.cpp | 92 +++-- clang/lib/Basic/FileSystemStatCache.cpp | 121 ------- clang/lib/Tooling/Tooling.cpp | 5 +- clang/unittests/Basic/FileManagerTest.cpp | 330 +++++------------- 7 files changed, 154 insertions(+), 510 deletions(-) delete mode 100644 clang/include/clang/Basic/FileSystemStatCache.h delete mode 100644 clang/lib/Basic/FileSystemStatCache.cpp diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h index e440a57e3a866..700e42f66bf8a 100644 --- a/clang/include/clang/Basic/FileManager.h +++ b/clang/include/clang/Basic/FileManager.h @@ -41,8 +41,6 @@ class MemoryBuffer; namespace clang { -class FileSystemStatCache; - /// Implements support for file system lookup, file system caching, /// and directory search management. /// @@ -120,9 +118,6 @@ class FileManager : public RefCountedBase<FileManager> { unsigned NumDirCacheMisses = 0; unsigned NumFileCacheMisses = 0; - // Caching. - std::unique_ptr<FileSystemStatCache> StatCache; - std::error_code getStatValue(StringRef Path, llvm::vfs::Status &Status, bool isFile, std::unique_ptr<llvm::vfs::File> *F, bool IsText = true); @@ -143,18 +138,6 @@ class FileManager : public RefCountedBase<FileManager> { IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = nullptr); ~FileManager(); - /// Installs the provided FileSystemStatCache object within - /// the FileManager. - /// - /// Ownership of this object is transferred to the FileManager. - /// - /// \param statCache the new stat cache to install. Ownership of this - /// object is transferred to the FileManager. - void setStatCache(std::unique_ptr<FileSystemStatCache> statCache); - - /// Removes the FileSystemStatCache object from the manager. - void clearStatCache(); - /// Returns the number of unique real file entries cached by the file manager. size_t getNumUniqueRealFiles() const { return UniqueRealFiles.size(); } @@ -276,15 +259,6 @@ class FileManager : public RefCountedBase<FileManager> { DirectoryEntry *&getRealDirEntry(const llvm::vfs::Status &Status); public: - /// Get the 'stat' information for the given \p Path. - /// - /// If the path is relative, it will be resolved against the WorkingDir of the - /// FileManager's FileSystemOptions. - /// - /// \returns a \c std::error_code describing an error, if there was one - std::error_code getNoncachedStatValue(StringRef Path, - llvm::vfs::Status &Result); - /// If path is not absolute and FileSystemOptions set the working /// directory, the path is modified to be relative to the given /// working directory. diff --git a/clang/include/clang/Basic/FileSystemStatCache.h b/clang/include/clang/Basic/FileSystemStatCache.h deleted file mode 100644 index 73c256a016971..0000000000000 --- a/clang/include/clang/Basic/FileSystemStatCache.h +++ /dev/null @@ -1,89 +0,0 @@ -//===- FileSystemStatCache.h - Caching for 'stat' calls ---------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// -// -/// \file -/// Defines the FileSystemStatCache interface. -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_CLANG_BASIC_FILESYSTEMSTATCACHE_H -#define LLVM_CLANG_BASIC_FILESYSTEMSTATCACHE_H - -#include "clang/Basic/LLVM.h" -#include "llvm/ADT/StringMap.h" -#include "llvm/ADT/StringRef.h" -#include "llvm/Support/Allocator.h" -#include "llvm/Support/FileSystem.h" -#include "llvm/Support/VirtualFileSystem.h" -#include <cstdint> -#include <ctime> -#include <memory> -#include <optional> -#include <string> -#include <utility> - -namespace clang { - -/// Abstract interface for introducing a FileManager cache for 'stat' -/// system calls, which is used by precompiled and pretokenized headers to -/// improve performance. -class FileSystemStatCache { - virtual void anchor(); - -public: - virtual ~FileSystemStatCache() = default; - - /// Get the 'stat' information for the specified path, using the cache - /// to accelerate it if possible. - /// - /// \returns \c true if the path does not exist or \c false if it exists. - /// - /// If isFile is true, then this lookup should only return success for files - /// (not directories). If it is false this lookup should only return - /// success for directories (not files). On a successful file lookup, the - /// implementation can optionally fill in \p F with a valid \p File object and - /// the client guarantees that it will close it. - static std::error_code get(StringRef Path, llvm::vfs::Status &Status, - bool isFile, std::unique_ptr<llvm::vfs::File> *F, - FileSystemStatCache *Cache, - llvm::vfs::FileSystem &FS, bool IsText = true); - -protected: - // FIXME: The pointer here is a non-owning/optional reference to the - // unique_ptr. std::optional<unique_ptr<vfs::File>&> might be nicer, but - // Optional needs some work to support references so this isn't possible yet. - virtual std::error_code getStat(StringRef Path, llvm::vfs::Status &Status, - bool isFile, - std::unique_ptr<llvm::vfs::File> *F, - llvm::vfs::FileSystem &FS) = 0; -}; - -/// A stat "cache" that can be used by FileManager to keep -/// track of the results of stat() calls that occur throughout the -/// execution of the front end. -class MemorizeStatCalls : public FileSystemStatCache { -public: - /// The set of stat() calls that have been seen. - llvm::StringMap<llvm::vfs::Status, llvm::BumpPtrAllocator> StatCalls; - - using iterator = - llvm::StringMap<llvm::vfs::Status, - llvm::BumpPtrAllocator>::const_iterator; - - iterator begin() const { return StatCalls.begin(); } - iterator end() const { return StatCalls.end(); } - - std::error_code getStat(StringRef Path, llvm::vfs::Status &Status, - bool isFile, - std::unique_ptr<llvm::vfs::File> *F, - llvm::vfs::FileSystem &FS) override; -}; - -} // namespace clang - -#endif // LLVM_CLANG_BASIC_FILESYSTEMSTATCACHE_H diff --git a/clang/lib/Basic/CMakeLists.txt b/clang/lib/Basic/CMakeLists.txt index adfc6ee326b5a..350c77696bcc4 100644 --- a/clang/lib/Basic/CMakeLists.txt +++ b/clang/lib/Basic/CMakeLists.txt @@ -69,7 +69,6 @@ add_clang_library(clangBasic ExpressionTraits.cpp FileEntry.cpp FileManager.cpp - FileSystemStatCache.cpp IdentifierTable.cpp LangOptions.cpp LangStandards.cpp diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp index a126d14087963..dd5c01f99ad3d 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -17,7 +17,6 @@ //===----------------------------------------------------------------------===// #include "clang/Basic/FileManager.h" -#include "clang/Basic/FileSystemStatCache.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/Statistic.h" #include "llvm/Config/llvm-config.h" @@ -75,13 +74,6 @@ FileManager::FileManager(const FileSystemOptions &FSO, FileManager::~FileManager() = default; -void FileManager::setStatCache(std::unique_ptr<FileSystemStatCache> statCache) { - assert(statCache && "No stat cache provided?"); - StatCache = std::move(statCache); -} - -void FileManager::clearStatCache() { StatCache.reset(); } - /// Retrieve the directory that the given file name resides in. /// Filename can point to either a real file or a virtual file. static llvm::Expected<DirectoryEntryRef> @@ -578,39 +570,79 @@ FileManager::getBufferForFileImpl(StringRef Filename, int64_t FileSize, isVolatile, IsText); } -/// getStatValue - Get the 'stat' information for the specified path, -/// using the cache to accelerate it if possible. This returns true -/// if the path points to a virtual file or does not exist, or returns -/// false if it's an existent real file. If FileDescriptor is NULL, -/// do directory look-up instead of file look-up. std::error_code FileManager::getStatValue(StringRef Path, llvm::vfs::Status &Status, bool isFile, std::unique_ptr<llvm::vfs::File> *F, bool IsText) { + SmallString<128> FilePath; + // FIXME: FileSystemOpts shouldn't be passed in here, all paths should be // absolute! - if (FileSystemOpts.WorkingDir.empty()) - return FileSystemStatCache::get(Path, Status, isFile, F, StatCache.get(), - *FS, IsText); + if (!FileSystemOpts.WorkingDir.empty()) { + FilePath = Path; + FixupRelativePath(FilePath); + Path = FilePath; + } - SmallString<128> FilePath(Path); - FixupRelativePath(FilePath); + bool isForDir = !isFile; + std::error_code RetCode; - return FileSystemStatCache::get(FilePath.c_str(), Status, isFile, F, - StatCache.get(), *FS, IsText); -} + if (isForDir || !F) { + // If this is a directory or a file descriptor is not needed, just go to the + // file system. + llvm::ErrorOr<llvm::vfs::Status> StatusOrErr = FS->status(Path); + if (!StatusOrErr) { + RetCode = StatusOrErr.getError(); + } else { + Status = *StatusOrErr; + } + } else { + // We can always just use 'stat' here, but (for files) the client is asking + // whether the file exists because it wants to turn around and *open* it. + // It is more efficient to do "open+fstat" on success than it is to do + // "stat+open". + // + // Because of this, check to see if the file exists with 'open'. If the + // open succeeds, use fstat to get the stat info. + auto OwnedFile = + IsText ? FS->openFileForRead(Path) : FS->openFileForReadBinary(Path); + + if (!OwnedFile) { + // If the open fails, our "stat" fails. + RetCode = OwnedFile.getError(); + } else { + // Otherwise, the open succeeded. Do an fstat to get the information + // about the file. We'll end up returning the open file descriptor to the + // client to do what they please with it. + llvm::ErrorOr<llvm::vfs::Status> StatusOrErr = (*OwnedFile)->status(); + if (StatusOrErr) { + Status = *StatusOrErr; + *F = std::move(*OwnedFile); + } else { + // fstat rarely fails. If it does, claim the initial open didn't + // succeed. + *F = nullptr; + RetCode = StatusOrErr.getError(); + } + } + } -std::error_code -FileManager::getNoncachedStatValue(StringRef Path, - llvm::vfs::Status &Result) { - SmallString<128> FilePath(Path); - FixupRelativePath(FilePath); + // If the path doesn't exist, return failure. + if (RetCode) + return RetCode; + + // If the path exists, make sure that its "directoryness" matches the clients + // demands. + if (Status.isDirectory() != isForDir) { + // If not, close the file if opened. + if (F) + *F = nullptr; + return std::make_error_code( + Status.isDirectory() ? + std::errc::is_a_directory : std::errc::not_a_directory); + } - llvm::ErrorOr<llvm::vfs::Status> S = FS->status(FilePath.c_str()); - if (!S) - return S.getError(); - Result = *S; return std::error_code(); } diff --git a/clang/lib/Basic/FileSystemStatCache.cpp b/clang/lib/Basic/FileSystemStatCache.cpp deleted file mode 100644 index 863c9c9c3959e..0000000000000 --- a/clang/lib/Basic/FileSystemStatCache.cpp +++ /dev/null @@ -1,121 +0,0 @@ -//===- FileSystemStatCache.cpp - Caching for 'stat' calls -----------------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// -// -// This file defines the FileSystemStatCache interface. -// -//===----------------------------------------------------------------------===// - -#include "clang/Basic/FileSystemStatCache.h" -#include "llvm/Support/ErrorOr.h" -#include "llvm/Support/Path.h" -#include "llvm/Support/VirtualFileSystem.h" -#include <utility> - -using namespace clang; - -void FileSystemStatCache::anchor() {} - -/// FileSystemStatCache::get - Get the 'stat' information for the specified -/// path, using the cache to accelerate it if possible. This returns true if -/// the path does not exist or false if it exists. -/// -/// If isFile is true, then this lookup should only return success for files -/// (not directories). If it is false this lookup should only return -/// success for directories (not files). On a successful file lookup, the -/// implementation can optionally fill in FileDescriptor with a valid -/// descriptor and the client guarantees that it will close it. -std::error_code FileSystemStatCache::get(StringRef Path, - llvm::vfs::Status &Status, bool isFile, - std::unique_ptr<llvm::vfs::File> *F, - FileSystemStatCache *Cache, - llvm::vfs::FileSystem &FS, - bool IsText) { - bool isForDir = !isFile; - std::error_code RetCode; - - // If we have a cache, use it to resolve the stat query. - if (Cache) - RetCode = Cache->getStat(Path, Status, isFile, F, FS); - else if (isForDir || !F) { - // If this is a directory or a file descriptor is not needed and we have - // no cache, just go to the file system. - llvm::ErrorOr<llvm::vfs::Status> StatusOrErr = FS.status(Path); - if (!StatusOrErr) { - RetCode = StatusOrErr.getError(); - } else { - Status = *StatusOrErr; - } - } else { - // Otherwise, we have to go to the filesystem. We can always just use - // 'stat' here, but (for files) the client is asking whether the file exists - // because it wants to turn around and *open* it. It is more efficient to - // do "open+fstat" on success than it is to do "stat+open". - // - // Because of this, check to see if the file exists with 'open'. If the - // open succeeds, use fstat to get the stat info. - auto OwnedFile = - IsText ? FS.openFileForRead(Path) : FS.openFileForReadBinary(Path); - - if (!OwnedFile) { - // If the open fails, our "stat" fails. - RetCode = OwnedFile.getError(); - } else { - // Otherwise, the open succeeded. Do an fstat to get the information - // about the file. We'll end up returning the open file descriptor to the - // client to do what they please with it. - llvm::ErrorOr<llvm::vfs::Status> StatusOrErr = (*OwnedFile)->status(); - if (StatusOrErr) { - Status = *StatusOrErr; - *F = std::move(*OwnedFile); - } else { - // fstat rarely fails. If it does, claim the initial open didn't - // succeed. - *F = nullptr; - RetCode = StatusOrErr.getError(); - } - } - } - - // If the path doesn't exist, return failure. - if (RetCode) - return RetCode; - - // If the path exists, make sure that its "directoryness" matches the clients - // demands. - if (Status.isDirectory() != isForDir) { - // If not, close the file if opened. - if (F) - *F = nullptr; - return std::make_error_code( - Status.isDirectory() ? - std::errc::is_a_directory : std::errc::not_a_directory); - } - - return std::error_code(); -} - -std::error_code -MemorizeStatCalls::getStat(StringRef Path, llvm::vfs::Status &Status, - bool isFile, - std::unique_ptr<llvm::vfs::File> *F, - llvm::vfs::FileSystem &FS) { - auto err = get(Path, Status, isFile, F, nullptr, FS); - if (err) { - // Do not cache failed stats, it is easy to construct common inconsistent - // situations if we do, and they are not important for PCH performance - // (which currently only needs the stats to construct the initial - // FileManager entries). - return err; - } - - // Cache file 'stat' results and directories with absolutely paths. - if (!Status.isDirectory() || llvm::sys::path::is_absolute(Path)) - StatCalls[Path] = Status; - - return std::error_code(); -} diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp index 9830096c2d0b9..5da9c31ec47ec 100644 --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -456,10 +456,7 @@ bool FrontendActionFactory::runInvocation( // pass it to an std::unique_ptr declared after the Compiler variable. std::unique_ptr<FrontendAction> ScopedToolAction(create()); - const bool Success = Compiler.ExecuteAction(*ScopedToolAction); - - Files->clearStatCache(); - return Success; + return Compiler.ExecuteAction(*ScopedToolAction); } ClangTool::ClangTool(const CompilationDatabase &Compilations, diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp index e54d3f8f9e416..0a67a5a78ae85 100644 --- a/clang/unittests/Basic/FileManagerTest.cpp +++ b/clang/unittests/Basic/FileManagerTest.cpp @@ -8,8 +8,6 @@ #include "clang/Basic/FileManager.h" #include "clang/Basic/FileSystemOptions.h" -#include "clang/Basic/FileSystemStatCache.h" -#include "llvm/ADT/STLExtras.h" #include "llvm/Support/Path.h" #include "llvm/Support/VirtualFileSystem.h" #include "llvm/Testing/Support/Error.h" @@ -20,83 +18,16 @@ using namespace clang; namespace { -// Used to create a fake file system for running the tests with such -// that the tests are not affected by the structure/contents of the -// file system on the machine running the tests. -class FakeStatCache : public FileSystemStatCache { -private: - // Maps a file/directory path to its desired stat result. Anything - // not in this map is considered to not exist in the file system. - llvm::StringMap<llvm::vfs::Status, llvm::BumpPtrAllocator> StatCalls; - - void InjectFileOrDirectory(const char *Path, ino_t INode, bool IsFile, - const char *StatPath) { - SmallString<128> NormalizedPath(Path); - SmallString<128> NormalizedStatPath; - if (is_style_posix(llvm::sys::path::Style::native)) { - llvm::sys::path::native(NormalizedPath); - Path = NormalizedPath.c_str(); - - if (StatPath) { - NormalizedStatPath = StatPath; - llvm::sys::path::native(NormalizedStatPath); - StatPath = NormalizedStatPath.c_str(); - } - } - - auto fileType = IsFile ? - llvm::sys::fs::file_type::regular_file : - llvm::sys::fs::file_type::directory_file; - llvm::vfs::Status Status(StatPath ? StatPath : Path, - llvm::sys::fs::UniqueID(1, INode), - /*MTime*/{}, /*User*/0, /*Group*/0, - /*Size*/0, fileType, - llvm::sys::fs::perms::all_all); - if (StatPath) - Status.ExposesExternalVFSPath = true; - StatCalls[Path] = Status; - } - -public: - // Inject a file with the given inode value to the fake file system. - void InjectFile(const char *Path, ino_t INode, - const char *StatPath = nullptr) { - InjectFileOrDirectory(Path, INode, /*IsFile=*/true, StatPath); - } - - // Inject a directory with the given inode value to the fake file system. - void InjectDirectory(const char *Path, ino_t INode) { - InjectFileOrDirectory(Path, INode, /*IsFile=*/false, nullptr); - } - - // Implement FileSystemStatCache::getStat(). - std::error_code getStat(StringRef Path, llvm::vfs::Status &Status, - bool isFile, - std::unique_ptr<llvm::vfs::File> *F, - llvm::vfs::FileSystem &FS) override { - SmallString<128> NormalizedPath(Path); - if (is_style_posix(llvm::sys::path::Style::native)) { - llvm::sys::path::native(NormalizedPath); - Path = NormalizedPath.c_str(); - } - - if (StatCalls.count(Path) != 0) { - Status = StatCalls[Path]; - return std::error_code(); - } - - return std::make_error_code(std::errc::no_such_file_or_directory); - } -}; - // The test fixture. class FileManagerTest : public ::testing::Test { - protected: - FileManagerTest() : manager(options) { - } - +protected: FileSystemOptions options; - FileManager manager; + IntrusiveRefCntPtr<vfs::InMemoryFileSystem> FS = + llvm::makeIntrusiveRefCnt<vfs::InMemoryFileSystem>(); + FileManager manager{options, FS}; + + time_t ModTime = 0; + std::unique_ptr<MemoryBuffer> EmptyBuf = MemoryBuffer::getMemBuffer(""); }; // When a virtual file is added, its getDir() field has correct name. @@ -110,12 +41,6 @@ TEST_F(FileManagerTest, getVirtualFileSetsTheDirFieldCorrectly) { // Before any virtual file is added, no virtual directory exists. TEST_F(FileManagerTest, NoVirtualDirectoryExistsBeforeAVirtualFileIsAdded) { - // An empty FakeStatCache causes all stat calls made by the - // FileManager to report "file/directory doesn't exist". This - // avoids the possibility of the result of this test being affected - // by what's in the real file system. - manager.setStatCache(std::make_unique<FakeStatCache>()); - ASSERT_FALSE(manager.getOptionalDirectoryRef("virtual/dir/foo")); ASSERT_FALSE(manager.getOptionalDirectoryRef("virtual/dir")); ASSERT_FALSE(manager.getOptionalDirectoryRef("virtual")); @@ -123,9 +48,6 @@ TEST_F(FileManagerTest, NoVirtualDirectoryExistsBeforeAVirtualFileIsAdded) { // When a virtual file is added, all of its ancestors should be created. TEST_F(FileManagerTest, getVirtualFileCreatesDirectoryEntriesForAncestors) { - // Fake an empty real file system. - manager.setStatCache(std::make_unique<FakeStatCache>()); - manager.getVirtualFileRef("virtual/dir/bar.h", 100, 0); auto dir = manager.getDirectoryRef("virtual/dir/foo"); @@ -143,37 +65,16 @@ TEST_F(FileManagerTest, getVirtualFileCreatesDirectoryEntriesForAncestors) { // getFileRef() succeeds if a real file exists at the given path. TEST_F(FileManagerTest, getFileReturnsValidFileEntryForExistingRealFile) { // Inject fake files into the file system. - auto statCache = std::make_unique<FakeStatCache>(); - statCache->InjectDirectory("/tmp", 42); - statCache->InjectFile("/tmp/test", 43); - -#ifdef _WIN32 - const char *DirName = "C:."; - const char *FileName = "C:test"; - statCache->InjectDirectory(DirName, 44); - statCache->InjectFile(FileName, 45); -#endif - - manager.setStatCache(std::move(statCache)); + FS->addFileNoOwn("/tmp/test", ModTime, *EmptyBuf); auto file = manager.getFileRef("/tmp/test"); ASSERT_THAT_EXPECTED(file, llvm::Succeeded()); EXPECT_EQ("/tmp/test", file->getName()); - EXPECT_EQ("/tmp", file->getDir().getName()); - -#ifdef _WIN32 - file = manager.getFileRef(FileName); - ASSERT_THAT_EXPECTED(file, llvm::Succeeded()); - EXPECT_EQ(DirName, file->getDir().getName()); -#endif } // getFileRef() succeeds if a virtual file exists at the given path. TEST_F(FileManagerTest, getFileReturnsValidFileEntryForExistingVirtualFile) { - // Fake an empty real file system. - manager.setStatCache(std::make_unique<FakeStatCache>()); - manager.getVirtualFileRef("virtual/dir/bar.h", 100, 0); auto file = manager.getFileRef("virtual/dir/bar.h"); ASSERT_THAT_EXPECTED(file, llvm::Succeeded()); @@ -184,13 +85,9 @@ TEST_F(FileManagerTest, getFileReturnsValidFileEntryForExistingVirtualFile) { // getFile() returns different FileEntries for different paths when // there's no aliasing. TEST_F(FileManagerTest, getFileReturnsDifferentFileEntriesForDifferentFiles) { - // Inject two fake files into the file system. Different inodes - // mean the files are not symlinked together. - auto statCache = std::make_unique<FakeStatCache>(); - statCache->InjectDirectory(".", 41); - statCache->InjectFile("foo.cpp", 42); - statCache->InjectFile("bar.cpp", 43); - manager.setStatCache(std::move(statCache)); + // Inject two fake files into the file system. + FS->addFileNoOwn("foo.cpp", ModTime, *EmptyBuf); + FS->addFileNoOwn("bar.cpp", ModTime, *EmptyBuf); auto fileFoo = manager.getOptionalFileRef("foo.cpp"); auto fileBar = manager.getOptionalFileRef("bar.cpp"); @@ -203,11 +100,8 @@ TEST_F(FileManagerTest, getFileReturnsDifferentFileEntriesForDifferentFiles) { // exists at the given path. TEST_F(FileManagerTest, getFileReturnsErrorForNonexistentFile) { // Inject a fake foo.cpp into the file system. - auto statCache = std::make_unique<FakeStatCache>(); - statCache->InjectDirectory(".", 41); - statCache->InjectFile("foo.cpp", 42); - statCache->InjectDirectory("MyDirectory", 49); - manager.setStatCache(std::move(statCache)); + FS->addFileNoOwn("foo.cpp", ModTime, *EmptyBuf); + FS->addFileNoOwn("MyDirectory/keep", ModTime, *EmptyBuf); // Create a virtual bar.cpp file. manager.getVirtualFileRef("bar.cpp", 200, 0); @@ -228,18 +122,11 @@ TEST_F(FileManagerTest, getFileReturnsErrorForNonexistentFile) { std::make_error_code(std::errc::not_a_directory)); } -// The following tests apply to Unix-like system only. - -#ifndef _WIN32 - // getFile() returns the same FileEntry for real files that are aliases. TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedRealFiles) { // Inject two real files with the same inode. - auto statCache = std::make_unique<FakeStatCache>(); - statCache->InjectDirectory("abc", 41); - statCache->InjectFile("abc/foo.cpp", 42); - statCache->InjectFile("abc/bar.cpp", 42); - manager.setStatCache(std::move(statCache)); + FS->addFileNoOwn("abc/foo.cpp", ModTime, *EmptyBuf); + FS->addHardLink("abc/bar.cpp", "abc/foo.cpp"); auto f1 = manager.getOptionalFileRef("abc/foo.cpp"); auto f2 = manager.getOptionalFileRef("abc/bar.cpp"); @@ -260,16 +147,11 @@ TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedRealFiles) { } TEST_F(FileManagerTest, getFileRefReturnsCorrectNameForDifferentStatPath) { - // Inject files with the same inode, but where some files have a stat that - // gives a different name. This is adding coverage for stat behaviour - // triggered by the RedirectingFileSystem for 'use-external-name' that - // FileManager::getFileRef has special logic for. - auto StatCache = std::make_unique<FakeStatCache>(); - StatCache->InjectDirectory("dir", 40); - StatCache->InjectFile("dir/f1.cpp", 41); - StatCache->InjectFile("dir/f1-alias.cpp", 41, "dir/f1.cpp"); - StatCache->InjectFile("dir/f2.cpp", 42); - StatCache->InjectFile("dir/f2-alias.cpp", 42, "dir/f2.cpp"); + // This is adding coverage for stat behaviour triggered by the + // RedirectingFileSystem for 'use-external-name' that FileManager::getFileRef + // has special logic for. + FS->addFileNoOwn("/dir/f1.cpp", ModTime, *EmptyBuf); + FS->addFileNoOwn("/dir/f2.cpp", ModTime, *EmptyBuf); // This unintuitive rename-the-file-on-stat behaviour supports how the // RedirectingFileSystem VFS layer responds to stats. However, even if you @@ -277,51 +159,56 @@ TEST_F(FileManagerTest, getFileRefReturnsCorrectNameForDifferentStatPath) { // following stat cache behaviour is not supported (the correct stat entry // for a double-redirection would be "dir/f1.cpp") and the getFileRef below // should assert. - StatCache->InjectFile("dir/f1-alias-alias.cpp", 41, "dir/f1-alias.cpp"); + IntrusiveRefCntPtr<vfs::FileSystem> NewFS = FS; + + NewFS = vfs::RedirectingFileSystem::create( + {{"/dir/f1-alias.cpp", "/dir/f1.cpp"}, + {"/dir/f2-alias.cpp", "/dir/f2.cpp"}}, + /*UseExternalNames=*/true, std::move(NewFS)); + + NewFS = vfs::RedirectingFileSystem::create( + {{"/dir/f1-alias-alias.cpp", "/dir/f1-alias.cpp"}}, + /*UseExternalNames=*/true, std::move(NewFS)); - manager.setStatCache(std::move(StatCache)); + manager.setVirtualFileSystem(std::move(NewFS)); // With F1, test accessing the non-redirected name first. - auto F1 = manager.getFileRef("dir/f1.cpp"); - auto F1Alias = manager.getFileRef("dir/f1-alias.cpp"); - auto F1Alias2 = manager.getFileRef("dir/f1-alias.cpp"); + auto F1 = manager.getFileRef("/dir/f1.cpp"); + auto F1Alias = manager.getFileRef("/dir/f1-alias.cpp"); + auto F1Alias2 = manager.getFileRef("/dir/f1-alias.cpp"); ASSERT_FALSE(!F1); ASSERT_FALSE(!F1Alias); ASSERT_FALSE(!F1Alias2); - EXPECT_EQ("dir/f1.cpp", F1->getName()); - EXPECT_EQ("dir/f1.cpp", F1Alias->getName()); - EXPECT_EQ("dir/f1.cpp", F1Alias2->getName()); + EXPECT_EQ("/dir/f1.cpp", F1->getName()); + EXPECT_EQ("/dir/f1.cpp", F1Alias->getName()); + EXPECT_EQ("/dir/f1.cpp", F1Alias2->getName()); EXPECT_EQ(&F1->getFileEntry(), &F1Alias->getFileEntry()); EXPECT_EQ(&F1->getFileEntry(), &F1Alias2->getFileEntry()); #if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST - EXPECT_DEATH((void)manager.getFileRef("dir/f1-alias-alias.cpp"), + EXPECT_DEATH((void)manager.getOptionalFileRef("/dir/f1-alias-alias.cpp"), "filename redirected to a non-canonical filename?"); #endif // With F2, test accessing the redirected name first. - auto F2Alias = manager.getFileRef("dir/f2-alias.cpp"); - auto F2 = manager.getFileRef("dir/f2.cpp"); - auto F2Alias2 = manager.getFileRef("dir/f2-alias.cpp"); + auto F2Alias = manager.getFileRef("/dir/f2-alias.cpp"); + auto F2 = manager.getFileRef("/dir/f2.cpp"); + auto F2Alias2 = manager.getFileRef("/dir/f2-alias.cpp"); ASSERT_FALSE(!F2); ASSERT_FALSE(!F2Alias); ASSERT_FALSE(!F2Alias2); - EXPECT_EQ("dir/f2.cpp", F2->getName()); - EXPECT_EQ("dir/f2.cpp", F2Alias->getName()); - EXPECT_EQ("dir/f2.cpp", F2Alias2->getName()); + EXPECT_EQ("/dir/f2.cpp", F2->getName()); + EXPECT_EQ("/dir/f2.cpp", F2Alias->getName()); + EXPECT_EQ("/dir/f2.cpp", F2Alias2->getName()); EXPECT_EQ(&F2->getFileEntry(), &F2Alias->getFileEntry()); EXPECT_EQ(&F2->getFileEntry(), &F2Alias2->getFileEntry()); } TEST_F(FileManagerTest, getFileRefReturnsCorrectDirNameForDifferentStatPath) { // Inject files with the same inode into distinct directories (name & inode). - auto StatCache = std::make_unique<FakeStatCache>(); - StatCache->InjectDirectory("dir1", 40); - StatCache->InjectDirectory("dir2", 41); - StatCache->InjectFile("dir1/f.cpp", 42); - StatCache->InjectFile("dir2/f.cpp", 42, "dir1/f.cpp"); + FS->addFileNoOwn("dir1/f.cpp", ModTime, *EmptyBuf); + FS->addHardLink("dir2/f.cpp", "dir1/f.cpp"); - manager.setStatCache(std::move(StatCache)); auto Dir1F = manager.getFileRef("dir1/f.cpp"); auto Dir2F = manager.getFileRef("dir2/f.cpp"); @@ -337,11 +224,8 @@ TEST_F(FileManagerTest, getFileRefReturnsCorrectDirNameForDifferentStatPath) { // corresponding real files that are aliases. TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedVirtualFiles) { // Inject two real files with the same inode. - auto statCache = std::make_unique<FakeStatCache>(); - statCache->InjectDirectory("abc", 41); - statCache->InjectFile("abc/foo.cpp", 42); - statCache->InjectFile("abc/bar.cpp", 42); - manager.setStatCache(std::move(statCache)); + FS->addFileNoOwn("abc/foo.cpp", ModTime, *EmptyBuf); + FS->addHardLink("abc/bar.cpp", "abc/foo.cpp"); auto f1 = manager.getOptionalFileRef("abc/foo.cpp"); auto f2 = manager.getOptionalFileRef("abc/bar.cpp"); @@ -351,20 +235,23 @@ TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedVirtualFiles) { } TEST_F(FileManagerTest, getFileRefEquality) { - auto StatCache = std::make_unique<FakeStatCache>(); - StatCache->InjectDirectory("dir", 40); - StatCache->InjectFile("dir/f1.cpp", 41); - StatCache->InjectFile("dir/f1-also.cpp", 41); - StatCache->InjectFile("dir/f1-redirect.cpp", 41, "dir/f1.cpp"); - StatCache->InjectFile("dir/f2.cpp", 42); - manager.setStatCache(std::move(StatCache)); - - auto F1 = manager.getFileRef("dir/f1.cpp"); - auto F1Again = manager.getFileRef("dir/f1.cpp"); - auto F1Also = manager.getFileRef("dir/f1-also.cpp"); - auto F1Redirect = manager.getFileRef("dir/f1-redirect.cpp"); - auto F1RedirectAgain = manager.getFileRef("dir/f1-redirect.cpp"); - auto F2 = manager.getFileRef("dir/f2.cpp"); + FS->addFileNoOwn("/dir/f1.cpp", ModTime, *EmptyBuf); + FS->addHardLink("/dir/f1-also.cpp", "/dir/f1.cpp"); + FS->addFileNoOwn("/dir/f2.cpp", ModTime, *EmptyBuf); + + IntrusiveRefCntPtr<vfs::FileSystem> NewFS = + vfs::RedirectingFileSystem::create( + {{"/dir/f1-redirect.cpp", "/dir/f1.cpp"}}, + /*UseExternalNames=*/true, FS); + + manager.setVirtualFileSystem(std::move(NewFS)); + + auto F1 = manager.getFileRef("/dir/f1.cpp"); + auto F1Again = manager.getFileRef("/dir/f1.cpp"); + auto F1Also = manager.getFileRef("/dir/f1-also.cpp"); + auto F1Redirect = manager.getFileRef("/dir/f1-redirect.cpp"); + auto F1RedirectAgain = manager.getFileRef("/dir/f1-redirect.cpp"); + auto F2 = manager.getFileRef("/dir/f2.cpp"); // Check Expected<FileEntryRef> for error. ASSERT_FALSE(!F1); @@ -375,15 +262,15 @@ TEST_F(FileManagerTest, getFileRefEquality) { ASSERT_FALSE(!F2); // Check names. - EXPECT_EQ("dir/f1.cpp", F1->getName()); - EXPECT_EQ("dir/f1.cpp", F1Again->getName()); - EXPECT_EQ("dir/f1-also.cpp", F1Also->getName()); - EXPECT_EQ("dir/f1.cpp", F1Redirect->getName()); - EXPECT_EQ("dir/f1.cpp", F1RedirectAgain->getName()); - EXPECT_EQ("dir/f2.cpp", F2->getName()); + EXPECT_EQ("/dir/f1.cpp", F1->getName()); + EXPECT_EQ("/dir/f1.cpp", F1Again->getName()); + EXPECT_EQ("/dir/f1-also.cpp", F1Also->getName()); + EXPECT_EQ("/dir/f1.cpp", F1Redirect->getName()); + EXPECT_EQ("/dir/f1.cpp", F1RedirectAgain->getName()); + EXPECT_EQ("/dir/f2.cpp", F2->getName()); - EXPECT_EQ("dir/f1.cpp", F1->getNameAsRequested()); - EXPECT_EQ("dir/f1-redirect.cpp", F1Redirect->getNameAsRequested()); + EXPECT_EQ("/dir/f1.cpp", F1->getNameAsRequested()); + EXPECT_EQ("/dir/f1-redirect.cpp", F1Redirect->getNameAsRequested()); // Compare against FileEntry*. EXPECT_EQ(&F1->getFileEntry(), *F1); @@ -418,30 +305,22 @@ TEST_F(FileManagerTest, getFileRefEquality) { // here by checking the size. TEST_F(FileManagerTest, getVirtualFileWithDifferentName) { // Inject fake files into the file system. - auto statCache = std::make_unique<FakeStatCache>(); - statCache->InjectDirectory("c:\\tmp", 42); - statCache->InjectFile("c:\\tmp\\test", 43); - - manager.setStatCache(std::move(statCache)); + FS->addFileNoOwn("/tmp/test", ModTime, *EmptyBuf); // Inject the virtual file: - FileEntryRef file1 = manager.getVirtualFileRef("c:\\tmp\\test", 123, 1); - EXPECT_EQ(43U, file1.getUniqueID().getFile()); + FileEntryRef file1 = manager.getVirtualFileRef("/tmp/test", 123, 1); EXPECT_EQ(123, file1.getSize()); // Lookup the virtual file with a different name: - auto file2 = manager.getOptionalFileRef("c:/tmp/test", 100, 1); + auto file2 = manager.getOptionalFileRef("/tmp/./test", 100, 1); ASSERT_TRUE(file2); // Check that it's the same UFE: EXPECT_EQ(file1, *file2); - EXPECT_EQ(43U, file2->getUniqueID().getFile()); // Check that the contents of the UFE are not overwritten by the entry in the // filesystem: EXPECT_EQ(123, file2->getSize()); } -#endif // !_WIN32 - static StringRef getSystemRoot() { return is_style_windows(llvm::sys::path::Style::native) ? "C:\\" : "/"; } @@ -454,19 +333,15 @@ TEST_F(FileManagerTest, makeAbsoluteUsesVFS) { : StringRef("/"); llvm::sys::path::append(CustomWorkingDir, "some", "weird", "path"); - auto FS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>(); - // setCurrentworkingdirectory must finish without error. + // setCurrentWorkingDirectory must finish without error. ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir)); - FileSystemOptions Opts; - FileManager Manager(Opts, FS); - SmallString<64> Path("a/foo.cpp"); SmallString<64> ExpectedResult(CustomWorkingDir); llvm::sys::path::append(ExpectedResult, Path); - ASSERT_TRUE(Manager.makeAbsolutePath(Path)); + ASSERT_TRUE(manager.makeAbsolutePath(Path)); EXPECT_EQ(Path, ExpectedResult); } @@ -474,22 +349,14 @@ TEST_F(FileManagerTest, makeAbsoluteUsesVFS) { TEST_F(FileManagerTest, getVirtualFileFillsRealPathName) { SmallString<64> CustomWorkingDir = getSystemRoot(); - auto FS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>(); - // setCurrentworkingdirectory must finish without error. + // setCurrentWorkingDirectory must finish without error. ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir)); - FileSystemOptions Opts; - FileManager Manager(Opts, FS); - // Inject fake files into the file system. - auto statCache = std::make_unique<FakeStatCache>(); - statCache->InjectDirectory("/tmp", 42); - statCache->InjectFile("/tmp/test", 43); - - Manager.setStatCache(std::move(statCache)); + FS->addFileNoOwn("/tmp/test", ModTime, *EmptyBuf); // Check for real path. - FileEntryRef file = Manager.getVirtualFileRef("/tmp/test", 123, 1); + FileEntryRef file = manager.getVirtualFileRef("/tmp/test", 123, 1); SmallString<64> ExpectedResult = CustomWorkingDir; llvm::sys::path::append(ExpectedResult, "tmp", "test"); @@ -503,22 +370,14 @@ TEST_F(FileManagerTest, getVirtualFileFillsRealPathName) { TEST_F(FileManagerTest, getFileDontOpenRealPath) { SmallString<64> CustomWorkingDir = getSystemRoot(); - auto FS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>(); - // setCurrentworkingdirectory must finish without error. + // setCurrentWorkingDirectory must finish without error. ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir)); - FileSystemOptions Opts; - FileManager Manager(Opts, FS); - // Inject fake files into the file system. - auto statCache = std::make_unique<FakeStatCache>(); - statCache->InjectDirectory("/tmp", 42); - statCache->InjectFile("/tmp/test", 43); - - Manager.setStatCache(std::move(statCache)); + FS->addFileNoOwn("/tmp/test", ModTime, *EmptyBuf); // Check for real path. - auto file = Manager.getOptionalFileRef("/tmp/test", /*OpenFile=*/false); + auto file = manager.getOptionalFileRef("/tmp/test", /*OpenFile=*/false); ASSERT_TRUE(file); SmallString<64> ExpectedResult = CustomWorkingDir; @@ -538,21 +397,14 @@ TEST_F(FileManagerTest, getBypassFile) { CustomWorkingDir = "/"; #endif - auto FS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>(); - // setCurrentworkingdirectory must finish without error. + // setCurrentWorkingDirectory must finish without error. ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir)); - FileSystemOptions Opts; - FileManager Manager(Opts, FS); - // Inject fake files into the file system. - auto Cache = std::make_unique<FakeStatCache>(); - Cache->InjectDirectory("/tmp", 42); - Cache->InjectFile("/tmp/test", 43); - Manager.setStatCache(std::move(Cache)); + FS->addFileNoOwn("/tmp/test", ModTime, *EmptyBuf); - // Set up a virtual file with a different size than FakeStatCache uses. - FileEntryRef File = Manager.getVirtualFileRef("/tmp/test", /*Size=*/10, 0); + // Set up a virtual file with a different size than the VFS uses. + FileEntryRef File = manager.getVirtualFileRef("/tmp/test", /*Size=*/10, 0); ASSERT_TRUE(File); const FileEntry &FE = *File; EXPECT_EQ(FE.getSize(), 10); @@ -560,14 +412,14 @@ TEST_F(FileManagerTest, getBypassFile) { // Calling a second time should not affect the UID or size. unsigned VirtualUID = FE.getUID(); OptionalFileEntryRef SearchRef; - ASSERT_THAT_ERROR(Manager.getFileRef("/tmp/test").moveInto(SearchRef), + ASSERT_THAT_ERROR(manager.getFileRef("/tmp/test").moveInto(SearchRef), Succeeded()); EXPECT_EQ(&FE, &SearchRef->getFileEntry()); EXPECT_EQ(FE.getUID(), VirtualUID); EXPECT_EQ(FE.getSize(), 10); // Bypass the file. - OptionalFileEntryRef BypassRef = Manager.getBypassFile(File); + OptionalFileEntryRef BypassRef = manager.getBypassFile(File); ASSERT_TRUE(BypassRef); EXPECT_EQ("/tmp/test", BypassRef->getName()); @@ -577,7 +429,7 @@ TEST_F(FileManagerTest, getBypassFile) { EXPECT_NE(BypassRef->getSize(), FE.getSize()); // The virtual file should still be returned when searching. - ASSERT_THAT_ERROR(Manager.getFileRef("/tmp/test").moveInto(SearchRef), + ASSERT_THAT_ERROR(manager.getFileRef("/tmp/test").moveInto(SearchRef), Succeeded()); EXPECT_EQ(&FE, &SearchRef->getFileEntry()); } >From e79d47f0414f7707eabed84ce1e6915964b13ea2 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Mon, 18 May 2026 15:33:29 -0700 Subject: [PATCH 2/3] git-clang-format --- clang/lib/Basic/FileManager.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp index dd5c01f99ad3d..c0c11277bf66c 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -638,9 +638,9 @@ std::error_code FileManager::getStatValue(StringRef Path, // If not, close the file if opened. if (F) *F = nullptr; - return std::make_error_code( - Status.isDirectory() ? - std::errc::is_a_directory : std::errc::not_a_directory); + return std::make_error_code(Status.isDirectory() + ? std::errc::is_a_directory + : std::errc::not_a_directory); } return std::error_code(); >From f73bc58953a80e66d98222e69f4dd4934309e07d Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Mon, 18 May 2026 15:57:45 -0700 Subject: [PATCH 3/3] clangd --- clang-tools-extra/clangd/index/StdLib.cpp | 4 +--- clang-tools-extra/clangd/index/SymbolCollector.cpp | 7 ++----- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clangd/index/StdLib.cpp b/clang-tools-extra/clangd/index/StdLib.cpp index 483589365eace..29600539c3daa 100644 --- a/clang-tools-extra/clangd/index/StdLib.cpp +++ b/clang-tools-extra/clangd/index/StdLib.cpp @@ -314,9 +314,7 @@ std::optional<StdLibLocation> StdLibSet::add(const LangOptions &LO, case DirectoryLookup::LT_NormalDir: { Path = DL.getDirRef()->getName(); llvm::sys::path::append(Path, ProbeHeader); - llvm::vfs::Status Stat; - if (!HS.getFileMgr().getNoncachedStatValue(Path, Stat) && - Stat.isRegularFile()) + if (HS.getFileMgr().getOptionalFileRef(Path)) RecordHeaderPath(Path); break; } diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index bd974e4c18818..b9602b36e2bba 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -392,17 +392,14 @@ class SymbolCollector::HeaderFileURICache { llvm::sys::path::append(UmbrellaPath, Framework + ".framework", "Headers", Framework + ".h"); - llvm::vfs::Status Status; - auto StatErr = HS.getFileMgr().getNoncachedStatValue(UmbrellaPath, Status); - if (!StatErr) + if (HS.getFileMgr().getOptionalFileRef(UmbrellaPath)) CachedSpelling->PublicHeader = llvm::formatv("<{0}/{0}.h>", Framework); UmbrellaPath = HeaderPath.FrameworkParentDir; llvm::sys::path::append(UmbrellaPath, Framework + ".framework", "PrivateHeaders", Framework + "_Private.h"); - StatErr = HS.getFileMgr().getNoncachedStatValue(UmbrellaPath, Status); - if (!StatErr) + if (HS.getFileMgr().getOptionalFileRef(UmbrellaPath)) CachedSpelling->PrivateHeader = llvm::formatv("<{0}/{0}_Private.h>", Framework); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
