In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.

Summary:
It seems that when the CachingFileSystem is first given a file to open that is actually a directory, it incorrectly
caches that path to be errenous and throws an error when subsequently a directory open call is made for the same
path.
This change makes it so that we do NOT cache a path if it turns out we asked for a file when its a directory.

Reviewers: arphaman

Subscribers: dexonsmith, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D68193

llvm-svn: 374366

1 files changed, 20 insertions, 35 deletions

diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 35ecbd4a7fb..63c68f58e6d 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -122,14 +122,11 @@ DependencyScanningFilesystemSharedCache::get(StringRef Key) {
   return It.first->getValue();
 }
 
-llvm::ErrorOr<llvm::vfs::Status>
-DependencyScanningWorkerFilesystem::status(const Twine &Path) {
-  SmallString<256> OwnedFilename;
-  StringRef Filename = Path.toStringRef(OwnedFilename);
-
-  // Check the local cache first.
-  if (const CachedFileSystemEntry *Entry = getCachedEntry(Filename))
-    return Entry->getStatus();
+llvm::ErrorOr<const CachedFileSystemEntry *>
+DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(const StringRef Filename) {
+  if (const CachedFileSystemEntry* Entry = getCachedEntry(Filename)) {
+    return Entry;
+  }
 
   // FIXME: Handle PCM/PCH files.
   // FIXME: Handle module map files.
@@ -160,7 +157,17 @@ DependencyScanningWorkerFilesystem::status(const Twine &Path) {
 
   // Store the result in the local cache.
   setCachedEntry(Filename, Result);
-  return Result->getStatus();
+  return Result;
+}
+
+llvm::ErrorOr<llvm::vfs::Status>
+DependencyScanningWorkerFilesystem::status(const Twine &Path) {
+  SmallString<256> OwnedFilename;
+  StringRef Filename = Path.toStringRef(OwnedFilename);
+  const llvm::ErrorOr<const CachedFileSystemEntry *> Result = getOrCreateFileSystemEntry(Filename);
+  if (!Result)
+    return Result.getError();
+  return (*Result)->getStatus();
 }
 
 namespace {
@@ -217,30 +224,8 @@ DependencyScanningWorkerFilesystem::openFileForRead(const Twine &Path) {
   SmallString<256> OwnedFilename;
   StringRef Filename = Path.toStringRef(OwnedFilename);
 
-  // Check the local cache first.
-  if (const CachedFileSystemEntry *Entry = getCachedEntry(Filename))
-    return createFile(Entry, PPSkipMappings);
-
-  // FIXME: Handle PCM/PCH files.
-  // FIXME: Handle module map files.
-
-  bool KeepOriginalSource = IgnoredFiles.count(Filename);
-  DependencyScanningFilesystemSharedCache::SharedFileSystemEntry
-      &SharedCacheEntry = SharedCache.get(Filename);
-  const CachedFileSystemEntry *Result;
-  {
-    std::unique_lock<std::mutex> LockGuard(SharedCacheEntry.ValueLock);
-    CachedFileSystemEntry &CacheEntry = SharedCacheEntry.Value;
-
-    if (!CacheEntry.isValid()) {
-      CacheEntry = CachedFileSystemEntry::createFileEntry(
-          Filename, getUnderlyingFS(), !KeepOriginalSource);
-    }
-
-    Result = &CacheEntry;
-  }
-
-  // Store the result in the local cache.
-  setCachedEntry(Filename, Result);
-  return createFile(Result, PPSkipMappings);
+  const llvm::ErrorOr<const CachedFileSystemEntry *> Result = getOrCreateFileSystemEntry(Filename);
+  if (!Result)
+    return Result.getError();
+  return createFile(Result.get(), PPSkipMappings);
 }