https://github.com/jansvoboda11 updated
https://github.com/llvm/llvm-project/pull/68645
>From fce5325720bcc945baed5923e00d09d84daf58e6 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Mon, 9 Oct 2023 10:14:17 -0700
Subject: [PATCH 01/14] [clang] Move lookup filename into function
---
.../DependencyScanningFilesystem.h | 4 ++
.../DependencyScanningFilesystem.cpp | 41 ++++++++++++-------
2 files changed, 30 insertions(+), 15 deletions(-)
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index 9a522a3e2fe252..4cd0f958fcff82 100644
---
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -402,6 +402,10 @@ class DependencyScanningWorkerFilesystem
llvm::ErrorOr<std::string> WorkingDirForCacheLookup;
void updateWorkingDirForCacheLookup();
+
+ llvm::ErrorOr<StringRef>
+ tryGetFilenameForLookup(StringRef OriginalFilename,
+ llvm::SmallVectorImpl<char> &PathBuf) const;
};
} // end namespace dependencies
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 9b7812a1adb9e3..c66780d50fa184 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -233,24 +233,15 @@ DependencyScanningWorkerFilesystem::computeAndStoreResult(
llvm::ErrorOr<EntryRef>
DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
StringRef OriginalFilename) {
- StringRef FilenameForLookup;
SmallString<256> PathBuf;
- if (llvm::sys::path::is_absolute_gnu(OriginalFilename)) {
- FilenameForLookup = OriginalFilename;
- } else if (!WorkingDirForCacheLookup) {
- return WorkingDirForCacheLookup.getError();
- } else {
- StringRef RelFilename = OriginalFilename;
- RelFilename.consume_front("./");
- PathBuf = *WorkingDirForCacheLookup;
- llvm::sys::path::append(PathBuf, RelFilename);
- FilenameForLookup = PathBuf.str();
- }
- assert(llvm::sys::path::is_absolute_gnu(FilenameForLookup));
+ auto FilenameForLookup = tryGetFilenameForLookup(OriginalFilename, PathBuf);
+ if (!FilenameForLookup)
+ return FilenameForLookup.getError();
+
if (const auto *Entry =
- findEntryByFilenameWithWriteThrough(FilenameForLookup))
+ findEntryByFilenameWithWriteThrough(*FilenameForLookup))
return EntryRef(OriginalFilename, *Entry).unwrapError();
- auto MaybeEntry = computeAndStoreResult(OriginalFilename, FilenameForLookup);
+ auto MaybeEntry = computeAndStoreResult(OriginalFilename,
*FilenameForLookup);
if (!MaybeEntry)
return MaybeEntry.getError();
return EntryRef(OriginalFilename, *MaybeEntry).unwrapError();
@@ -351,4 +342,24 @@ void
DependencyScanningWorkerFilesystem::updateWorkingDirForCacheLookup() {
llvm::sys::path::is_absolute_gnu(*WorkingDirForCacheLookup));
}
+llvm::ErrorOr<StringRef>
+DependencyScanningWorkerFilesystem::tryGetFilenameForLookup(
+ StringRef OriginalFilename, llvm::SmallVectorImpl<char> &PathBuf) const {
+ StringRef FilenameForLookup;
+ if (llvm::sys::path::is_absolute_gnu(OriginalFilename)) {
+ FilenameForLookup = OriginalFilename;
+ } else if (!WorkingDirForCacheLookup) {
+ return WorkingDirForCacheLookup.getError();
+ } else {
+ StringRef RelFilename = OriginalFilename;
+ RelFilename.consume_front("./");
+ PathBuf.assign(WorkingDirForCacheLookup->begin(),
+ WorkingDirForCacheLookup->end());
+ llvm::sys::path::append(PathBuf, RelFilename);
+ FilenameForLookup = StringRef{PathBuf.begin(), PathBuf.size()};
+ }
+ assert(llvm::sys::path::is_absolute_gnu(FilenameForLookup));
+ return FilenameForLookup;
+}
+
const char DependencyScanningWorkerFilesystem::ID = 0;
>From 548ce9b019eebd8ddfa40c6f097a7d1e7a464690 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Mon, 9 Oct 2023 10:14:22 -0700
Subject: [PATCH 02/14] [clang][deps] Cache `VFS::getRealPath()`
---
.../DependencyScanningFilesystem.h | 46 ++++++++++-
.../DependencyScanningFilesystem.cpp | 76 +++++++++++++++++++
2 files changed, 121 insertions(+), 1 deletion(-)
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index 4cd0f958fcff82..467b161fc2f0e3 100644
---
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -142,6 +142,8 @@ class CachedFileSystemEntry {
CachedFileContents *Contents;
};
+using CachedRealPath = llvm::ErrorOr<std::string>;
+
/// This class is a shared cache, that caches the 'stat' and 'open' calls to
the
/// underlying real file system, and the scanned preprocessor directives of
/// files.
@@ -168,6 +170,12 @@ class DependencyScanningFilesystemSharedCache {
/// The backing storage for cached contents.
llvm::SpecificBumpPtrAllocator<CachedFileContents> ContentsStorage;
+ /// Map from filenames to cached real paths.
+ llvm::StringMap<const CachedRealPath *> RealPathsByFilename;
+
+ /// The backing storage for cached real paths.
+ llvm::SpecificBumpPtrAllocator<CachedRealPath> RealPathStorage;
+
/// Returns entry associated with the filename or nullptr if none is found.
const CachedFileSystemEntry *findEntryByFilename(StringRef Filename) const;
@@ -194,6 +202,17 @@ class DependencyScanningFilesystemSharedCache {
const CachedFileSystemEntry &
getOrInsertEntryForFilename(StringRef Filename,
const CachedFileSystemEntry &Entry);
+
+ /// Returns real path associated with the filename or nullptr if none is
+ /// found.
+ const CachedRealPath *findRealPathByFilename(StringRef Filename) const;
+
+ /// Returns real path associated with the filename if there is some.
+ /// Otherwise, constructs new one with the given one, associates it with
the
+ /// filename and returns the result.
+ const CachedRealPath &
+ getOrEmplaceRealPathForFilename(StringRef Filename,
+ llvm::ErrorOr<StringRef> RealPath);
};
DependencyScanningFilesystemSharedCache();
@@ -212,6 +231,8 @@ class DependencyScanningFilesystemSharedCache {
class DependencyScanningFilesystemLocalCache {
llvm::StringMap<const CachedFileSystemEntry *, llvm::BumpPtrAllocator> Cache;
+ llvm::StringMap<const CachedRealPath *, llvm::BumpPtrAllocator>
RealPathCache;
+
public:
/// Returns entry associated with the filename or nullptr if none is found.
const CachedFileSystemEntry *findEntryByFilename(StringRef Filename) const {
@@ -230,6 +251,26 @@ class DependencyScanningFilesystemLocalCache {
assert(InsertedEntry == &Entry && "entry already present");
return *InsertedEntry;
}
+
+ /// Returns real path associated with the filename or nullptr if none is
+ /// found.
+ const CachedRealPath *findRealPathByFilename(StringRef Filename) const {
+ assert(llvm::sys::path::is_absolute_gnu(Filename));
+ auto It = RealPathCache.find(Filename);
+ return It == RealPathCache.end() ? nullptr : It->getValue();
+ }
+
+ /// Associates the given real path with the filename and returns the given
+ /// entry pointer (for convenience).
+ const CachedRealPath &
+ insertRealPathForFilename(StringRef Filename,
+ const CachedRealPath &RealPath) {
+ assert(llvm::sys::path::is_absolute_gnu(Filename));
+ const auto *InsertedRealPath =
+ RealPathCache.insert({Filename, &RealPath}).first->second;
+ assert(InsertedRealPath == &RealPath && "entry already present");
+ return *InsertedRealPath;
+ }
};
/// Reference to a CachedFileSystemEntry.
@@ -296,6 +337,9 @@ class DependencyScanningWorkerFilesystem
llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
openFileForRead(const Twine &Path) override;
+ std::error_code getRealPath(const Twine &Path,
+ SmallVectorImpl<char> &Output) const override;
+
std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
/// Returns entry for the given filename.
@@ -395,7 +439,7 @@ class DependencyScanningWorkerFilesystem
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;
+ mutable DependencyScanningFilesystemLocalCache LocalCache;
/// The working directory to use for making relative paths absolute before
/// using them for cache lookups.
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index c66780d50fa184..b44db58745d62c 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -162,6 +162,35 @@ DependencyScanningFilesystemSharedCache::CacheShard::
return *EntriesByFilename.insert({Filename, &Entry}).first->getValue();
}
+const CachedRealPath *
+DependencyScanningFilesystemSharedCache::CacheShard::findRealPathByFilename(
+ StringRef Filename) const {
+ assert(llvm::sys::path::is_absolute_gnu(Filename));
+ std::lock_guard<std::mutex> LockGuard(CacheLock);
+ auto It = RealPathsByFilename.find(Filename);
+ return It == RealPathsByFilename.end() ? nullptr : It->getValue();
+}
+
+const CachedRealPath &DependencyScanningFilesystemSharedCache::CacheShard::
+ getOrEmplaceRealPathForFilename(StringRef Filename,
+ llvm::ErrorOr<llvm::StringRef> RealPath) {
+ std::lock_guard<std::mutex> LockGuard(CacheLock);
+
+ auto Insertion = RealPathsByFilename.insert({Filename, nullptr});
+ if (Insertion.second) {
+ auto OwnedRealPath = [&]() -> CachedRealPath {
+ if (!RealPath)
+ return RealPath.getError();
+ return RealPath->str();
+ }();
+
+ Insertion.first->second = new (RealPathStorage.Allocate())
+ CachedRealPath(std::move(OwnedRealPath));
+ }
+
+ return *Insertion.first->second;
+}
+
static bool shouldCacheStatFailures(StringRef Filename) {
StringRef Ext = llvm::sys::path::extension(Filename);
if (Ext.empty())
@@ -321,6 +350,53 @@ DependencyScanningWorkerFilesystem::openFileForRead(const
Twine &Path) {
return DepScanFile::create(Result.get());
}
+std::error_code DependencyScanningWorkerFilesystem::getRealPath(
+ const Twine &Path, SmallVectorImpl<char> &Output) const {
+ SmallString<256> OwnedFilename;
+ StringRef OriginalFilename = Path.toStringRef(OwnedFilename);
+
+ SmallString<256> PathBuf;
+ auto FilenameForLookup = tryGetFilenameForLookup(OriginalFilename, PathBuf);
+ if (!FilenameForLookup)
+ return FilenameForLookup.getError();
+
+ auto HandleCachedRealPath =
+ [&Output](const CachedRealPath &RealPath) -> std::error_code {
+ if (!RealPath)
+ return RealPath.getError();
+ Output.assign(RealPath->begin(), RealPath->end());
+ return {};
+ };
+
+ // If we already have the result in local cache, no work required.
+ if (const auto *RealPath =
+ LocalCache.findRealPathByFilename(*FilenameForLookup))
+ return HandleCachedRealPath(*RealPath);
+
+ // If we have the result in the shared cache, cache it locally.
+ auto &Shard = SharedCache.getShardForFilename(*FilenameForLookup);
+ if (const auto *ShardRealPath =
+ Shard.findRealPathByFilename(*FilenameForLookup)) {
+ const auto &RealPath =
+ LocalCache.insertRealPathForFilename(*FilenameForLookup,
*ShardRealPath);
+ return HandleCachedRealPath(RealPath);
+ }
+
+ // If we don't know the real path, compute it...
+ std::error_code EC = getUnderlyingFS().getRealPath(OriginalFilename, Output);
+ llvm::ErrorOr<llvm::StringRef> ComputedRealPath = EC;
+ if (!EC)
+ ComputedRealPath = StringRef{Output.data(), Output.size()};
+
+ // ...and try to write it into the shared cache. In case some other thread
won
+ // this race and already wrote its own result there, just adopt it. Write
+ // whatever is in the shared cache into the local one.
+ const auto &RealPath = Shard.getOrEmplaceRealPathForFilename(
+ *FilenameForLookup, ComputedRealPath);
+ return HandleCachedRealPath(
+ LocalCache.insertRealPathForFilename(*FilenameForLookup, RealPath));
+}
+
std::error_code DependencyScanningWorkerFilesystem::setCurrentWorkingDirectory(
const Twine &Path) {
std::error_code EC = ProxyFileSystem::setCurrentWorkingDirectory(Path);
>From e36bb849c84f99551b4fd19d8ac574d0f4c64256 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Fri, 13 Oct 2023 13:26:28 -0700
Subject: [PATCH 03/14] Squash real path cache with the existing cache
---
.../DependencyScanningFilesystem.h | 37 +++++++++----------
.../DependencyScanningFilesystem.cpp | 27 +++++++-------
2 files changed, 32 insertions(+), 32 deletions(-)
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index 467b161fc2f0e3..438081cac0fb8f 100644
---
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -156,9 +156,11 @@ class DependencyScanningFilesystemSharedCache {
/// The mutex that needs to be locked before mutation of any member.
mutable std::mutex CacheLock;
- /// Map from filenames to cached entries.
- llvm::StringMap<const CachedFileSystemEntry *, llvm::BumpPtrAllocator>
- EntriesByFilename;
+ /// Map from filenames to cached entries and real paths.
+ llvm::StringMap<
+ std::pair<const CachedFileSystemEntry *, const CachedRealPath *>,
+ llvm::BumpPtrAllocator>
+ CacheByFilename;
/// Map from unique IDs to cached entries.
llvm::DenseMap<llvm::sys::fs::UniqueID, const CachedFileSystemEntry *>
@@ -170,9 +172,6 @@ class DependencyScanningFilesystemSharedCache {
/// The backing storage for cached contents.
llvm::SpecificBumpPtrAllocator<CachedFileContents> ContentsStorage;
- /// Map from filenames to cached real paths.
- llvm::StringMap<const CachedRealPath *> RealPathsByFilename;
-
/// The backing storage for cached real paths.
llvm::SpecificBumpPtrAllocator<CachedRealPath> RealPathStorage;
@@ -229,16 +228,17 @@ class DependencyScanningFilesystemSharedCache {
/// This class is a local cache, that caches the 'stat' and 'open' calls to the
/// underlying real file system.
class DependencyScanningFilesystemLocalCache {
- llvm::StringMap<const CachedFileSystemEntry *, llvm::BumpPtrAllocator> Cache;
-
- llvm::StringMap<const CachedRealPath *, llvm::BumpPtrAllocator>
RealPathCache;
+ llvm::StringMap<
+ std::pair<const CachedFileSystemEntry *, const CachedRealPath *>,
+ llvm::BumpPtrAllocator>
+ Cache;
public:
/// Returns entry associated with the filename or nullptr if none is found.
const CachedFileSystemEntry *findEntryByFilename(StringRef Filename) const {
assert(llvm::sys::path::is_absolute_gnu(Filename));
auto It = Cache.find(Filename);
- return It == Cache.end() ? nullptr : It->getValue();
+ return It == Cache.end() ? nullptr : It->getValue().first;
}
/// Associates the given entry with the filename and returns the given entry
@@ -247,17 +247,17 @@ class DependencyScanningFilesystemLocalCache {
insertEntryForFilename(StringRef Filename,
const CachedFileSystemEntry &Entry) {
assert(llvm::sys::path::is_absolute_gnu(Filename));
- const auto *InsertedEntry = Cache.insert({Filename, &Entry}).first->second;
- assert(InsertedEntry == &Entry && "entry already present");
- return *InsertedEntry;
+ assert(Cache[Filename].first == nullptr && "entry already present");
+ Cache[Filename].first = &Entry;
+ return Entry;
}
/// Returns real path associated with the filename or nullptr if none is
/// found.
const CachedRealPath *findRealPathByFilename(StringRef Filename) const {
assert(llvm::sys::path::is_absolute_gnu(Filename));
- auto It = RealPathCache.find(Filename);
- return It == RealPathCache.end() ? nullptr : It->getValue();
+ auto It = Cache.find(Filename);
+ return It == Cache.end() ? nullptr : It->getValue().second;
}
/// Associates the given real path with the filename and returns the given
@@ -266,10 +266,9 @@ class DependencyScanningFilesystemLocalCache {
insertRealPathForFilename(StringRef Filename,
const CachedRealPath &RealPath) {
assert(llvm::sys::path::is_absolute_gnu(Filename));
- const auto *InsertedRealPath =
- RealPathCache.insert({Filename, &RealPath}).first->second;
- assert(InsertedRealPath == &RealPath && "entry already present");
- return *InsertedRealPath;
+ assert(Cache[Filename].second == nullptr && "entry already present");
+ Cache[Filename].second = &RealPath;
+ return RealPath;
}
};
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index b44db58745d62c..204c0edb9b5eb2 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -113,8 +113,8 @@
DependencyScanningFilesystemSharedCache::CacheShard::findEntryByFilename(
StringRef Filename) const {
assert(llvm::sys::path::is_absolute_gnu(Filename));
std::lock_guard<std::mutex> LockGuard(CacheLock);
- auto It = EntriesByFilename.find(Filename);
- return It == EntriesByFilename.end() ? nullptr : It->getValue();
+ auto It = CacheByFilename.find(Filename);
+ return It == CacheByFilename.end() ? nullptr : It->getValue().first;
}
const CachedFileSystemEntry *
@@ -130,11 +130,11 @@ DependencyScanningFilesystemSharedCache::CacheShard::
getOrEmplaceEntryForFilename(StringRef Filename,
llvm::ErrorOr<llvm::vfs::Status> Stat) {
std::lock_guard<std::mutex> LockGuard(CacheLock);
- auto Insertion = EntriesByFilename.insert({Filename, nullptr});
- if (Insertion.second)
- Insertion.first->second =
+ const CachedFileSystemEntry *StoredEntry = CacheByFilename[Filename].first;
+ if (!StoredEntry)
+ StoredEntry =
new (EntryStorage.Allocate()) CachedFileSystemEntry(std::move(Stat));
- return *Insertion.first->second;
+ return *StoredEntry;
}
const CachedFileSystemEntry &
@@ -159,7 +159,8 @@ DependencyScanningFilesystemSharedCache::CacheShard::
getOrInsertEntryForFilename(StringRef Filename,
const CachedFileSystemEntry &Entry) {
std::lock_guard<std::mutex> LockGuard(CacheLock);
- return *EntriesByFilename.insert({Filename, &Entry}).first->getValue();
+ CacheByFilename[Filename].first = &Entry;
+ return Entry;
}
const CachedRealPath *
@@ -167,8 +168,8 @@
DependencyScanningFilesystemSharedCache::CacheShard::findRealPathByFilename(
StringRef Filename) const {
assert(llvm::sys::path::is_absolute_gnu(Filename));
std::lock_guard<std::mutex> LockGuard(CacheLock);
- auto It = RealPathsByFilename.find(Filename);
- return It == RealPathsByFilename.end() ? nullptr : It->getValue();
+ auto It = CacheByFilename.find(Filename);
+ return It == CacheByFilename.end() ? nullptr : It->getValue().second;
}
const CachedRealPath &DependencyScanningFilesystemSharedCache::CacheShard::
@@ -176,19 +177,19 @@ const CachedRealPath
&DependencyScanningFilesystemSharedCache::CacheShard::
llvm::ErrorOr<llvm::StringRef> RealPath) {
std::lock_guard<std::mutex> LockGuard(CacheLock);
- auto Insertion = RealPathsByFilename.insert({Filename, nullptr});
- if (Insertion.second) {
+ const CachedRealPath *StoredRealPath = CacheByFilename[Filename].second;
+ if (!StoredRealPath) {
auto OwnedRealPath = [&]() -> CachedRealPath {
if (!RealPath)
return RealPath.getError();
return RealPath->str();
}();
- Insertion.first->second = new (RealPathStorage.Allocate())
+ StoredRealPath = new (RealPathStorage.Allocate())
CachedRealPath(std::move(OwnedRealPath));
}
- return *Insertion.first->second;
+ return *StoredRealPath;
}
static bool shouldCacheStatFailures(StringRef Filename) {
>From fd1f9674532480cdbd42a61af671ed10f9a5eff4 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Wed, 10 Apr 2024 10:21:42 -0700
Subject: [PATCH 04/14] Make `getRealPath()` non-const
---
.../DependencyScanningFilesystem.h | 4 ++--
.../DependencyScanningFilesystem.cpp | 5 +++--
llvm/include/llvm/Support/VirtualFileSystem.h | 10 ++++-----
llvm/lib/Support/FileCollector.cpp | 2 +-
llvm/lib/Support/VirtualFileSystem.cpp | 21 ++++++++-----------
.../Support/VirtualFileSystemTest.cpp | 2 +-
6 files changed, 21 insertions(+), 23 deletions(-)
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index 438081cac0fb8f..68890039e953a8 100644
---
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -337,7 +337,7 @@ class DependencyScanningWorkerFilesystem
openFileForRead(const Twine &Path) override;
std::error_code getRealPath(const Twine &Path,
- SmallVectorImpl<char> &Output) const override;
+ SmallVectorImpl<char> &Output) override;
std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
@@ -438,7 +438,7 @@ class DependencyScanningWorkerFilesystem
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.
- mutable DependencyScanningFilesystemLocalCache LocalCache;
+ DependencyScanningFilesystemLocalCache LocalCache;
/// The working directory to use for making relative paths absolute before
/// using them for cache lookups.
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 204c0edb9b5eb2..374fad52be92a1 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -351,8 +351,9 @@ DependencyScanningWorkerFilesystem::openFileForRead(const
Twine &Path) {
return DepScanFile::create(Result.get());
}
-std::error_code DependencyScanningWorkerFilesystem::getRealPath(
- const Twine &Path, SmallVectorImpl<char> &Output) const {
+std::error_code
+DependencyScanningWorkerFilesystem::getRealPath(const Twine &Path,
+ SmallVectorImpl<char> &Output)
{
SmallString<256> OwnedFilename;
StringRef OriginalFilename = Path.toStringRef(OwnedFilename);
diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h
b/llvm/include/llvm/Support/VirtualFileSystem.h
index 770ca8764426a4..4b1ca0c3d262b6 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -298,7 +298,7 @@ class FileSystem : public
llvm::ThreadSafeRefCountedBase<FileSystem>,
/// symlinks. For real file system, this uses `llvm::sys::fs::real_path`.
/// This returns errc::operation_not_permitted if not implemented by
subclass.
virtual std::error_code getRealPath(const Twine &Path,
- SmallVectorImpl<char> &Output) const;
+ SmallVectorImpl<char> &Output);
/// Check whether a file exists. Provided for convenience.
bool exists(const Twine &Path);
@@ -393,7 +393,7 @@ class OverlayFileSystem : public
RTTIExtends<OverlayFileSystem, FileSystem> {
std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
std::error_code isLocal(const Twine &Path, bool &Result) override;
std::error_code getRealPath(const Twine &Path,
- SmallVectorImpl<char> &Output) const override;
+ SmallVectorImpl<char> &Output) override;
using iterator = FileSystemList::reverse_iterator;
using const_iterator = FileSystemList::const_reverse_iterator;
@@ -453,7 +453,7 @@ class ProxyFileSystem : public RTTIExtends<ProxyFileSystem,
FileSystem> {
return FS->setCurrentWorkingDirectory(Path);
}
std::error_code getRealPath(const Twine &Path,
- SmallVectorImpl<char> &Output) const override {
+ SmallVectorImpl<char> &Output) override {
return FS->getRealPath(Path, Output);
}
std::error_code isLocal(const Twine &Path, bool &Result) override {
@@ -622,7 +622,7 @@ class InMemoryFileSystem : public
RTTIExtends<InMemoryFileSystem, FileSystem> {
/// This doesn't resolve symlinks as they are not supported in in-memory file
/// system.
std::error_code getRealPath(const Twine &Path,
- SmallVectorImpl<char> &Output) const override;
+ SmallVectorImpl<char> &Output) override;
std::error_code isLocal(const Twine &Path, bool &Result) override;
std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
@@ -1047,7 +1047,7 @@ class RedirectingFileSystem
ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path) override;
std::error_code getRealPath(const Twine &Path,
- SmallVectorImpl<char> &Output) const override;
+ SmallVectorImpl<char> &Output) override;
llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override;
diff --git a/llvm/lib/Support/FileCollector.cpp
b/llvm/lib/Support/FileCollector.cpp
index 92bcdf00b8a82a..be0b06b0aff805 100644
--- a/llvm/lib/Support/FileCollector.cpp
+++ b/llvm/lib/Support/FileCollector.cpp
@@ -281,7 +281,7 @@ class FileCollectorFileSystem : public vfs::FileSystem {
}
std::error_code getRealPath(const Twine &Path,
- SmallVectorImpl<char> &Output) const override {
+ SmallVectorImpl<char> &Output) override {
auto EC = FS->getRealPath(Path, Output);
if (!EC) {
Collector->addFile(Path);
diff --git a/llvm/lib/Support/VirtualFileSystem.cpp
b/llvm/lib/Support/VirtualFileSystem.cpp
index 057f8eae0552c6..32b480028e71b4 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -138,7 +138,7 @@ std::error_code
FileSystem::makeAbsolute(SmallVectorImpl<char> &Path) const {
}
std::error_code FileSystem::getRealPath(const Twine &Path,
- SmallVectorImpl<char> &Output) const {
+ SmallVectorImpl<char> &Output) {
return errc::operation_not_permitted;
}
@@ -275,7 +275,7 @@ class RealFileSystem : public FileSystem {
std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
std::error_code isLocal(const Twine &Path, bool &Result) override;
std::error_code getRealPath(const Twine &Path,
- SmallVectorImpl<char> &Output) const override;
+ SmallVectorImpl<char> &Output) override;
protected:
void printImpl(raw_ostream &OS, PrintType Type,
@@ -357,9 +357,8 @@ std::error_code RealFileSystem::isLocal(const Twine &Path,
bool &Result) {
return llvm::sys::fs::is_local(adjustPath(Path, Storage), Result);
}
-std::error_code
-RealFileSystem::getRealPath(const Twine &Path,
- SmallVectorImpl<char> &Output) const {
+std::error_code RealFileSystem::getRealPath(const Twine &Path,
+ SmallVectorImpl<char> &Output) {
SmallString<256> Storage;
return llvm::sys::fs::real_path(adjustPath(Path, Storage), Output);
}
@@ -471,9 +470,8 @@ std::error_code OverlayFileSystem::isLocal(const Twine
&Path, bool &Result) {
return errc::no_such_file_or_directory;
}
-std::error_code
-OverlayFileSystem::getRealPath(const Twine &Path,
- SmallVectorImpl<char> &Output) const {
+std::error_code OverlayFileSystem::getRealPath(const Twine &Path,
+ SmallVectorImpl<char> &Output) {
for (const auto &FS : FSList)
if (FS->exists(Path))
return FS->getRealPath(Path, Output);
@@ -1157,9 +1155,8 @@ std::error_code
InMemoryFileSystem::setCurrentWorkingDirectory(const Twine &P) {
return {};
}
-std::error_code
-InMemoryFileSystem::getRealPath(const Twine &Path,
- SmallVectorImpl<char> &Output) const {
+std::error_code InMemoryFileSystem::getRealPath(const Twine &Path,
+ SmallVectorImpl<char> &Output)
{
auto CWD = getCurrentWorkingDirectory();
if (!CWD || CWD->empty())
return errc::operation_not_permitted;
@@ -2535,7 +2532,7 @@ RedirectingFileSystem::openFileForRead(const Twine
&OriginalPath) {
std::error_code
RedirectingFileSystem::getRealPath(const Twine &OriginalPath,
- SmallVectorImpl<char> &Output) const {
+ SmallVectorImpl<char> &Output) {
SmallString<256> Path;
OriginalPath.toVector(Path);
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp
b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index 695b09343257f1..49a2e19e4f74cd 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -82,7 +82,7 @@ class DummyFileSystem : public vfs::FileSystem {
}
// Map any symlink to "/symlink".
std::error_code getRealPath(const Twine &Path,
- SmallVectorImpl<char> &Output) const override {
+ SmallVectorImpl<char> &Output) override {
auto I = findEntry(Path);
if (I == FilesAndDirs.end())
return make_error_code(llvm::errc::no_such_file_or_directory);
>From 6ad469598dff25a8db9645496a9933f994663ede Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Wed, 10 Apr 2024 10:50:14 -0700
Subject: [PATCH 05/14] Add test
---
clang/unittests/Tooling/CMakeLists.txt | 3 +-
.../DependencyScannerTest.cpp | 2 +-
.../DependencyScanningFilesystemTest.cpp | 64 +++++++++++++++++++
3 files changed, 67 insertions(+), 2 deletions(-)
rename clang/unittests/Tooling/{ =>
DependencyScanning}/DependencyScannerTest.cpp (99%)
create mode 100644
clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
diff --git a/clang/unittests/Tooling/CMakeLists.txt
b/clang/unittests/Tooling/CMakeLists.txt
index 5a10a6b285390e..0eb612f8d94987 100644
--- a/clang/unittests/Tooling/CMakeLists.txt
+++ b/clang/unittests/Tooling/CMakeLists.txt
@@ -13,7 +13,6 @@ add_clang_unittest(ToolingTests
CastExprTest.cpp
CommentHandlerTest.cpp
CompilationDatabaseTest.cpp
- DependencyScannerTest.cpp
DiagnosticsYamlTest.cpp
ExecutionTest.cpp
FixItTest.cpp
@@ -24,6 +23,8 @@ add_clang_unittest(ToolingTests
LookupTest.cpp
QualTypeNamesTest.cpp
RangeSelectorTest.cpp
+ DependencyScanning/DependencyScannerTest.cpp
+ DependencyScanning/DependencyScanningFilesystemTest.cpp
RecursiveASTVisitorTests/Attr.cpp
RecursiveASTVisitorTests/BitfieldInitializer.cpp
RecursiveASTVisitorTests/CallbacksLeaf.cpp
diff --git a/clang/unittests/Tooling/DependencyScannerTest.cpp
b/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp
similarity index 99%
rename from clang/unittests/Tooling/DependencyScannerTest.cpp
rename to clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp
index 8735fcad200461..ec0e143be4a209 100644
--- a/clang/unittests/Tooling/DependencyScannerTest.cpp
+++ b/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp
@@ -1,4 +1,4 @@
-//===- unittest/Tooling/ToolingTest.cpp - Tooling unit tests
--------------===//
+//===- DependencyScannerTest.cpp
------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
diff --git
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
new file mode 100644
index 00000000000000..4355decb0afa8a
--- /dev/null
+++
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
@@ -0,0 +1,64 @@
+//===- DependencyScanningFilesystemTest.cpp
-------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "gtest/gtest.h"
+
+using namespace clang::tooling::dependencies;
+
+namespace {
+struct InstrumentingFilesystem
+ : llvm::RTTIExtends<InstrumentingFilesystem, llvm::vfs::ProxyFileSystem> {
+ unsigned NumGetRealPathCalls = 0;
+
+ using llvm::RTTIExtends<InstrumentingFilesystem,
+ llvm::vfs::ProxyFileSystem>::RTTIExtends;
+
+ std::error_code getRealPath(const llvm::Twine &Path,
+ llvm::SmallVectorImpl<char> &Output) override {
+ ++NumGetRealPathCalls;
+ return ProxyFileSystem::getRealPath(Path, Output);
+ }
+};
+} // namespace
+
+TEST(DependencyScanningFilesystem, CacheGetRealPath) {
+ auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+ InMemoryFS->setCurrentWorkingDirectory("/");
+ InMemoryFS->addFile("/foo", 0, llvm::MemoryBuffer::getMemBuffer(""));
+ InMemoryFS->addFile("/bar", 0, llvm::MemoryBuffer::getMemBuffer(""));
+
+ auto InstrumentingFS =
+ llvm::makeIntrusiveRefCnt<InstrumentingFilesystem>(InMemoryFS);
+
+ DependencyScanningFilesystemSharedCache SharedCache;
+ DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
+
+ {
+ llvm::SmallString<128> Result;
+ DepFS.getRealPath("/foo", Result);
+ EXPECT_EQ(Result, "/foo");
+ EXPECT_EQ(InstrumentingFS->NumGetRealPathCalls, 1u);
+ }
+
+ {
+ llvm::SmallString<128> Result;
+ DepFS.getRealPath("/foo", Result);
+ EXPECT_EQ(Result, "/foo");
+ EXPECT_EQ(InstrumentingFS->NumGetRealPathCalls, 1u); // Cached, no
increase.
+ }
+
+ {
+ llvm::SmallString<128> Result;
+ DepFS.getRealPath("/bar", Result);
+ EXPECT_EQ(Result, "/bar");
+ EXPECT_EQ(InstrumentingFS->NumGetRealPathCalls, 2u);
+ }
+}
>From b172ba22b82694c3cc17e77e03acc73c733b6a55 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Wed, 10 Apr 2024 11:01:14 -0700
Subject: [PATCH 06/14] Formatting
---
.../DependencyScanning/DependencyScanningFilesystem.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 374fad52be92a1..6634e6d85f329b 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -379,8 +379,8 @@ DependencyScanningWorkerFilesystem::getRealPath(const Twine
&Path,
auto &Shard = SharedCache.getShardForFilename(*FilenameForLookup);
if (const auto *ShardRealPath =
Shard.findRealPathByFilename(*FilenameForLookup)) {
- const auto &RealPath =
- LocalCache.insertRealPathForFilename(*FilenameForLookup,
*ShardRealPath);
+ const auto &RealPath = LocalCache.insertRealPathForFilename(
+ *FilenameForLookup, *ShardRealPath);
return HandleCachedRealPath(RealPath);
}
>From 91ee74e8e87c47fa41ae32f2731f1d50988da6fd Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Wed, 10 Apr 2024 11:26:05 -0700
Subject: [PATCH 07/14] Fix propagation to shared cache
---
.../DependencyScanning/DependencyScanningFilesystem.cpp | 2 +-
.../DependencyScanningFilesystemTest.cpp | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 6634e6d85f329b..fe2f178def60be 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -177,7 +177,7 @@ const CachedRealPath
&DependencyScanningFilesystemSharedCache::CacheShard::
llvm::ErrorOr<llvm::StringRef> RealPath) {
std::lock_guard<std::mutex> LockGuard(CacheLock);
- const CachedRealPath *StoredRealPath = CacheByFilename[Filename].second;
+ const CachedRealPath *&StoredRealPath = CacheByFilename[Filename].second;
if (!StoredRealPath) {
auto OwnedRealPath = [&]() -> CachedRealPath {
if (!RealPath)
diff --git
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
index 4355decb0afa8a..e9cb92fd7c9172 100644
---
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
+++
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
@@ -40,6 +40,7 @@ TEST(DependencyScanningFilesystem, CacheGetRealPath) {
DependencyScanningFilesystemSharedCache SharedCache;
DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
+ DependencyScanningWorkerFilesystem DepFS2(SharedCache, InstrumentingFS);
{
llvm::SmallString<128> Result;
@@ -61,4 +62,11 @@ TEST(DependencyScanningFilesystem, CacheGetRealPath) {
EXPECT_EQ(Result, "/bar");
EXPECT_EQ(InstrumentingFS->NumGetRealPathCalls, 2u);
}
+
+ {
+ llvm::SmallString<128> Result;
+ DepFS2.getRealPath("/foo", Result);
+ EXPECT_EQ(Result, "/foo");
+ EXPECT_EQ(InstrumentingFS->NumGetRealPathCalls, 2u); // Shared cache.
+ }
}
>From 48ef2e77db9ed7ab07a53018c8ed6336f6019356 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Wed, 10 Apr 2024 16:33:35 -0700
Subject: [PATCH 08/14] Remove unnecessary Result checks that fail on Windows
---
.../DependencyScanning/DependencyScanningFilesystemTest.cpp | 4 ----
1 file changed, 4 deletions(-)
diff --git
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
index e9cb92fd7c9172..408e7874ee4f83 100644
---
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
+++
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
@@ -45,28 +45,24 @@ TEST(DependencyScanningFilesystem, CacheGetRealPath) {
{
llvm::SmallString<128> Result;
DepFS.getRealPath("/foo", Result);
- EXPECT_EQ(Result, "/foo");
EXPECT_EQ(InstrumentingFS->NumGetRealPathCalls, 1u);
}
{
llvm::SmallString<128> Result;
DepFS.getRealPath("/foo", Result);
- EXPECT_EQ(Result, "/foo");
EXPECT_EQ(InstrumentingFS->NumGetRealPathCalls, 1u); // Cached, no
increase.
}
{
llvm::SmallString<128> Result;
DepFS.getRealPath("/bar", Result);
- EXPECT_EQ(Result, "/bar");
EXPECT_EQ(InstrumentingFS->NumGetRealPathCalls, 2u);
}
{
llvm::SmallString<128> Result;
DepFS2.getRealPath("/foo", Result);
- EXPECT_EQ(Result, "/foo");
EXPECT_EQ(InstrumentingFS->NumGetRealPathCalls, 2u); // Shared cache.
}
}
>From 2412cc4d87cb34d29539b86b0dd91dc3fb25f5de Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Thu, 11 Apr 2024 13:28:58 -0700
Subject: [PATCH 09/14] Add "the" to the doc comment
---
.../Tooling/DependencyScanning/DependencyScanningFilesystem.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index 68890039e953a8..3b1bf03e113130 100644
---
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -202,11 +202,11 @@ class DependencyScanningFilesystemSharedCache {
getOrInsertEntryForFilename(StringRef Filename,
const CachedFileSystemEntry &Entry);
- /// Returns real path associated with the filename or nullptr if none is
+ /// Returns the real path associated with the filename or nullptr if none
is
/// found.
const CachedRealPath *findRealPathByFilename(StringRef Filename) const;
- /// Returns real path associated with the filename if there is some.
+ /// Returns the real path associated with the filename if there is some.
/// Otherwise, constructs new one with the given one, associates it with
the
/// filename and returns the result.
const CachedRealPath &
>From bd4fb4287a375f48633d0d1b8ba4cec867bddcea Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Thu, 11 Apr 2024 13:29:36 -0700
Subject: [PATCH 10/14] Remove double lookup in local cache for assert builds
---
.../DependencyScanningFilesystem.h | 24 ++++++++++++++-----
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index 3b1bf03e113130..8b6f149c7cb266 100644
---
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -247,9 +247,15 @@ class DependencyScanningFilesystemLocalCache {
insertEntryForFilename(StringRef Filename,
const CachedFileSystemEntry &Entry) {
assert(llvm::sys::path::is_absolute_gnu(Filename));
- assert(Cache[Filename].first == nullptr && "entry already present");
- Cache[Filename].first = &Entry;
- return Entry;
+ auto [It, Inserted] = Cache.insert({Filename, {&Entry, nullptr}});
+ auto &[CachedEntry, CachedRealPath] = It->getValue();
+ if (!Inserted) {
+ // The file is already present in the local cache. If we got here, it
only
+ // contains the real path. Let's make sure the entry is populated too.
+ assert((!CachedEntry && CachedRealPath) && "entry already present");
+ CachedEntry = &Entry;
+ }
+ return *CachedEntry;
}
/// Returns real path associated with the filename or nullptr if none is
@@ -266,9 +272,15 @@ class DependencyScanningFilesystemLocalCache {
insertRealPathForFilename(StringRef Filename,
const CachedRealPath &RealPath) {
assert(llvm::sys::path::is_absolute_gnu(Filename));
- assert(Cache[Filename].second == nullptr && "entry already present");
- Cache[Filename].second = &RealPath;
- return RealPath;
+ auto [It, Inserted] = Cache.insert({Filename, {nullptr, &RealPath}});
+ auto &[CachedEntry, CachedRealPath] = It->getValue();
+ if (!Inserted) {
+ // The file is already present in the local cache. If we got here, it
only
+ // contains the entry. Let's make sure the real path is populated too.
+ assert((!CachedRealPath && CachedEntry) && "real path already present");
+ CachedRealPath = &RealPath;
+ }
+ return *CachedRealPath;
}
};
>From 315976ee6ddcf9a3f2d722ca0c6524c23b403db4 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Thu, 11 Apr 2024 13:32:34 -0700
Subject: [PATCH 11/14] Do not replace existing entry in shared cache
---
.../DependencyScanning/DependencyScanningFilesystem.cpp | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index fe2f178def60be..6bc36d44c984d7 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -159,8 +159,11 @@ DependencyScanningFilesystemSharedCache::CacheShard::
getOrInsertEntryForFilename(StringRef Filename,
const CachedFileSystemEntry &Entry) {
std::lock_guard<std::mutex> LockGuard(CacheLock);
- CacheByFilename[Filename].first = &Entry;
- return Entry;
+ auto [It, Inserted] = CacheByFilename.insert({Filename, {&Entry, nullptr}});
+ auto &[CachedEntry, CachedRealPath] = It->getValue();
+ if (!Inserted || !CachedEntry)
+ CachedEntry = &Entry;
+ return *CachedEntry;
}
const CachedRealPath *
>From b0a392f175b467c613fbc5c1e8a34aa76fcd0857 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Thu, 11 Apr 2024 13:34:18 -0700
Subject: [PATCH 12/14] Propagate stat failures to shared cache by adding
forgotten reference
---
.../DependencyScanningFilesystem.cpp | 9 +++---
.../DependencyScanningFilesystemTest.cpp | 29 +++++++++++++++++++
2 files changed, 34 insertions(+), 4 deletions(-)
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 6bc36d44c984d7..669e0ee33bd224 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -130,11 +130,12 @@ DependencyScanningFilesystemSharedCache::CacheShard::
getOrEmplaceEntryForFilename(StringRef Filename,
llvm::ErrorOr<llvm::vfs::Status> Stat) {
std::lock_guard<std::mutex> LockGuard(CacheLock);
- const CachedFileSystemEntry *StoredEntry = CacheByFilename[Filename].first;
- if (!StoredEntry)
- StoredEntry =
+ auto [It, Inserted] = CacheByFilename.insert({Filename, {nullptr, nullptr}});
+ auto &[CachedEntry, CachedRealPath] = It->getValue();
+ if (!CachedEntry)
+ CachedEntry =
new (EntryStorage.Allocate()) CachedFileSystemEntry(std::move(Stat));
- return *StoredEntry;
+ return *CachedEntry;
}
const CachedFileSystemEntry &
diff --git
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
index 408e7874ee4f83..a50134ba6beae6 100644
---
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
+++
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
@@ -16,11 +16,17 @@ using namespace clang::tooling::dependencies;
namespace {
struct InstrumentingFilesystem
: llvm::RTTIExtends<InstrumentingFilesystem, llvm::vfs::ProxyFileSystem> {
+ unsigned NumStatusCalls = 0;
unsigned NumGetRealPathCalls = 0;
using llvm::RTTIExtends<InstrumentingFilesystem,
llvm::vfs::ProxyFileSystem>::RTTIExtends;
+ llvm::ErrorOr<llvm::vfs::Status> status(const llvm::Twine &Path) override {
+ ++NumStatusCalls;
+ return ProxyFileSystem::status(Path);
+ }
+
std::error_code getRealPath(const llvm::Twine &Path,
llvm::SmallVectorImpl<char> &Output) override {
++NumGetRealPathCalls;
@@ -29,6 +35,29 @@ struct InstrumentingFilesystem
};
} // namespace
+TEST(DependencyScanningWorkerFilesystem, CacheStatusFailures) {
+ auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+
+ auto InstrumentingFS =
+ llvm::makeIntrusiveRefCnt<InstrumentingFilesystem>(InMemoryFS);
+
+ DependencyScanningFilesystemSharedCache SharedCache;
+ DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
+ DependencyScanningWorkerFilesystem DepFS2(SharedCache, InstrumentingFS);
+
+ DepFS.status("/foo.c");
+ EXPECT_EQ(InstrumentingFS->NumStatusCalls, 1u);
+
+ DepFS.status("/foo.c");
+ EXPECT_EQ(InstrumentingFS->NumStatusCalls, 1u); // Cached, no increase.
+
+ DepFS.status("/bar.c");
+ EXPECT_EQ(InstrumentingFS->NumStatusCalls, 2u);
+
+ DepFS2.status("/foo.c");
+ EXPECT_EQ(InstrumentingFS->NumStatusCalls, 2u); // Shared cache.
+}
+
TEST(DependencyScanningFilesystem, CacheGetRealPath) {
auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
InMemoryFS->setCurrentWorkingDirectory("/");
>From 5bf9ac3a78d282ca84e3d529b12303717a96b9a2 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Thu, 11 Apr 2024 13:41:31 -0700
Subject: [PATCH 13/14] NFCI: Use structured binding to align with rest of file
---
.../DependencyScanning/DependencyScanningFilesystem.cpp | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 669e0ee33bd224..ff70ecf1d8d2ac 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -143,16 +143,17 @@
DependencyScanningFilesystemSharedCache::CacheShard::getOrEmplaceEntryForUID(
llvm::sys::fs::UniqueID UID, llvm::vfs::Status Stat,
std::unique_ptr<llvm::MemoryBuffer> Contents) {
std::lock_guard<std::mutex> LockGuard(CacheLock);
- auto Insertion = EntriesByUID.insert({UID, nullptr});
- if (Insertion.second) {
+ auto [It, Inserted] = EntriesByUID.insert({UID, nullptr});
+ auto &CachedEntry = It->getSecond();
+ if (Inserted) {
CachedFileContents *StoredContents = nullptr;
if (Contents)
StoredContents = new (ContentsStorage.Allocate())
CachedFileContents(std::move(Contents));
- Insertion.first->second = new (EntryStorage.Allocate())
+ CachedEntry = new (EntryStorage.Allocate())
CachedFileSystemEntry(std::move(Stat), StoredContents);
}
- return *Insertion.first->second;
+ return *CachedEntry;
}
const CachedFileSystemEntry &
>From d81b73949174356ea5fb3141606360efc2be58a5 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Thu, 11 Apr 2024 14:32:10 -0700
Subject: [PATCH 14/14] Add and test extra assertion
---
.../DependencyScanningFilesystem.cpp | 6 +-
.../DependencyScanningFilesystemTest.cpp | 55 +++++++++++++++++++
2 files changed, 60 insertions(+), 1 deletion(-)
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index ff70ecf1d8d2ac..84185c931b552c 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -132,9 +132,13 @@ DependencyScanningFilesystemSharedCache::CacheShard::
std::lock_guard<std::mutex> LockGuard(CacheLock);
auto [It, Inserted] = CacheByFilename.insert({Filename, {nullptr, nullptr}});
auto &[CachedEntry, CachedRealPath] = It->getValue();
- if (!CachedEntry)
+ if (!CachedEntry) {
+ // The entry is not present in the shared cache. Either the cache doesn't
+ // know about the file at all, or it only knows about its real path.
+ assert((Inserted || CachedRealPath) && "existing file with empty pair");
CachedEntry =
new (EntryStorage.Allocate()) CachedFileSystemEntry(std::move(Stat));
+ }
return *CachedEntry;
}
diff --git
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
index a50134ba6beae6..50cad72d223e35 100644
---
a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
+++
b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
@@ -95,3 +95,58 @@ TEST(DependencyScanningFilesystem, CacheGetRealPath) {
EXPECT_EQ(InstrumentingFS->NumGetRealPathCalls, 2u); // Shared cache.
}
}
+
+TEST(DependencyScanningFilesystem, RealPathAndStatusInvariants) {
+ auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+ InMemoryFS->setCurrentWorkingDirectory("/");
+ InMemoryFS->addFile("/foo.c", 0, llvm::MemoryBuffer::getMemBuffer(""));
+ InMemoryFS->addFile("/bar.c", 0, llvm::MemoryBuffer::getMemBuffer(""));
+
+ auto InstrumentingFS =
+ llvm::makeIntrusiveRefCnt<InstrumentingFilesystem>(InMemoryFS);
+
+ DependencyScanningFilesystemSharedCache SharedCache;
+ DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
+
+ // Success.
+ {
+ DepFS.status("/foo.c");
+
+ llvm::SmallString<128> Result;
+ DepFS.getRealPath("/foo.c", Result);
+ }
+ {
+ llvm::SmallString<128> Result;
+ DepFS.getRealPath("/bar.c", Result);
+
+ DepFS.status("/bar.c");
+ }
+
+ // Failure.
+ {
+ DepFS.status("/foo.m");
+
+ llvm::SmallString<128> Result;
+ DepFS.getRealPath("/foo.m", Result);
+ }
+ {
+ llvm::SmallString<128> Result;
+ DepFS.getRealPath("/bar.m", Result);
+
+ DepFS.status("/bar.m");
+ }
+
+ // Failure without caching.
+ {
+ DepFS.status("/foo");
+
+ llvm::SmallString<128> Result;
+ DepFS.getRealPath("/foo", Result);
+ }
+ {
+ llvm::SmallString<128> Result;
+ DepFS.getRealPath("/bar", Result);
+
+ DepFS.status("/bar");
+ }
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
