Revert "[clang][scan-deps] Add option to disable caching stat failures"

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

@@ -23,8 +23,6 @@ namespace clang {
 namespace tooling {
 namespace dependencies {
 
-class DependencyScanningService;
-
 using DependencyDirectivesTy =
     SmallVector<dependency_directives_scan::Directive, 20>;
 
@@ -351,7 +349,7 @@ class DependencyScanningWorkerFilesystem
   static const char ID;
 
   DependencyScanningWorkerFilesystem(
-      DependencyScanningService &Service,
+      DependencyScanningFilesystemSharedCache &SharedCache,
       IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS);
 
   llvm::ErrorOr<llvm::vfs::Status> status(const Twine &Path) override;
@@ -437,7 +435,10 @@ class DependencyScanningWorkerFilesystem
   /// Returns entry associated with the unique ID in the shared cache or nullptr
   /// if none is found.
   const CachedFileSystemEntry *
-  findSharedEntryByUID(llvm::vfs::Status Stat) const;
+  findSharedEntryByUID(llvm::vfs::Status Stat) const {
+    return SharedCache.getShardForUID(Stat.getUniqueID())
+        .findEntryByUID(Stat.getUniqueID());
+  }
 
   /// Associates the given entry with the filename in the local cache and
   /// returns it.
@@ -451,14 +452,20 @@ class DependencyScanningWorkerFilesystem
   /// some. Otherwise, constructs new one with the given error code, associates
   /// it with the filename and returns the result.
   const CachedFileSystemEntry &
-  getOrEmplaceSharedEntryForFilename(StringRef Filename, std::error_code EC);
+  getOrEmplaceSharedEntryForFilename(StringRef Filename, std::error_code EC) {
+    return SharedCache.getShardForFilename(Filename)
+        .getOrEmplaceEntryForFilename(Filename, EC);
+  }
 
   /// Returns entry associated with the filename in the shared cache if there is
   /// some. Otherwise, associates the given entry with the filename and returns
   /// it.
   const CachedFileSystemEntry &
   getOrInsertSharedEntryForFilename(StringRef Filename,
-                                    const CachedFileSystemEntry &Entry);
+                                    const CachedFileSystemEntry &Entry) {
+    return SharedCache.getShardForFilename(Filename)
+        .getOrInsertEntryForFilename(Filename, Entry);
+  }
 
   void printImpl(raw_ostream &OS, PrintType Type,
                  unsigned IndentLevel) const override {
@@ -471,9 +478,8 @@ class DependencyScanningWorkerFilesystem
   /// VFS.
   bool shouldBypass(StringRef Path) const;
 
-  /// The service associated with this VFS.
-  DependencyScanningService &Service;
-
+  /// The global cache shared between worker threads.
+  DependencyScanningFilesystemSharedCache &SharedCache;
   /// The local cache is used by the worker thread to cache file system queries
   /// locally instead of querying the global cache every time.
   DependencyScanningFilesystemLocalCache LocalCache;

clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h

@@ -87,8 +87,7 @@ class DependencyScanningService {
       ScanningOptimizations OptimizeArgs = ScanningOptimizations::Default,
       bool EagerLoadModules = false, bool TraceVFS = false,
       std::time_t BuildSessionTimestamp =
-          llvm::sys::toTimeT(std::chrono::system_clock::now()),
-      bool CacheNegativeStats = true);
+          llvm::sys::toTimeT(std::chrono::system_clock::now()));
 
   ScanningMode getMode() const { return Mode; }
 
@@ -100,8 +99,6 @@ class DependencyScanningService {
 
   bool shouldTraceVFS() const { return TraceVFS; }
 
-  bool shouldCacheNegativeStats() const { return CacheNegativeStats; }
-
   DependencyScanningFilesystemSharedCache &getSharedCache() {
     return SharedCache;
   }
@@ -119,7 +116,6 @@ class DependencyScanningService {
   const bool EagerLoadModules;
   /// Whether to trace VFS accesses.
   const bool TraceVFS;
-  const bool CacheNegativeStats;
   /// The global file system cache.
   DependencyScanningFilesystemSharedCache SharedCache;
   /// The global module cache entries.

clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

@@ -7,7 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
-#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Threading.h"
 #include <optional>
@@ -233,19 +232,19 @@ bool DependencyScanningWorkerFilesystem::shouldBypass(StringRef Path) const {
 }
 
 DependencyScanningWorkerFilesystem::DependencyScanningWorkerFilesystem(
-    DependencyScanningService &Service,
+    DependencyScanningFilesystemSharedCache &SharedCache,
     IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
     : llvm::RTTIExtends<DependencyScanningWorkerFilesystem,
                         llvm::vfs::ProxyFileSystem>(std::move(FS)),
-      Service(Service), WorkingDirForCacheLookup(llvm::errc::invalid_argument) {
+      SharedCache(SharedCache),
+      WorkingDirForCacheLookup(llvm::errc::invalid_argument) {
   updateWorkingDirForCacheLookup();
 }
 
 const CachedFileSystemEntry &
 DependencyScanningWorkerFilesystem::getOrEmplaceSharedEntryForUID(
     TentativeEntry TEntry) {
-  auto &Shard =
-      Service.getSharedCache().getShardForUID(TEntry.Status.getUniqueID());
+  auto &Shard = SharedCache.getShardForUID(TEntry.Status.getUniqueID());
   return Shard.getOrEmplaceEntryForUID(TEntry.Status.getUniqueID(),
                                        std::move(TEntry.Status),
                                        std::move(TEntry.Contents));
@@ -256,44 +255,18 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough(
     StringRef Filename) {
   if (const auto *Entry = LocalCache.findEntryByFilename(Filename))
     return Entry;
-  auto &Shard = Service.getSharedCache().getShardForFilename(Filename);
+  auto &Shard = SharedCache.getShardForFilename(Filename);
   if (const auto *Entry = Shard.findEntryByFilename(Filename))
     return &LocalCache.insertEntryForFilename(Filename, *Entry);
   return nullptr;
 }
 
-const CachedFileSystemEntry *
-DependencyScanningWorkerFilesystem::findSharedEntryByUID(
-    llvm::vfs::Status Stat) const {
-  return Service.getSharedCache()
-      .getShardForUID(Stat.getUniqueID())
-      .findEntryByUID(Stat.getUniqueID());
-}
-
-const CachedFileSystemEntry &
-DependencyScanningWorkerFilesystem::getOrEmplaceSharedEntryForFilename(
-    StringRef Filename, std::error_code EC) {
-  return Service.getSharedCache()
-      .getShardForFilename(Filename)
-      .getOrEmplaceEntryForFilename(Filename, EC);
-}
-
-const CachedFileSystemEntry &
-DependencyScanningWorkerFilesystem::getOrInsertSharedEntryForFilename(
-    StringRef Filename, const CachedFileSystemEntry &Entry) {
-  return Service.getSharedCache()
-      .getShardForFilename(Filename)
-      .getOrInsertEntryForFilename(Filename, Entry);
-}
-
 llvm::ErrorOr<const CachedFileSystemEntry &>
 DependencyScanningWorkerFilesystem::computeAndStoreResult(
     StringRef OriginalFilename, StringRef FilenameForLookup) {
   llvm::ErrorOr<llvm::vfs::Status> Stat =
       getUnderlyingFS().status(OriginalFilename);
   if (!Stat) {
-    if (!Service.shouldCacheNegativeStats())
-      return Stat.getError();
     const auto &Entry =
         getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError());
     return insertLocalEntryForFilename(FilenameForLookup, Entry);
@@ -447,8 +420,7 @@ DependencyScanningWorkerFilesystem::getRealPath(const Twine &Path,
     return HandleCachedRealPath(*RealPath);
 
   // If we have the result in the shared cache, cache it locally.
-  auto &Shard =
-      Service.getSharedCache().getShardForFilename(*FilenameForLookup);
+  auto &Shard = SharedCache.getShardForFilename(*FilenameForLookup);
   if (const auto *ShardRealPath =
           Shard.findRealPathByFilename(*FilenameForLookup)) {
     const auto &RealPath = LocalCache.insertRealPathForFilename(

clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp

@@ -15,8 +15,7 @@ using namespace dependencies;
 DependencyScanningService::DependencyScanningService(
     ScanningMode Mode, ScanningOutputFormat Format,
     ScanningOptimizations OptimizeArgs, bool EagerLoadModules, bool TraceVFS,
-    std::time_t BuildSessionTimestamp, bool CacheNegativeStats)
+    std::time_t BuildSessionTimestamp)
     : Mode(Mode), Format(Format), OptimizeArgs(OptimizeArgs),
       EagerLoadModules(EagerLoadModules), TraceVFS(TraceVFS),
-      CacheNegativeStats(CacheNegativeStats),
       BuildSessionTimestamp(BuildSessionTimestamp) {}

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

@@ -611,7 +611,8 @@ DependencyScanningWorker::DependencyScanningWorker(
 
   switch (Service.getMode()) {
   case ScanningMode::DependencyDirectivesScan:
-    DepFS = new DependencyScanningWorkerFilesystem(Service, FS);
+    DepFS =
+        new DependencyScanningWorkerFilesystem(Service.getSharedCache(), FS);
     BaseFS = DepFS;
     break;
   case ScanningMode::CanonicalPreprocessing:

clang/tools/clang-scan-deps/ClangScanDeps.cpp

@@ -85,7 +85,6 @@ static ScanningOutputFormat Format = ScanningOutputFormat::Make;
 static ScanningOptimizations OptimizeArgs;
 static std::string ModuleFilesDir;
 static bool EagerLoadModules;
-static bool CacheNegativeStats = true;
 static unsigned NumThreads = 0;
 static std::string CompilationDB;
 static std::optional<std::string> ModuleName;
@@ -192,8 +191,6 @@ static void ParseArgs(int argc, char **argv) {
 
   EagerLoadModules = Args.hasArg(OPT_eager_load_pcm);
 
-  CacheNegativeStats = !Args.hasArg(OPT_no_cache_negative_stats);
-
   if (const llvm::opt::Arg *A = Args.getLastArg(OPT_j)) {
     StringRef S{A->getValue()};
     if (!llvm::to_integer(S, NumThreads, 0)) {
@@ -1083,9 +1080,8 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
     });
   };
 
-  DependencyScanningService Service(
-      ScanMode, Format, OptimizeArgs, EagerLoadModules, /*TraceVFS=*/Verbose,
-      llvm::sys::toTimeT(std::chrono::system_clock::now()), CacheNegativeStats);
+  DependencyScanningService Service(ScanMode, Format, OptimizeArgs,
+                                    EagerLoadModules, /*TraceVFS=*/Verbose);
 
   llvm::Timer T;
   T.startTimer();

clang/tools/clang-scan-deps/Opts.td

@@ -22,7 +22,6 @@ defm module_files_dir : Eq<"module-files-dir",
 
 def optimize_args_EQ : CommaJoined<["-", "--"], "optimize-args=">, HelpText<"Which command-line arguments of modules to optimize">;
 def eager_load_pcm : F<"eager-load-pcm", "Load PCM files eagerly (instead of lazily on import)">;
-def no_cache_negative_stats : F<"no-cache-negative-stats", "Don't cache stat failures">;
 
 def j : Arg<"j", "Number of worker threads to use (default: use all concurrent threads)">;
 

clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp

@@ -384,55 +384,3 @@ TEST(DependencyScanner, ScanDepsWithDiagConsumer) {
     EXPECT_TRUE(DiagConsumer.Finished);
   }
 }
-
-TEST(DependencyScanner, NoNegativeCache) {
-  StringRef CWD = "/root";
-
-  auto VFS = new llvm::vfs::InMemoryFileSystem();
-  VFS->setCurrentWorkingDirectory(CWD);
-  auto Sept = llvm::sys::path::get_separator();
-  std::string HeaderPath =
-      std::string(llvm::formatv("{0}root{0}header.h", Sept));
-  std::string Test0Path =
-      std::string(llvm::formatv("{0}root{0}test0.cpp", Sept));
-  std::string Test1Path =
-      std::string(llvm::formatv("{0}root{0}test1.cpp", Sept));
-
-  VFS->addFile(Test0Path, 0,
-               llvm::MemoryBuffer::getMemBuffer(
-                   "#if __has_include(\"header.h\")\n#endif"));
-  VFS->addFile(Test1Path, 0,
-               llvm::MemoryBuffer::getMemBuffer("#include \"header.h\""));
-
-  DependencyScanningService Service(
-      ScanningMode::DependencyDirectivesScan, ScanningOutputFormat::Make,
-      ScanningOptimizations::All, false, false,
-      llvm::sys::toTimeT(std::chrono::system_clock::now()), false);
-  DependencyScanningTool ScanTool(Service, VFS);
-
-  std::vector<std::string> CommandLine0 = {"clang",
-                                           "-target",
-                                           "x86_64-apple-macosx10.7",
-                                           "-c",
-                                           "test0.cpp",
-                                           "-o"
-                                           "test0.cpp.o"};
-  std::vector<std::string> CommandLine1 = {"clang",
-                                           "-target",
-                                           "x86_64-apple-macosx10.7",
-                                           "-c",
-                                           "test1.cpp",
-                                           "-o"
-                                           "test1.cpp.o"};
-
-  std::string Result;
-  ASSERT_THAT_ERROR(
-      ScanTool.getDependencyFile(CommandLine0, CWD).moveInto(Result),
-      llvm::Succeeded());
-
-  VFS->addFile(HeaderPath, 0, llvm::MemoryBuffer::getMemBuffer(""));
-
-  ASSERT_THAT_ERROR(
-      ScanTool.getDependencyFile(CommandLine1, CWD).moveInto(Result),
-      llvm::Succeeded());
-}

clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp

@@ -7,7 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
-#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "gtest/gtest.h"
@@ -20,10 +19,9 @@ TEST(DependencyScanningWorkerFilesystem, CacheStatusFailures) {
   auto InstrumentingFS =
       llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS);
 
-  DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
-                                    ScanningOutputFormat::Make);
-  DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS);
-  DependencyScanningWorkerFilesystem DepFS2(Service, InstrumentingFS);
+  DependencyScanningFilesystemSharedCache SharedCache;
+  DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
+  DependencyScanningWorkerFilesystem DepFS2(SharedCache, InstrumentingFS);
 
   DepFS.status("/foo.c");
   EXPECT_EQ(InstrumentingFS->NumStatusCalls, 1u);
@@ -47,10 +45,9 @@ TEST(DependencyScanningFilesystem, CacheGetRealPath) {
   auto InstrumentingFS =
       llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS);
 
-  DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
-                                    ScanningOutputFormat::Make);
-  DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS);
-  DependencyScanningWorkerFilesystem DepFS2(Service, InstrumentingFS);
+  DependencyScanningFilesystemSharedCache SharedCache;
+  DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
+  DependencyScanningWorkerFilesystem DepFS2(SharedCache, InstrumentingFS);
 
   {
     llvm::SmallString<128> Result;
@@ -83,9 +80,8 @@ TEST(DependencyScanningFilesystem, RealPathAndStatusInvariants) {
   InMemoryFS->addFile("/foo.c", 0, llvm::MemoryBuffer::getMemBuffer(""));
   InMemoryFS->addFile("/bar.c", 0, llvm::MemoryBuffer::getMemBuffer(""));
 
-  DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
-                                    ScanningOutputFormat::Make);
-  DependencyScanningWorkerFilesystem DepFS(Service, InMemoryFS);
+  DependencyScanningFilesystemSharedCache SharedCache;
+  DependencyScanningWorkerFilesystem DepFS(SharedCache, InMemoryFS);
 
   // Success.
   {
@@ -137,9 +133,8 @@ TEST(DependencyScanningFilesystem, CacheStatOnExists) {
   InMemoryFS->setCurrentWorkingDirectory("/");
   InMemoryFS->addFile("/foo", 0, llvm::MemoryBuffer::getMemBuffer(""));
   InMemoryFS->addFile("/bar", 0, llvm::MemoryBuffer::getMemBuffer(""));
-  DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
-                                    ScanningOutputFormat::Make);
-  DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS);
+  DependencyScanningFilesystemSharedCache SharedCache;
+  DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
 
   DepFS.status("/foo");
   DepFS.status("/foo");
@@ -161,9 +156,8 @@ TEST(DependencyScanningFilesystem, CacheStatFailures) {
   auto InstrumentingFS =
       llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS);
 
-  DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
-                                    ScanningOutputFormat::Make);
-  DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS);
+  DependencyScanningFilesystemSharedCache SharedCache;
+  DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
 
   DepFS.status("/dir");
   DepFS.status("/dir");
@@ -189,9 +183,8 @@ TEST(DependencyScanningFilesystem, DiagnoseStaleStatFailures) {
   auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
   InMemoryFS->setCurrentWorkingDirectory("/");
 
-  DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
-                                    ScanningOutputFormat::Make);
-  DependencyScanningWorkerFilesystem DepFS(Service, InMemoryFS);
+  DependencyScanningFilesystemSharedCache SharedCache;
+  DependencyScanningWorkerFilesystem DepFS(SharedCache, InMemoryFS);
 
   bool Path1Exists = DepFS.exists("/path1.suffix");
   EXPECT_EQ(Path1Exists, false);
@@ -204,7 +197,7 @@ TEST(DependencyScanningFilesystem, DiagnoseStaleStatFailures) {
   EXPECT_EQ(Path1Exists, false);
 
   std::vector<llvm::StringRef> InvalidPaths =
-      Service.getSharedCache().getInvalidNegativeStatCachedPaths(*InMemoryFS);
+      SharedCache.getInvalidNegativeStatCachedPaths(*InMemoryFS);
 
   EXPECT_EQ(InvalidPaths.size(), 1u);
   ASSERT_STREQ("/path1.suffix", InvalidPaths[0].str().c_str());