https://github.com/qiongsiwu updated
https://github.com/llvm/llvm-project/pull/135703
>From d4b1210c16b4fccc6faa9445bee457a1e330a025 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi...@apple.com>
Date: Mon, 14 Apr 2025 16:49:07 -0700
Subject: [PATCH 1/5] Initial commit.
---
.../DependencyScanningFilesystem.h | 8 ++++++
.../DependencyScanningFilesystem.cpp | 26 +++++++++++++++++
.../DependencyScanningFilesystemTest.cpp | 28 +++++++++++++++++++
3 files changed, 62 insertions(+)
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index d12814e7c9253..648158c5287c6 100644
---
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -220,6 +220,14 @@ class DependencyScanningFilesystemSharedCache {
CacheShard &getShardForFilename(StringRef Filename) const;
CacheShard &getShardForUID(llvm::sys::fs::UniqueID UID) const;
+ /// Visits all cached entries and re-stat an entry using the UnderlyingFS if
+ /// it is negatively stat cached. If the re-stat succeeds, print diagnostics
+ /// information to OS indicating that negative stat caching has been
+ /// invalidated.
+ void
+ diagnoseNegativeStatCachedPaths(llvm::raw_ostream &OS,
+ llvm::vfs::FileSystem &UnderlyingFS) const;
+
private:
std::unique_ptr<CacheShard[]> CacheShards;
unsigned NumShards;
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 4d738e4bea41a..fe72a1a91909e 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -108,6 +108,32 @@ DependencyScanningFilesystemSharedCache::getShardForUID(
return CacheShards[Hash % NumShards];
}
+void DependencyScanningFilesystemSharedCache::diagnoseNegativeStatCachedPaths(
+ llvm::raw_ostream &OS, llvm::vfs::FileSystem &UnderlyingFS) const {
+ // Iterate through all shards and look for cached stat errors.
+ for (unsigned i = 0; i < NumShards; i++) {
+ const CacheShard &Shard = CacheShards[i];
+ for (const auto &Path : Shard.CacheByFilename.keys()) {
+ auto CachedPairIt = Shard.CacheByFilename.find(Path);
+ auto CachedPair = CachedPairIt->getValue();
+ const CachedFileSystemEntry *Entry = CachedPair.first;
+
+ if (Entry->getError()) {
+ // Only examine cached errors.
+ llvm::ErrorOr<llvm::vfs::Status> Stat = UnderlyingFS.status(Path);
+ if (Stat) {
+ // This is the case where we have negatively cached the non-existence
+ // of the file at Path, but the cache is later invalidated. Report
+ // diagnostics.
+ OS << Path << " did not exist when it was first searched "
+ << "but was created later. This may have led to a build failure "
+ "due to missing files.\n";
+ }
+ }
+ }
+ }
+}
+
const CachedFileSystemEntry *
DependencyScanningFilesystemSharedCache::CacheShard::findEntryByFilename(
StringRef Filename) const {
diff --git
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
index 111351fb90cee..02b192f1ba587 100644
---
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
+++
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
@@ -178,3 +178,31 @@ TEST(DependencyScanningFilesystem, CacheStatFailures) {
DepFS.exists("/cache/a.pcm");
EXPECT_EQ(InstrumentingFS->NumStatusCalls, 5u);
}
+
+TEST(DependencyScanningFilesystem, DiagnoseStaleStatFailures) {
+ auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+ InMemoryFS->setCurrentWorkingDirectory("/");
+ InMemoryFS->addFile("/dir", 0, llvm::MemoryBuffer::getMemBuffer(""));
+
+ DependencyScanningFilesystemSharedCache SharedCache;
+ DependencyScanningWorkerFilesystem DepFS(SharedCache, InMemoryFS);
+
+ bool Path1Exists = DepFS.exists("/path1");
+ EXPECT_EQ(Path1Exists, false);
+
+ // Adding a file that has been stat-ed,
+ InMemoryFS->addFile("/path1", 0, llvm::MemoryBuffer::getMemBuffer(""));
+ Path1Exists = DepFS.exists("/path1");
+ // Due to caching in SharedCache, path1 should not exist in
+ // DepFS's eyes.
+ EXPECT_EQ(Path1Exists, false);
+
+ std::string Diags;
+ llvm::raw_string_ostream DiagsStream(Diags);
+ SharedCache.diagnoseNegativeStatCachedPaths(DiagsStream, *InMemoryFS.get());
+
+ ASSERT_STREQ(
+ "/path1 did not exist when it was first searched but was created later. "
+ "This may have led to a build failure due to missing files.\n",
+ Diags.c_str());
+}
>From b1058390940f40c9f62e9bcf745b4381a40db5e5 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi...@apple.com>
Date: Tue, 15 Apr 2025 10:37:52 -0700
Subject: [PATCH 2/5] Address code review.
---
.../DependencyScanningFilesystem.cpp | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index fe72a1a91909e..e61d8afeb51bb 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -113,18 +113,17 @@ void
DependencyScanningFilesystemSharedCache::diagnoseNegativeStatCachedPaths(
// Iterate through all shards and look for cached stat errors.
for (unsigned i = 0; i < NumShards; i++) {
const CacheShard &Shard = CacheShards[i];
- for (const auto &Path : Shard.CacheByFilename.keys()) {
- auto CachedPairIt = Shard.CacheByFilename.find(Path);
- auto CachedPair = CachedPairIt->getValue();
+ std::lock_guard<std::mutex> LockGuard(Shard.CacheLock);
+ for (const auto &[Path, CachedPair] : Shard.CacheByFilename) {
const CachedFileSystemEntry *Entry = CachedPair.first;
if (Entry->getError()) {
// Only examine cached errors.
llvm::ErrorOr<llvm::vfs::Status> Stat = UnderlyingFS.status(Path);
if (Stat) {
- // This is the case where we have negatively cached the non-existence
- // of the file at Path, but the cache is later invalidated. Report
- // diagnostics.
+ // This is the case where we have cached the non-existence
+ // of the file at Path, but the file is created but the cache is
+ // not invalidated. Report diagnostics.
OS << Path << " did not exist when it was first searched "
<< "but was created later. This may have led to a build failure "
"due to missing files.\n";
>From 17a3426d04ca69c6c01b7240e9811946c00819bd Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi...@apple.com>
Date: Tue, 15 Apr 2025 14:34:17 -0700
Subject: [PATCH 3/5] Shorten the error message.
---
.../DependencyScanningFilesystem.cpp | 13 ++++++++++---
.../DependencyScanningFilesystemTest.cpp | 6 +++---
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index e61d8afeb51bb..bdd8695991f17 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -111,6 +111,7 @@ DependencyScanningFilesystemSharedCache::getShardForUID(
void DependencyScanningFilesystemSharedCache::diagnoseNegativeStatCachedPaths(
llvm::raw_ostream &OS, llvm::vfs::FileSystem &UnderlyingFS) const {
// Iterate through all shards and look for cached stat errors.
+ SmallVector<StringRef, 4> InvalidCachedPaths;
for (unsigned i = 0; i < NumShards; i++) {
const CacheShard &Shard = CacheShards[i];
std::lock_guard<std::mutex> LockGuard(Shard.CacheLock);
@@ -124,13 +125,19 @@ void
DependencyScanningFilesystemSharedCache::diagnoseNegativeStatCachedPaths(
// This is the case where we have cached the non-existence
// of the file at Path, but the file is created but the cache is
// not invalidated. Report diagnostics.
- OS << Path << " did not exist when it was first searched "
- << "but was created later. This may have led to a build failure "
- "due to missing files.\n";
+ InvalidCachedPaths.push_back(Path);
}
}
}
}
+
+ if (InvalidCachedPaths.size()) {
+ OS << "The following paths did not exist when they were first searched, "
+ "but files they point to were created later:\n";
+ for (const auto P : InvalidCachedPaths)
+ OS << "\t" << P << "\n";
+ OS << "Files missing at the paths above may have caused build errors.\n";
+ }
}
const CachedFileSystemEntry *
diff --git
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
index 02b192f1ba587..0e5c31f52cce2 100644
---
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
+++
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
@@ -182,7 +182,6 @@ TEST(DependencyScanningFilesystem, CacheStatFailures) {
TEST(DependencyScanningFilesystem, DiagnoseStaleStatFailures) {
auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
InMemoryFS->setCurrentWorkingDirectory("/");
- InMemoryFS->addFile("/dir", 0, llvm::MemoryBuffer::getMemBuffer(""));
DependencyScanningFilesystemSharedCache SharedCache;
DependencyScanningWorkerFilesystem DepFS(SharedCache, InMemoryFS);
@@ -202,7 +201,8 @@ TEST(DependencyScanningFilesystem,
DiagnoseStaleStatFailures) {
SharedCache.diagnoseNegativeStatCachedPaths(DiagsStream, *InMemoryFS.get());
ASSERT_STREQ(
- "/path1 did not exist when it was first searched but was created later. "
- "This may have led to a build failure due to missing files.\n",
+ "The following paths did not exist when they were first searched, but "
+ "files they point to were created later:\n\t/path1\nFiles missing at the
"
+ "paths above may have caused build errors.\n",
Diags.c_str());
}
>From 54b679ed6b00f37c1092e4f14fc9906221abe945 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi...@apple.com>
Date: Thu, 17 Apr 2025 13:20:28 -0700
Subject: [PATCH 4/5] Address code review.
---
.../DependencyScanningFilesystem.h | 15 +++++++------
.../DependencyScanningFilesystem.cpp | 22 +++++++------------
.../DependencyScanningFilesystemTest.cpp | 15 +++++--------
3 files changed, 22 insertions(+), 30 deletions(-)
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index 648158c5287c6..f19ad19013aa7 100644
---
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -220,13 +220,14 @@ class DependencyScanningFilesystemSharedCache {
CacheShard &getShardForFilename(StringRef Filename) const;
CacheShard &getShardForUID(llvm::sys::fs::UniqueID UID) const;
- /// Visits all cached entries and re-stat an entry using the UnderlyingFS if
- /// it is negatively stat cached. If the re-stat succeeds, print diagnostics
- /// information to OS indicating that negative stat caching has been
- /// invalidated.
- void
- diagnoseNegativeStatCachedPaths(llvm::raw_ostream &OS,
- llvm::vfs::FileSystem &UnderlyingFS) const;
+ /// Visits all cached entries and re-stat an entry using FS if
+ /// it is negatively stat cached. If re-stat succeeds on a path,
+ /// the path is added to InvalidPaths, indicating that the cache
+ /// may have erroneously negatively cached it. The caller can then
+ /// use InvalidPaths to issue diagnostics.
+ void diagnoseInvalidNegativeStatCachedPaths(
+ std::vector<std::string> &InvalidPaths,
+ llvm::vfs::FileSystem &UnderlyingFS) const;
private:
std::unique_ptr<CacheShard[]> CacheShards;
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index bdd8695991f17..8af6ad3265fa1 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -108,10 +108,11 @@ DependencyScanningFilesystemSharedCache::getShardForUID(
return CacheShards[Hash % NumShards];
}
-void DependencyScanningFilesystemSharedCache::diagnoseNegativeStatCachedPaths(
- llvm::raw_ostream &OS, llvm::vfs::FileSystem &UnderlyingFS) const {
+void DependencyScanningFilesystemSharedCache::
+ diagnoseInvalidNegativeStatCachedPaths(
+ std::vector<std::string> &InvalidPaths,
+ llvm::vfs::FileSystem &UnderlyingFS) const {
// Iterate through all shards and look for cached stat errors.
- SmallVector<StringRef, 4> InvalidCachedPaths;
for (unsigned i = 0; i < NumShards; i++) {
const CacheShard &Shard = CacheShards[i];
std::lock_guard<std::mutex> LockGuard(Shard.CacheLock);
@@ -123,21 +124,14 @@ void
DependencyScanningFilesystemSharedCache::diagnoseNegativeStatCachedPaths(
llvm::ErrorOr<llvm::vfs::Status> Stat = UnderlyingFS.status(Path);
if (Stat) {
// This is the case where we have cached the non-existence
- // of the file at Path, but the file is created but the cache is
- // not invalidated. Report diagnostics.
- InvalidCachedPaths.push_back(Path);
+ // of the file at Path first, and a a file at the path is created
+ // later. The cache entry is not invalidated (as we have no good
+ // way to do it now), which may lead to missing file build errors.
+ InvalidPaths.push_back(std::string(Path));
}
}
}
}
-
- if (InvalidCachedPaths.size()) {
- OS << "The following paths did not exist when they were first searched, "
- "but files they point to were created later:\n";
- for (const auto P : InvalidCachedPaths)
- OS << "\t" << P << "\n";
- OS << "Files missing at the paths above may have caused build errors.\n";
- }
}
const CachedFileSystemEntry *
diff --git
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
index 0e5c31f52cce2..4c537ee455e78 100644
---
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
+++
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
@@ -196,13 +196,10 @@ TEST(DependencyScanningFilesystem,
DiagnoseStaleStatFailures) {
// DepFS's eyes.
EXPECT_EQ(Path1Exists, false);
- std::string Diags;
- llvm::raw_string_ostream DiagsStream(Diags);
- SharedCache.diagnoseNegativeStatCachedPaths(DiagsStream, *InMemoryFS.get());
-
- ASSERT_STREQ(
- "The following paths did not exist when they were first searched, but "
- "files they point to were created later:\n\t/path1\nFiles missing at the
"
- "paths above may have caused build errors.\n",
- Diags.c_str());
+ std::vector<std::string> InvalidPaths;
+ SharedCache.diagnoseInvalidNegativeStatCachedPaths(InvalidPaths,
+ *InMemoryFS.get());
+
+ EXPECT_EQ(InvalidPaths.size(), 1u);
+ ASSERT_STREQ("/path1", InvalidPaths[0].c_str());
}
>From be6edb001fd2b5257a65efc581e9516a4bb939dc Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi...@apple.com>
Date: Thu, 17 Apr 2025 16:00:00 -0700
Subject: [PATCH 5/5] Address code review.
---
.../DependencyScanning/DependencyScanningFilesystem.h | 5 ++---
.../DependencyScanning/DependencyScanningFilesystem.cpp | 9 +++++----
.../DependencyScanningFilesystemTest.cpp | 7 +++----
3 files changed, 10 insertions(+), 11 deletions(-)
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index f19ad19013aa7..74b40f7452edb 100644
---
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -225,9 +225,8 @@ class DependencyScanningFilesystemSharedCache {
/// the path is added to InvalidPaths, indicating that the cache
/// may have erroneously negatively cached it. The caller can then
/// use InvalidPaths to issue diagnostics.
- void diagnoseInvalidNegativeStatCachedPaths(
- std::vector<std::string> &InvalidPaths,
- llvm::vfs::FileSystem &UnderlyingFS) const;
+ std::vector<StringRef>
+ getInvalidNegativeStatCachedPaths(llvm::vfs::FileSystem &UnderlyingFS) const;
private:
std::unique_ptr<CacheShard[]> CacheShards;
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 8af6ad3265fa1..da596a46f8240 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -108,11 +108,11 @@ DependencyScanningFilesystemSharedCache::getShardForUID(
return CacheShards[Hash % NumShards];
}
-void DependencyScanningFilesystemSharedCache::
- diagnoseInvalidNegativeStatCachedPaths(
- std::vector<std::string> &InvalidPaths,
- llvm::vfs::FileSystem &UnderlyingFS) const {
+std::vector<StringRef>
+DependencyScanningFilesystemSharedCache::getInvalidNegativeStatCachedPaths(
+ llvm::vfs::FileSystem &UnderlyingFS) const {
// Iterate through all shards and look for cached stat errors.
+ std::vector<StringRef> InvalidPaths;
for (unsigned i = 0; i < NumShards; i++) {
const CacheShard &Shard = CacheShards[i];
std::lock_guard<std::mutex> LockGuard(Shard.CacheLock);
@@ -132,6 +132,7 @@ void DependencyScanningFilesystemSharedCache::
}
}
}
+ return InvalidPaths;
}
const CachedFileSystemEntry *
diff --git
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
index 4c537ee455e78..5ea8d02353dc3 100644
---
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
+++
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
@@ -196,10 +196,9 @@ TEST(DependencyScanningFilesystem,
DiagnoseStaleStatFailures) {
// DepFS's eyes.
EXPECT_EQ(Path1Exists, false);
- std::vector<std::string> InvalidPaths;
- SharedCache.diagnoseInvalidNegativeStatCachedPaths(InvalidPaths,
- *InMemoryFS.get());
+ std::vector<llvm::StringRef> InvalidPaths =
+ SharedCache.getInvalidNegativeStatCachedPaths(*InMemoryFS.get());
EXPECT_EQ(InvalidPaths.size(), 1u);
- ASSERT_STREQ("/path1", InvalidPaths[0].c_str());
+ ASSERT_STREQ("/path1", InvalidPaths[0].str().c_str());
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
