diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp index 2437b2d3595e5..b641e4a0f0abb 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp @@ -117,7 +117,6 @@ DependencyScanningFilesystemSharedCache::getOutOfDateEntries( std::lock_guard LockGuard(Shard.CacheLock); for (const auto &[Path, CachedPair] : Shard.CacheByFilename) { const CachedFileSystemEntry *Entry = CachedPair.first; - llvm::ErrorOr Status = UnderlyingFS.status(Path); if (Status) { if (Entry->getError()) { @@ -128,12 +127,22 @@ DependencyScanningFilesystemSharedCache::getOutOfDateEntries( InvalidDiagInfo.emplace_back(Path.data()); } else { llvm::vfs::Status CachedStatus = Entry->getStatus(); - uint64_t CachedSize = CachedStatus.getSize(); - uint64_t ActualSize = Status->getSize(); - if (CachedSize != ActualSize) { - // This is the case where the cached file has a different size - // from the actual file that comes from the underlying FS. - InvalidDiagInfo.emplace_back(Path.data(), CachedSize, ActualSize); + if (Status->getType() == llvm::sys::fs::file_type::regular_file && + Status->getType() == CachedStatus.getType()) { + // We only check regular files. Directory files sizes could change + // due to content changes, and reporting directory size changes can + // lead to false positives. + // TODO: At the moment, we do not detect symlinks to files whose + // size may change. We need to decide if we want to detect cached + // symlink size changes. We can also expand this to detect file + // type changes. + uint64_t CachedSize = CachedStatus.getSize(); + uint64_t ActualSize = Status->getSize(); + if (CachedSize != ActualSize) { + // This is the case where the cached file has a different size + // from the actual file that comes from the underlying FS. + InvalidDiagInfo.emplace_back(Path.data(), CachedSize, ActualSize); + } } } } diff --git a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp index b461d9109271c..023c02ddaa3e4 100644 --- a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp +++ b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp @@ -233,3 +233,34 @@ TEST(DependencyScanningFilesystem, DiagnoseCachedFileSizeChange) { ASSERT_EQ(SizeInfo->CachedSize, 0u); ASSERT_EQ(SizeInfo->ActualSize, 8u); } + +TEST(DependencyScanningFilesystem, DoNotDiagnoseDirSizeChange) { + llvm::SmallString<128> Dir; + ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("tmp", Dir)); + + llvm::IntrusiveRefCntPtr FS = + llvm::vfs::createPhysicalFileSystem(); + + DependencyScanningFilesystemSharedCache SharedCache; + DependencyScanningWorkerFilesystem DepFS(SharedCache, FS); + + // Trigger the file system cache. + ASSERT_EQ(DepFS.exists(Dir), true); + + // Add a file to the FS to change its size. + // It seems that directory sizes reported are not meaningful, + // and should not be used to check for size changes. + // This test is setup only to trigger a size change so that we + // know we are excluding directories from reporting. + llvm::SmallString<128> FilePath = Dir; + llvm::sys::path::append(FilePath, "file.h"); + { + std::error_code EC; + llvm::raw_fd_ostream TempFile(FilePath, EC); + ASSERT_FALSE(EC); + } + + // We do not report directory size changes. + auto InvalidEntries = SharedCache.getOutOfDateEntries(*FS); + EXPECT_EQ(InvalidEntries.size(), 0u); +}