Skip to content

Commit 885fbb7

Browse files
committed
[clang][Dependency Scanning] Report only Regular File Size Changes When Computing Out-of-Date File System Cache Entries (llvm#148082)
We noticed that when a directory's content changes, its size reported by `status` can change as well. We do not want to include such "false positive" cases. This PR revises the computation so that only regular file size changes are considered out-of-date. rdar://152247357 (cherry picked from commit 5c08bfa)
1 parent d20efe3 commit 885fbb7

File tree

2 files changed

+31
-1
lines changed

2 files changed

+31
-1
lines changed

clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ DependencyScanningFilesystemSharedCache::getOutOfDateEntries(
123123
std::lock_guard<std::mutex> LockGuard(Shard.CacheLock);
124124
for (const auto &[Path, CachedPair] : Shard.CacheByFilename) {
125125
const CachedFileSystemEntry *Entry = CachedPair.first;
126-
127126
llvm::ErrorOr<llvm::vfs::Status> Status = UnderlyingFS.status(Path);
128127
if (Status) {
129128
if (Entry->getError()) {

clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,3 +201,34 @@ TEST(DependencyScanningFilesystem, DiagnoseCachedFileSizeChange) {
201201
ASSERT_EQ(SizeInfo->CachedSize, 0u);
202202
ASSERT_EQ(SizeInfo->ActualSize, 8u);
203203
}
204+
205+
TEST(DependencyScanningFilesystem, DoNotDiagnoseDirSizeChange) {
206+
llvm::SmallString<128> Dir;
207+
ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("tmp", Dir));
208+
209+
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS =
210+
llvm::vfs::createPhysicalFileSystem();
211+
212+
DependencyScanningFilesystemSharedCache SharedCache;
213+
DependencyScanningWorkerFilesystem DepFS(SharedCache, FS);
214+
215+
// Trigger the file system cache.
216+
ASSERT_EQ(DepFS.exists(Dir), true);
217+
218+
// Add a file to the FS to change its size.
219+
// It seems that directory sizes reported are not meaningful,
220+
// and should not be used to check for size changes.
221+
// This test is setup only to trigger a size change so that we
222+
// know we are excluding directories from reporting.
223+
llvm::SmallString<128> FilePath = Dir;
224+
llvm::sys::path::append(FilePath, "file.h");
225+
{
226+
std::error_code EC;
227+
llvm::raw_fd_ostream TempFile(FilePath, EC);
228+
ASSERT_FALSE(EC);
229+
}
230+
231+
// We do not report directory size changes.
232+
auto InvalidEntries = SharedCache.getOutOfDateEntries(*FS);
233+
EXPECT_EQ(InvalidEntries.size(), 0u);
234+
}

0 commit comments

Comments
 (0)