r358509 - [FileSystemStatCache] Return std::error_code from stat cache methods
Harlan Haskins via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 16 10:34:26 PDT 2019
Author: harlanhaskins
Date: Tue Apr 16 10:34:26 2019
New Revision: 358509
URL: http://llvm.org/viewvc/llvm-project?rev=358509&view=rev
Log:
[FileSystemStatCache] Return std::error_code from stat cache methods
Summary:
Previously, we would return true/false signifying if the cache/lookup
succeeded or failed. Instead, provide clients with the underlying error
that was thrown while attempting to look up in the cache.
Since clang::FileManager doesn't make use of this information, it discards the
error that's received and casts away to bool.
This change is NFC.
Reviewers: benlangmuir, arphaman
Subscribers: dexonsmith, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60735
Modified:
cfe/trunk/include/clang/Basic/FileSystemStatCache.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Basic/FileSystemStatCache.cpp
Modified: cfe/trunk/include/clang/Basic/FileSystemStatCache.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileSystemStatCache.h?rev=358509&r1=358508&r2=358509&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/FileSystemStatCache.h (original)
+++ cfe/trunk/include/clang/Basic/FileSystemStatCache.h Tue Apr 16 10:34:26 2019
@@ -37,14 +37,6 @@ class FileSystemStatCache {
public:
virtual ~FileSystemStatCache() = default;
- enum LookupResult {
- /// We know the file exists and its cached stat data.
- CacheExists,
-
- /// We know that the file doesn't exist.
- CacheMissing
- };
-
/// Get the 'stat' information for the specified path, using the cache
/// to accelerate it if possible.
///
@@ -55,18 +47,19 @@ public:
/// success for directories (not files). On a successful file lookup, the
/// implementation can optionally fill in \p F with a valid \p File object and
/// the client guarantees that it will close it.
- static bool get(StringRef Path, llvm::vfs::Status &Status, bool isFile,
- std::unique_ptr<llvm::vfs::File> *F,
- FileSystemStatCache *Cache, llvm::vfs::FileSystem &FS);
+ static std::error_code
+ get(StringRef Path, llvm::vfs::Status &Status, bool isFile,
+ std::unique_ptr<llvm::vfs::File> *F,
+ FileSystemStatCache *Cache, llvm::vfs::FileSystem &FS);
protected:
// FIXME: The pointer here is a non-owning/optional reference to the
// unique_ptr. Optional<unique_ptr<vfs::File>&> might be nicer, but
// Optional needs some work to support references so this isn't possible yet.
- virtual LookupResult getStat(StringRef Path, llvm::vfs::Status &Status,
- bool isFile,
- std::unique_ptr<llvm::vfs::File> *F,
- llvm::vfs::FileSystem &FS) = 0;
+ virtual std::error_code getStat(StringRef Path, llvm::vfs::Status &Status,
+ bool isFile,
+ std::unique_ptr<llvm::vfs::File> *F,
+ llvm::vfs::FileSystem &FS) = 0;
};
/// A stat "cache" that can be used by FileManager to keep
@@ -84,9 +77,10 @@ public:
iterator begin() const { return StatCalls.begin(); }
iterator end() const { return StatCalls.end(); }
- LookupResult getStat(StringRef Path, llvm::vfs::Status &Status, bool isFile,
- std::unique_ptr<llvm::vfs::File> *F,
- llvm::vfs::FileSystem &FS) override;
+ std::error_code getStat(StringRef Path, llvm::vfs::Status &Status,
+ bool isFile,
+ std::unique_ptr<llvm::vfs::File> *F,
+ llvm::vfs::FileSystem &FS) override;
};
} // namespace clang
Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=358509&r1=358508&r2=358509&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Tue Apr 16 10:34:26 2019
@@ -429,13 +429,14 @@ bool FileManager::getStatValue(StringRef
// FIXME: FileSystemOpts shouldn't be passed in here, all paths should be
// absolute!
if (FileSystemOpts.WorkingDir.empty())
- return FileSystemStatCache::get(Path, Status, isFile, F,StatCache.get(), *FS);
+ return bool(FileSystemStatCache::get(Path, Status, isFile, F,
+ StatCache.get(), *FS));
SmallString<128> FilePath(Path);
FixupRelativePath(FilePath);
- return FileSystemStatCache::get(FilePath.c_str(), Status, isFile, F,
- StatCache.get(), *FS);
+ return bool(FileSystemStatCache::get(FilePath.c_str(), Status, isFile, F,
+ StatCache.get(), *FS));
}
bool FileManager::getNoncachedStatValue(StringRef Path,
Modified: cfe/trunk/lib/Basic/FileSystemStatCache.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileSystemStatCache.cpp?rev=358509&r1=358508&r2=358509&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileSystemStatCache.cpp (original)
+++ cfe/trunk/lib/Basic/FileSystemStatCache.cpp Tue Apr 16 10:34:26 2019
@@ -30,25 +30,24 @@ void FileSystemStatCache::anchor() {}
/// success for directories (not files). On a successful file lookup, the
/// implementation can optionally fill in FileDescriptor with a valid
/// descriptor and the client guarantees that it will close it.
-bool FileSystemStatCache::get(StringRef Path, llvm::vfs::Status &Status,
- bool isFile,
- std::unique_ptr<llvm::vfs::File> *F,
- FileSystemStatCache *Cache,
- llvm::vfs::FileSystem &FS) {
- LookupResult R;
+std::error_code
+FileSystemStatCache::get(StringRef Path, llvm::vfs::Status &Status,
+ bool isFile, std::unique_ptr<llvm::vfs::File> *F,
+ FileSystemStatCache *Cache,
+ llvm::vfs::FileSystem &FS) {
bool isForDir = !isFile;
+ std::error_code RetCode;
// If we have a cache, use it to resolve the stat query.
if (Cache)
- R = Cache->getStat(Path, Status, isFile, F, FS);
+ RetCode = Cache->getStat(Path, Status, isFile, F, FS);
else if (isForDir || !F) {
// If this is a directory or a file descriptor is not needed and we have
// no cache, just go to the file system.
llvm::ErrorOr<llvm::vfs::Status> StatusOrErr = FS.status(Path);
if (!StatusOrErr) {
- R = CacheMissing;
+ RetCode = StatusOrErr.getError();
} else {
- R = CacheExists;
Status = *StatusOrErr;
}
} else {
@@ -63,27 +62,27 @@ bool FileSystemStatCache::get(StringRef
if (!OwnedFile) {
// If the open fails, our "stat" fails.
- R = CacheMissing;
+ RetCode = OwnedFile.getError();
} else {
// Otherwise, the open succeeded. Do an fstat to get the information
// about the file. We'll end up returning the open file descriptor to the
// client to do what they please with it.
llvm::ErrorOr<llvm::vfs::Status> StatusOrErr = (*OwnedFile)->status();
if (StatusOrErr) {
- R = CacheExists;
Status = *StatusOrErr;
*F = std::move(*OwnedFile);
} else {
// fstat rarely fails. If it does, claim the initial open didn't
// succeed.
- R = CacheMissing;
*F = nullptr;
+ RetCode = StatusOrErr.getError();
}
}
}
// If the path doesn't exist, return failure.
- if (R == CacheMissing) return true;
+ if (RetCode)
+ return RetCode;
// If the path exists, make sure that its "directoryness" matches the clients
// demands.
@@ -91,29 +90,31 @@ bool FileSystemStatCache::get(StringRef
// If not, close the file if opened.
if (F)
*F = nullptr;
-
- return true;
+ return std::make_error_code(
+ Status.isDirectory() ?
+ std::errc::is_a_directory : std::errc::not_a_directory);
}
- return false;
+ return std::error_code();
}
-MemorizeStatCalls::LookupResult
+std::error_code
MemorizeStatCalls::getStat(StringRef Path, llvm::vfs::Status &Status,
bool isFile,
std::unique_ptr<llvm::vfs::File> *F,
llvm::vfs::FileSystem &FS) {
- if (get(Path, Status, isFile, F, nullptr, FS)) {
+ auto err = get(Path, Status, isFile, F, nullptr, FS);
+ if (err) {
// Do not cache failed stats, it is easy to construct common inconsistent
// situations if we do, and they are not important for PCH performance
// (which currently only needs the stats to construct the initial
// FileManager entries).
- return CacheMissing;
+ return err;
}
// Cache file 'stat' results and directories with absolutely paths.
if (!Status.isDirectory() || llvm::sys::path::is_absolute(Path))
StatCalls[Path] = Status;
- return CacheExists;
+ return std::error_code();
}
More information about the cfe-commits
mailing list