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(); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
