[clang][deps] Overload 'Filesystem::exists' in 'DependencyScanningFilesystem' to have it use cached status

As-is, calls to `exists()` fallback on the implementation in 'ProxyFileSystem::exists' which explicitly calls out to the underlying `FS`, which for the 'DependencyScanningFilesystem' (overlay) is the real underlying filesystem.

Instead, directly overloading 'exists' allows us to have it rely on the cached `status` behavior used elsewhere by the 'DependencyScanningFilesystem'.

Author: artemcm

clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h

@@ -310,6 +310,10 @@ class DependencyScanningWorkerFilesystem
   /// false if not (i.e. this entry is not a file or its scan fails).
   bool ensureDirectiveTokensArePopulated(EntryRef Entry);
 
+  /// Check whether \p Path exists. By default checks cached result of \c status(),
+  /// and falls back on FS if unable to do so.
+  bool exists(const Twine &Path) override;
+				 
 private:
   /// For a filename that's not yet associated with any entry in the caches,
   /// uses the underlying filesystem to either look up the entry based in the

clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

@@ -270,6 +270,12 @@ DependencyScanningWorkerFilesystem::status(const Twine &Path) {
   return Result->getStatus();
 }
 
+bool
+DependencyScanningWorkerFilesystem::exists(const Twine &Path) {
+  llvm::ErrorOr<llvm::vfs::Status> Status = status(Path);
+  return Status && Status->exists();
+}
+
 namespace {
 
 /// The VFS that is used by clang consumes the \c CachedFileSystemEntry using

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

clang/unittests/Tooling/DependencyScannerTest.cpp renamed to clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp

@@ -1,4 +1,4 @@
-//===- unittest/Tooling/ToolingTest.cpp - Tooling unit tests --------------===//
+//===- DependencyScannerTest.cpp - Tooling unit tests ---------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp

@@ -0,0 +1,51 @@
+//===- 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/Twine.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 NumStatCalls = 0;
+
+   using llvm::RTTIExtends<InstrumentingFilesystem,
+                           llvm::vfs::ProxyFileSystem>::RTTIExtends;
+
+   llvm::ErrorOr<llvm::vfs::Status> status(const llvm::Twine &Path) override {
+     ++NumStatCalls;
+     return ProxyFileSystem::status(Path);
+   }
+ };
+ } // namespace
+
+
+TEST(DependencyScanningFilesystem, CacheStatOnExists) {
+  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);
+
+  DepFS.status("/foo");
+  DepFS.status("/foo");
+  DepFS.status("/bar");
+  EXPECT_EQ(InstrumentingFS->NumStatCalls, 2u);
+
+  DepFS.exists("/foo");
+  DepFS.exists("/bar");
+  EXPECT_EQ(InstrumentingFS->NumStatCalls, 2u);  
+}

llvm/unittests/Support/VirtualFileSystemTest.cpp

@@ -211,7 +211,7 @@ class NoStatusDummyFileSystem : public DummyFileSystem {
   }
 
   bool exists(const Twine &Path) override {
-    auto Status = DummyFileSystem::status(Path);
+    llvm::ErrorOr<llvm::vfs::Status> Status = status(Path);
     return Status && Status->exists();
   }
 };
@@ -3560,3 +3560,27 @@ TEST(RedirectingFileSystemTest, ExistsRedirectOnly) {
   EXPECT_FALSE(Redirecting->exists("/b"));
   EXPECT_TRUE(Redirecting->exists("/vfile"));
 }
+
+TEST(DependencyScanningWorkerFilesystemTest, ExistsUsesStatCache) {
+  struct StatCountingProxyFS : llvm::vfs::ProxyFileSystem {
+    int StatCount;
+
+    StatCountingProxyFS(IntrusiveRefCntPtr<FileSystem> UnderlyingFS)
+    : ProxyFileSystem(UnderlyingFS), StatCount(0) {}
+
+    llvm::ErrorOr<llvm::vfs::Status> status(const Twine &Path) override {
+      StatCount++;
+      return ProxyFileSystem::status(Path);
+    }
+  };
+  auto BaseFS = IntrusiveRefCntPtr<DummyFileSystem>(new DummyFileSystem());
+  auto ScanFS = makeIntrusiveRefCnt<StatCountingProxyFS>(BaseFS);
+  BaseFS->addRegularFile("/a");
+  BaseFS->addRegularFile("/b");
+  ScanFS->status("/a");
+  ScanFS->status("/b");
+  EXPECT_TRUE(ScanFS->StatCount == 2);
+  ScanFS->exists("/a");
+  ScanFS->exists("/b");
+  EXPECT_TRUE(ScanFS->StatCount == 2);
+}