[clang] Revert "[clang][scan-deps] Add option to disable caching stat failures" (PR #145528)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 24 08:12:07 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Qinkun Bao (qinkunbao)
<details>
<summary>Changes</summary>
Reverts llvm/llvm-project#<!-- -->144000
First buildbot failure:
https://lab.llvm.org/buildbot/#/builders/164/builds/11064
---
Full diff: https://github.com/llvm/llvm-project/pull/145528.diff
9 Files Affected:
- (modified) clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h (+15-9)
- (modified) clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h (+1-5)
- (modified) clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp (+6-34)
- (modified) clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp (+1-2)
- (modified) clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp (+2-1)
- (modified) clang/tools/clang-scan-deps/ClangScanDeps.cpp (+2-6)
- (modified) clang/tools/clang-scan-deps/Opts.td (-1)
- (modified) clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp (-52)
- (modified) clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp (+15-22)
``````````diff
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index da83220babea3..a20a89a4c2b76 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -23,8 +23,6 @@ namespace clang {
namespace tooling {
namespace dependencies {
-class DependencyScanningService;
-
using DependencyDirectivesTy =
SmallVector<dependency_directives_scan::Directive, 20>;
@@ -351,7 +349,7 @@ class DependencyScanningWorkerFilesystem
static const char ID;
DependencyScanningWorkerFilesystem(
- DependencyScanningService &Service,
+ DependencyScanningFilesystemSharedCache &SharedCache,
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS);
llvm::ErrorOr<llvm::vfs::Status> status(const Twine &Path) override;
@@ -437,7 +435,10 @@ class DependencyScanningWorkerFilesystem
/// Returns entry associated with the unique ID in the shared cache or nullptr
/// if none is found.
const CachedFileSystemEntry *
- findSharedEntryByUID(llvm::vfs::Status Stat) const;
+ findSharedEntryByUID(llvm::vfs::Status Stat) const {
+ return SharedCache.getShardForUID(Stat.getUniqueID())
+ .findEntryByUID(Stat.getUniqueID());
+ }
/// Associates the given entry with the filename in the local cache and
/// returns it.
@@ -451,14 +452,20 @@ class DependencyScanningWorkerFilesystem
/// some. Otherwise, constructs new one with the given error code, associates
/// it with the filename and returns the result.
const CachedFileSystemEntry &
- getOrEmplaceSharedEntryForFilename(StringRef Filename, std::error_code EC);
+ getOrEmplaceSharedEntryForFilename(StringRef Filename, std::error_code EC) {
+ return SharedCache.getShardForFilename(Filename)
+ .getOrEmplaceEntryForFilename(Filename, EC);
+ }
/// Returns entry associated with the filename in the shared cache if there is
/// some. Otherwise, associates the given entry with the filename and returns
/// it.
const CachedFileSystemEntry &
getOrInsertSharedEntryForFilename(StringRef Filename,
- const CachedFileSystemEntry &Entry);
+ const CachedFileSystemEntry &Entry) {
+ return SharedCache.getShardForFilename(Filename)
+ .getOrInsertEntryForFilename(Filename, Entry);
+ }
void printImpl(raw_ostream &OS, PrintType Type,
unsigned IndentLevel) const override {
@@ -471,9 +478,8 @@ class DependencyScanningWorkerFilesystem
/// VFS.
bool shouldBypass(StringRef Path) const;
- /// The service associated with this VFS.
- DependencyScanningService &Service;
-
+ /// 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;
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index ceaf3c2279e7f..4e97c7bc9f36e 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -87,8 +87,7 @@ class DependencyScanningService {
ScanningOptimizations OptimizeArgs = ScanningOptimizations::Default,
bool EagerLoadModules = false, bool TraceVFS = false,
std::time_t BuildSessionTimestamp =
- llvm::sys::toTimeT(std::chrono::system_clock::now()),
- bool CacheNegativeStats = true);
+ llvm::sys::toTimeT(std::chrono::system_clock::now()));
ScanningMode getMode() const { return Mode; }
@@ -100,8 +99,6 @@ class DependencyScanningService {
bool shouldTraceVFS() const { return TraceVFS; }
- bool shouldCacheNegativeStats() const { return CacheNegativeStats; }
-
DependencyScanningFilesystemSharedCache &getSharedCache() {
return SharedCache;
}
@@ -119,7 +116,6 @@ class DependencyScanningService {
const bool EagerLoadModules;
/// Whether to trace VFS accesses.
const bool TraceVFS;
- const bool CacheNegativeStats;
/// The global file system cache.
DependencyScanningFilesystemSharedCache SharedCache;
/// The global module cache entries.
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 2868522f80018..140833050f4e9 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -7,7 +7,6 @@
//===----------------------------------------------------------------------===//
#include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
-#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Threading.h"
#include <optional>
@@ -233,19 +232,19 @@ bool DependencyScanningWorkerFilesystem::shouldBypass(StringRef Path) const {
}
DependencyScanningWorkerFilesystem::DependencyScanningWorkerFilesystem(
- DependencyScanningService &Service,
+ DependencyScanningFilesystemSharedCache &SharedCache,
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
: llvm::RTTIExtends<DependencyScanningWorkerFilesystem,
llvm::vfs::ProxyFileSystem>(std::move(FS)),
- Service(Service), WorkingDirForCacheLookup(llvm::errc::invalid_argument) {
+ SharedCache(SharedCache),
+ WorkingDirForCacheLookup(llvm::errc::invalid_argument) {
updateWorkingDirForCacheLookup();
}
const CachedFileSystemEntry &
DependencyScanningWorkerFilesystem::getOrEmplaceSharedEntryForUID(
TentativeEntry TEntry) {
- auto &Shard =
- Service.getSharedCache().getShardForUID(TEntry.Status.getUniqueID());
+ auto &Shard = SharedCache.getShardForUID(TEntry.Status.getUniqueID());
return Shard.getOrEmplaceEntryForUID(TEntry.Status.getUniqueID(),
std::move(TEntry.Status),
std::move(TEntry.Contents));
@@ -256,44 +255,18 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough(
StringRef Filename) {
if (const auto *Entry = LocalCache.findEntryByFilename(Filename))
return Entry;
- auto &Shard = Service.getSharedCache().getShardForFilename(Filename);
+ auto &Shard = SharedCache.getShardForFilename(Filename);
if (const auto *Entry = Shard.findEntryByFilename(Filename))
return &LocalCache.insertEntryForFilename(Filename, *Entry);
return nullptr;
}
-const CachedFileSystemEntry *
-DependencyScanningWorkerFilesystem::findSharedEntryByUID(
- llvm::vfs::Status Stat) const {
- return Service.getSharedCache()
- .getShardForUID(Stat.getUniqueID())
- .findEntryByUID(Stat.getUniqueID());
-}
-
-const CachedFileSystemEntry &
-DependencyScanningWorkerFilesystem::getOrEmplaceSharedEntryForFilename(
- StringRef Filename, std::error_code EC) {
- return Service.getSharedCache()
- .getShardForFilename(Filename)
- .getOrEmplaceEntryForFilename(Filename, EC);
-}
-
-const CachedFileSystemEntry &
-DependencyScanningWorkerFilesystem::getOrInsertSharedEntryForFilename(
- StringRef Filename, const CachedFileSystemEntry &Entry) {
- return Service.getSharedCache()
- .getShardForFilename(Filename)
- .getOrInsertEntryForFilename(Filename, Entry);
-}
-
llvm::ErrorOr<const CachedFileSystemEntry &>
DependencyScanningWorkerFilesystem::computeAndStoreResult(
StringRef OriginalFilename, StringRef FilenameForLookup) {
llvm::ErrorOr<llvm::vfs::Status> Stat =
getUnderlyingFS().status(OriginalFilename);
if (!Stat) {
- if (!Service.shouldCacheNegativeStats())
- return Stat.getError();
const auto &Entry =
getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError());
return insertLocalEntryForFilename(FilenameForLookup, Entry);
@@ -447,8 +420,7 @@ DependencyScanningWorkerFilesystem::getRealPath(const Twine &Path,
return HandleCachedRealPath(*RealPath);
// If we have the result in the shared cache, cache it locally.
- auto &Shard =
- Service.getSharedCache().getShardForFilename(*FilenameForLookup);
+ auto &Shard = SharedCache.getShardForFilename(*FilenameForLookup);
if (const auto *ShardRealPath =
Shard.findRealPathByFilename(*FilenameForLookup)) {
const auto &RealPath = LocalCache.insertRealPathForFilename(
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
index c2f3cdbb02e37..7f40c99f07287 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
@@ -15,8 +15,7 @@ using namespace dependencies;
DependencyScanningService::DependencyScanningService(
ScanningMode Mode, ScanningOutputFormat Format,
ScanningOptimizations OptimizeArgs, bool EagerLoadModules, bool TraceVFS,
- std::time_t BuildSessionTimestamp, bool CacheNegativeStats)
+ std::time_t BuildSessionTimestamp)
: Mode(Mode), Format(Format), OptimizeArgs(OptimizeArgs),
EagerLoadModules(EagerLoadModules), TraceVFS(TraceVFS),
- CacheNegativeStats(CacheNegativeStats),
BuildSessionTimestamp(BuildSessionTimestamp) {}
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index d9820ca3c584e..9bd85479d9810 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -611,7 +611,8 @@ DependencyScanningWorker::DependencyScanningWorker(
switch (Service.getMode()) {
case ScanningMode::DependencyDirectivesScan:
- DepFS = new DependencyScanningWorkerFilesystem(Service, FS);
+ DepFS =
+ new DependencyScanningWorkerFilesystem(Service.getSharedCache(), FS);
BaseFS = DepFS;
break;
case ScanningMode::CanonicalPreprocessing:
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index ce0770a51d65f..921ba7aadd67d 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -85,7 +85,6 @@ static ScanningOutputFormat Format = ScanningOutputFormat::Make;
static ScanningOptimizations OptimizeArgs;
static std::string ModuleFilesDir;
static bool EagerLoadModules;
-static bool CacheNegativeStats = true;
static unsigned NumThreads = 0;
static std::string CompilationDB;
static std::optional<std::string> ModuleName;
@@ -192,8 +191,6 @@ static void ParseArgs(int argc, char **argv) {
EagerLoadModules = Args.hasArg(OPT_eager_load_pcm);
- CacheNegativeStats = !Args.hasArg(OPT_no_cache_negative_stats);
-
if (const llvm::opt::Arg *A = Args.getLastArg(OPT_j)) {
StringRef S{A->getValue()};
if (!llvm::to_integer(S, NumThreads, 0)) {
@@ -1083,9 +1080,8 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
});
};
- DependencyScanningService Service(
- ScanMode, Format, OptimizeArgs, EagerLoadModules, /*TraceVFS=*/Verbose,
- llvm::sys::toTimeT(std::chrono::system_clock::now()), CacheNegativeStats);
+ DependencyScanningService Service(ScanMode, Format, OptimizeArgs,
+ EagerLoadModules, /*TraceVFS=*/Verbose);
llvm::Timer T;
T.startTimer();
diff --git a/clang/tools/clang-scan-deps/Opts.td b/clang/tools/clang-scan-deps/Opts.td
index 582ae60851e1e..9cccbb3aaf0c8 100644
--- a/clang/tools/clang-scan-deps/Opts.td
+++ b/clang/tools/clang-scan-deps/Opts.td
@@ -22,7 +22,6 @@ defm module_files_dir : Eq<"module-files-dir",
def optimize_args_EQ : CommaJoined<["-", "--"], "optimize-args=">, HelpText<"Which command-line arguments of modules to optimize">;
def eager_load_pcm : F<"eager-load-pcm", "Load PCM files eagerly (instead of lazily on import)">;
-def no_cache_negative_stats : F<"no-cache-negative-stats", "Don't cache stat failures">;
def j : Arg<"j", "Number of worker threads to use (default: use all concurrent threads)">;
diff --git a/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp b/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp
index d194b2877ad8f..683d9070b1dcf 100644
--- a/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp
+++ b/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp
@@ -384,55 +384,3 @@ TEST(DependencyScanner, ScanDepsWithDiagConsumer) {
EXPECT_TRUE(DiagConsumer.Finished);
}
}
-
-TEST(DependencyScanner, NoNegativeCache) {
- StringRef CWD = "/root";
-
- auto VFS = new llvm::vfs::InMemoryFileSystem();
- VFS->setCurrentWorkingDirectory(CWD);
- auto Sept = llvm::sys::path::get_separator();
- std::string HeaderPath =
- std::string(llvm::formatv("{0}root{0}header.h", Sept));
- std::string Test0Path =
- std::string(llvm::formatv("{0}root{0}test0.cpp", Sept));
- std::string Test1Path =
- std::string(llvm::formatv("{0}root{0}test1.cpp", Sept));
-
- VFS->addFile(Test0Path, 0,
- llvm::MemoryBuffer::getMemBuffer(
- "#if __has_include(\"header.h\")\n#endif"));
- VFS->addFile(Test1Path, 0,
- llvm::MemoryBuffer::getMemBuffer("#include \"header.h\""));
-
- DependencyScanningService Service(
- ScanningMode::DependencyDirectivesScan, ScanningOutputFormat::Make,
- ScanningOptimizations::All, false, false,
- llvm::sys::toTimeT(std::chrono::system_clock::now()), false);
- DependencyScanningTool ScanTool(Service, VFS);
-
- std::vector<std::string> CommandLine0 = {"clang",
- "-target",
- "x86_64-apple-macosx10.7",
- "-c",
- "test0.cpp",
- "-o"
- "test0.cpp.o"};
- std::vector<std::string> CommandLine1 = {"clang",
- "-target",
- "x86_64-apple-macosx10.7",
- "-c",
- "test1.cpp",
- "-o"
- "test1.cpp.o"};
-
- std::string Result;
- ASSERT_THAT_ERROR(
- ScanTool.getDependencyFile(CommandLine0, CWD).moveInto(Result),
- llvm::Succeeded());
-
- VFS->addFile(HeaderPath, 0, llvm::MemoryBuffer::getMemBuffer(""));
-
- ASSERT_THAT_ERROR(
- ScanTool.getDependencyFile(CommandLine1, CWD).moveInto(Result),
- llvm::Succeeded());
-}
diff --git a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
index a68ea72d3816c..7420743c97a2a 100644
--- a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
+++ b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
@@ -7,7 +7,6 @@
//===----------------------------------------------------------------------===//
#include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
-#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Support/VirtualFileSystem.h"
#include "gtest/gtest.h"
@@ -20,10 +19,9 @@ TEST(DependencyScanningWorkerFilesystem, CacheStatusFailures) {
auto InstrumentingFS =
llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS);
- DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
- ScanningOutputFormat::Make);
- DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS);
- DependencyScanningWorkerFilesystem DepFS2(Service, InstrumentingFS);
+ DependencyScanningFilesystemSharedCache SharedCache;
+ DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
+ DependencyScanningWorkerFilesystem DepFS2(SharedCache, InstrumentingFS);
DepFS.status("/foo.c");
EXPECT_EQ(InstrumentingFS->NumStatusCalls, 1u);
@@ -47,10 +45,9 @@ TEST(DependencyScanningFilesystem, CacheGetRealPath) {
auto InstrumentingFS =
llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS);
- DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
- ScanningOutputFormat::Make);
- DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS);
- DependencyScanningWorkerFilesystem DepFS2(Service, InstrumentingFS);
+ DependencyScanningFilesystemSharedCache SharedCache;
+ DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
+ DependencyScanningWorkerFilesystem DepFS2(SharedCache, InstrumentingFS);
{
llvm::SmallString<128> Result;
@@ -83,9 +80,8 @@ TEST(DependencyScanningFilesystem, RealPathAndStatusInvariants) {
InMemoryFS->addFile("/foo.c", 0, llvm::MemoryBuffer::getMemBuffer(""));
InMemoryFS->addFile("/bar.c", 0, llvm::MemoryBuffer::getMemBuffer(""));
- DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
- ScanningOutputFormat::Make);
- DependencyScanningWorkerFilesystem DepFS(Service, InMemoryFS);
+ DependencyScanningFilesystemSharedCache SharedCache;
+ DependencyScanningWorkerFilesystem DepFS(SharedCache, InMemoryFS);
// Success.
{
@@ -137,9 +133,8 @@ TEST(DependencyScanningFilesystem, CacheStatOnExists) {
InMemoryFS->setCurrentWorkingDirectory("/");
InMemoryFS->addFile("/foo", 0, llvm::MemoryBuffer::getMemBuffer(""));
InMemoryFS->addFile("/bar", 0, llvm::MemoryBuffer::getMemBuffer(""));
- DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
- ScanningOutputFormat::Make);
- DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS);
+ DependencyScanningFilesystemSharedCache SharedCache;
+ DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
DepFS.status("/foo");
DepFS.status("/foo");
@@ -161,9 +156,8 @@ TEST(DependencyScanningFilesystem, CacheStatFailures) {
auto InstrumentingFS =
llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS);
- DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
- ScanningOutputFormat::Make);
- DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS);
+ DependencyScanningFilesystemSharedCache SharedCache;
+ DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
DepFS.status("/dir");
DepFS.status("/dir");
@@ -189,9 +183,8 @@ TEST(DependencyScanningFilesystem, DiagnoseStaleStatFailures) {
auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
InMemoryFS->setCurrentWorkingDirectory("/");
- DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
- ScanningOutputFormat::Make);
- DependencyScanningWorkerFilesystem DepFS(Service, InMemoryFS);
+ DependencyScanningFilesystemSharedCache SharedCache;
+ DependencyScanningWorkerFilesystem DepFS(SharedCache, InMemoryFS);
bool Path1Exists = DepFS.exists("/path1.suffix");
EXPECT_EQ(Path1Exists, false);
@@ -204,7 +197,7 @@ TEST(DependencyScanningFilesystem, DiagnoseStaleStatFailures) {
EXPECT_EQ(Path1Exists, false);
std::vector<llvm::StringRef> InvalidPaths =
- Service.getSharedCache().getInvalidNegativeStatCachedPaths(*InMemoryFS);
+ SharedCache.getInvalidNegativeStatCachedPaths(*InMemoryFS);
EXPECT_EQ(InvalidPaths.size(), 1u);
ASSERT_STREQ("/path1.suffix", InvalidPaths[0].str().c_str());
``````````
</details>
https://github.com/llvm/llvm-project/pull/145528
More information about the cfe-commits
mailing list