[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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@358509 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Basic/FileSystemStatCache.h

@@ -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

lib/Basic/FileManager.cpp

@@ -429,13 +429,14 @@ bool FileManager::getStatValue(StringRef Path, llvm::vfs::Status &Status,
   // 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,

lib/Basic/FileSystemStatCache.cpp

@@ -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 Path, llvm::vfs::Status &Status,
 
     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 Path, llvm::vfs::Status &Status,
     // 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();
 }