[clang-tools-extra] 9899319 - [clangd] Add hot-reload of compile_commands.json and compile_flags.txt

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 18 02:18:07 PST 2020


Author: Sam McCall
Date: 2020-12-18T11:16:46+01:00
New Revision: 98993193e9037345ad13720a62974064a5f3d953

URL: https://github.com/llvm/llvm-project/commit/98993193e9037345ad13720a62974064a5f3d953
DIFF: https://github.com/llvm/llvm-project/commit/98993193e9037345ad13720a62974064a5f3d953.diff

LOG: [clangd] Add hot-reload of compile_commands.json and compile_flags.txt

When querying the CDB, we stat the underlying file to check it hasn't changed.
We don't do this every time, but only if we didn't check within 5 seconds.

This behavior only exists for compile_commands.json and compile_flags.txt.
The CDB plugin system doesn't expose enough information to handle others.

Slight behavior change: we now only look for `build/compile_commands.json`
rather than trying every CDB strategy under `build` subdirectories.

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/ClangdLSPServer.cpp
    clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
    clang-tools-extra/clangd/GlobalCompilationDatabase.h
    clang-tools-extra/clangd/tool/Check.cpp
    clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 66dee68ec474..b32c9e13973b 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -496,8 +496,10 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params,
   if (const auto &Dir = Params.initializationOptions.compilationDatabasePath)
     Opts.CompileCommandsDir = Dir;
   if (Opts.UseDirBasedCDB) {
-    BaseCDB = std::make_unique<DirectoryBasedGlobalCompilationDatabase>(
-        Opts.CompileCommandsDir);
+    DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS);
+    CDBOpts.CompileCommandsDir = Opts.CompileCommandsDir;
+    BaseCDB =
+        std::make_unique<DirectoryBasedGlobalCompilationDatabase>(CDBOpts);
     BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs),
                                      std::move(BaseCDB));
   }
@@ -704,6 +706,10 @@ void ClangdLSPServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
   //  - this is useful e.g. when switching git branches, but we're likely to see
   //    fresh headers but still have the old-branch main-file content
   Server->onFileEvent(Params);
+  // FIXME: observe config files, immediately expire time-based caches, reparse:
+  //  - compile_commands.json and compile_flags.txt
+  //  - .clang_format and .clang-tidy
+  //  - .clangd and clangd/config.yaml
 }
 
 void ClangdLSPServer::onCommand(const ExecuteCommandParams &Params,

diff  --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
index add0eec4a2c8..41b549cefc7c 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -8,11 +8,15 @@
 
 #include "GlobalCompilationDatabase.h"
 #include "FS.h"
+#include "SourceCode.h"
 #include "support/Logger.h"
 #include "support/Path.h"
+#include "support/ThreadsafeFS.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/CompilationDatabasePluginRegistry.h"
+#include "clang/Tooling/JSONCompilationDatabase.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
@@ -22,6 +26,7 @@
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include <chrono>
 #include <string>
 #include <tuple>
@@ -85,33 +90,78 @@ GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
 //  - 1) determine all the paths that might be searched
 //  - 2) acquire the map lock and get-or-create all the DirectoryCache entries
 //  - 3) release the map lock and query the caches as desired
-//
-// FIXME: this should revalidate the cache sometimes
-// FIXME: IO should go through a VFS
 class DirectoryBasedGlobalCompilationDatabase::DirectoryCache {
-  // Absolute canonical path that we're the cache for. (Not case-folded).
-  const std::string Path;
-
-  // True if we've looked for a CDB here and found none.
-  // (This makes it possible for get() to return without taking a lock)
-  // FIXME: this should have an expiry time instead of lasting forever.
-  std::atomic<bool> FinalizedNoCDB = {false};
-
-  // Guards following cache state.
+  using stopwatch = std::chrono::steady_clock;
+
+  // CachedFile is used to read a CDB file on disk (e.g. compile_commands.json).
+  // It specializes in being able to quickly bail out if the file is unchanged,
+  // which is the common case.
+  // Internally, it stores file metadata so a stat() can verify it's unchanged.
+  // We don't actually cache the content as it's not needed - if the file is
+  // unchanged then the previous CDB is valid.
+  struct CachedFile {
+    CachedFile(llvm::StringRef Parent, llvm::StringRef Rel) {
+      llvm::SmallString<256> Path = Parent;
+      llvm::sys::path::append(Path, Rel);
+      this->Path = Path.str().str();
+    }
+    std::string Path;
+    size_t Size = NoFileCached;
+    llvm::sys::TimePoint<> ModifiedTime;
+    FileDigest ContentHash;
+
+    static constexpr size_t NoFileCached = -1;
+
+    struct LoadResult {
+      enum {
+        FileNotFound,
+        TransientError,
+        FoundSameData,
+        FoundNewData,
+      } Result;
+      std::unique_ptr<llvm::MemoryBuffer> Buffer; // Set only if FoundNewData
+    };
+
+    LoadResult load(llvm::vfs::FileSystem &FS, bool HasOldData);
+  };
+
+  // If we've looked for a CDB here and found none, the time when that happened.
+  // (Atomics make it possible for get() to return without taking a lock)
+  std::atomic<stopwatch::rep> NoCDBAt = {
+      stopwatch::time_point::min().time_since_epoch().count()};
+
+  // Guards the following cache state.
   std::mutex Mu;
-  // Has cache been filled from disk? FIXME: this should be an expiry time.
-  bool CachePopulated = false;
+  // When was the cache last known to be in sync with disk state?
+  stopwatch::time_point CachePopulatedAt = stopwatch::time_point::min();
   // Whether a new CDB has been loaded but not broadcast yet.
   bool NeedsBroadcast = false;
-  // Last loaded CDB, meaningful if CachePopulated is set.
+  // Last loaded CDB, meaningful if CachePopulatedAt was ever set.
   // shared_ptr so we can overwrite this when callers are still using the CDB.
   std::shared_ptr<tooling::CompilationDatabase> CDB;
+  // File metadata for the CDB files we support tracking directly.
+  CachedFile CompileCommandsJson;
+  CachedFile BuildCompileCommandsJson;
+  CachedFile CompileFlagsTxt;
+  // CachedFile member corresponding to CDB.
+  //   CDB  | ACF  | Scenario
+  //   null | null | no CDB found, or initial empty cache
+  //   set  | null | CDB was loaded via generic plugin interface
+  //   null | set  | found known CDB file, but parsing it failed
+  //   set  | set  | CDB was parsed from a known file
+  CachedFile *ActiveCachedFile = nullptr;
 
 public:
-  DirectoryCache(llvm::StringRef Path) : Path(Path) {
+  DirectoryCache(llvm::StringRef Path)
+      : CompileCommandsJson(Path, "compile_commands.json"),
+        BuildCompileCommandsJson(Path, "build/compile_commands.json"),
+        CompileFlagsTxt(Path, "compile_flags.txt"), Path(Path) {
     assert(llvm::sys::path::is_absolute(Path));
   }
 
+  // Absolute canonical path that we're the cache for. (Not case-folded).
+  const std::string Path;
+
   // Get the CDB associated with this directory.
   // ShouldBroadcast:
   //  - as input, signals whether the caller is willing to broadcast a
@@ -120,12 +170,15 @@ class DirectoryBasedGlobalCompilationDatabase::DirectoryCache {
   // (If a new CDB is discovered and ShouldBroadcast is false, we mark the
   // CDB as needing broadcast, and broadcast it next time we can).
   std::shared_ptr<const tooling::CompilationDatabase>
-  get(bool &ShouldBroadcast) {
+  get(const ThreadsafeFS &TFS, bool &ShouldBroadcast,
+      stopwatch::time_point FreshTime, stopwatch::time_point FreshTimeMissing) {
     // Fast path for common case without taking lock.
-    if (FinalizedNoCDB.load()) {
+    if (stopwatch::time_point(stopwatch::duration(NoCDBAt.load())) >
+        FreshTimeMissing) {
       ShouldBroadcast = false;
       return nullptr;
     }
+
     std::lock_guard<std::mutex> Lock(Mu);
     auto RequestBroadcast = llvm::make_scope_exit([&, OldCDB(CDB.get())] {
       // If we loaded a new CDB, it should be broadcast at some point.
@@ -140,50 +193,172 @@ class DirectoryBasedGlobalCompilationDatabase::DirectoryCache {
       NeedsBroadcast = false;
     });
 
-    // For now, we never actually attempt to revalidate a populated cache.
-    if (CachePopulated)
+    // If our cache is valid, serve from it.
+    if (CachePopulatedAt > FreshTime)
       return CDB;
-    assert(CDB == nullptr);
 
-    load();
-    CachePopulated = true;
+    if (/*MayCache=*/load(*TFS.view(/*CWD=*/llvm::None))) {
+      // Use new timestamp, as loading may be slow.
+      CachePopulatedAt = stopwatch::now();
+      NoCDBAt.store((CDB ? stopwatch::time_point::min() : CachePopulatedAt)
+                        .time_since_epoch()
+                        .count());
+    }
 
-    if (!CDB)
-      FinalizedNoCDB.store(true);
     return CDB;
   }
 
-  llvm::StringRef path() const { return Path; }
-
 private:
-  // Updates `CDB` from disk state.
-  void load() {
-    std::string Error; // ignored, because it's often "didn't find anything".
-    CDB = tooling::CompilationDatabase::loadFromDirectory(Path, Error);
-    if (!CDB) {
-      // Fallback: check for $src/build, the conventional CMake build root.
-      // Probe existence first to avoid each plugin doing IO if it doesn't
-      // exist.
-      llvm::SmallString<256> BuildDir(Path);
-      llvm::sys::path::append(BuildDir, "build");
-      if (llvm::sys::fs::is_directory(BuildDir)) {
-        vlog("Found candidate build directory {0}", BuildDir);
-        CDB = tooling::CompilationDatabase::loadFromDirectory(BuildDir, Error);
+  // Updates `CDB` from disk state. Returns false on failure.
+  bool load(llvm::vfs::FileSystem &FS);
+};
+
+DirectoryBasedGlobalCompilationDatabase::DirectoryCache::CachedFile::LoadResult
+DirectoryBasedGlobalCompilationDatabase::DirectoryCache::CachedFile::load(
+    llvm::vfs::FileSystem &FS, bool HasOldData) {
+  auto Stat = FS.status(Path);
+  if (!Stat || !Stat->isRegularFile()) {
+    Size = NoFileCached;
+    ContentHash = {};
+    return {LoadResult::FileNotFound, nullptr};
+  }
+  // If both the size and mtime match, presume unchanged without reading.
+  if (HasOldData && Stat->getLastModificationTime() == ModifiedTime &&
+      Stat->getSize() == Size)
+    return {LoadResult::FoundSameData, nullptr};
+  auto Buf = FS.getBufferForFile(Path);
+  if (!Buf || (*Buf)->getBufferSize() != Stat->getSize()) {
+    // Don't clear the cache - possible we're seeing inconsistent size as the
+    // file is being recreated. If it ends up identical later, great!
+    //
+    // This isn't a complete solution: if we see a partial file but stat/read
+    // agree on its size, we're ultimately going to have spurious CDB reloads.
+    // May be worth fixing if generators don't write atomically (CMake does).
+    elog("Failed to read {0}: {1}", Path,
+         Buf ? "size changed" : Buf.getError().message());
+    return {LoadResult::TransientError, nullptr};
+  }
+
+  FileDigest NewContentHash = digest((*Buf)->getBuffer());
+  if (HasOldData && NewContentHash == ContentHash) {
+    // mtime changed but data is the same: avoid rebuilding the CDB.
+    ModifiedTime = Stat->getLastModificationTime();
+    return {LoadResult::FoundSameData, nullptr};
+  }
+
+  Size = (*Buf)->getBufferSize();
+  ModifiedTime = Stat->getLastModificationTime();
+  ContentHash = NewContentHash;
+  return {LoadResult::FoundNewData, std::move(*Buf)};
+}
+
+// Adapt CDB-loading functions to a common interface for DirectoryCache::load().
+static std::unique_ptr<tooling::CompilationDatabase>
+parseJSON(PathRef Path, llvm::StringRef Data, std::string &Error) {
+  if (auto CDB = tooling::JSONCompilationDatabase::loadFromBuffer(
+          Data, Error, tooling::JSONCommandLineSyntax::AutoDetect)) {
+    // FS used for expanding response files.
+    // FIXME: ExpandResponseFilesDatabase appears not to provide the usual
+    // thread-safety guarantees, as the access to FS is not locked!
+    // For now, use the real FS, which is known to be threadsafe (if we don't
+    // use/change working directory, which ExpandResponseFilesDatabase doesn't).
+    auto FS = llvm::vfs::getRealFileSystem();
+    return tooling::inferTargetAndDriverMode(
+        tooling::inferMissingCompileCommands(
+            expandResponseFiles(std::move(CDB), std::move(FS))));
+  }
+  return nullptr;
+}
+static std::unique_ptr<tooling::CompilationDatabase>
+parseFixed(PathRef Path, llvm::StringRef Data, std::string &Error) {
+  return tooling::FixedCompilationDatabase::loadFromBuffer(Path, Data, Error);
+}
+
+bool DirectoryBasedGlobalCompilationDatabase::DirectoryCache::load(
+    llvm::vfs::FileSystem &FS) {
+  dlog("Probing directory {0}", Path);
+  std::string Error;
+
+  // Load from the specially-supported compilation databases (JSON + Fixed).
+  // For these, we know the files they read and cache their metadata so we can
+  // cheaply validate whether they've changed, and hot-reload if they have.
+  // (As a bonus, these are also VFS-clean)!
+  struct CDBFile {
+    CachedFile *File;
+    // Wrapper for {Fixed,JSON}CompilationDatabase::loadFromBuffer.
+    llvm::function_ref<std::unique_ptr<tooling::CompilationDatabase>(
+        PathRef,
+        /*Data*/ llvm::StringRef,
+        /*ErrorMsg*/ std::string &)>
+        Parser;
+  };
+  for (const auto &Entry : {CDBFile{&CompileCommandsJson, parseJSON},
+                            CDBFile{&BuildCompileCommandsJson, parseJSON},
+                            CDBFile{&CompileFlagsTxt, parseFixed}}) {
+    bool Active = ActiveCachedFile == Entry.File;
+    auto Loaded = Entry.File->load(FS, Active);
+    switch (Loaded.Result) {
+    case CachedFile::LoadResult::FileNotFound:
+      if (Active) {
+        log("Unloaded compilation database from {0}", Entry.File->Path);
+        ActiveCachedFile = nullptr;
+        CDB = nullptr;
       }
+      // Continue looking at other candidates.
+      break;
+    case CachedFile::LoadResult::TransientError:
+      // File existed but we couldn't read it. Reuse the cache, retry later.
+      return false; // Load again next time.
+    case CachedFile::LoadResult::FoundSameData:
+      assert(Active && "CachedFile may not return 'same data' if !HasOldData");
+      // This is the critical file, and it hasn't changed.
+      return true;
+    case CachedFile::LoadResult::FoundNewData:
+      // We have a new CDB!
+      CDB = Entry.Parser(Entry.File->Path, Loaded.Buffer->getBuffer(), Error);
+      if (CDB)
+        log("{0} compilation database from {1}", Active ? "Reloaded" : "Loaded",
+            Entry.File->Path);
+      else
+        elog("Failed to load compilation database from {0}: {1}",
+             Entry.File->Path, Error);
+      ActiveCachedFile = Entry.File;
+      return true;
     }
-    if (CDB) {
-      log("Loaded compilation database from {0}", Path);
-    } else {
-      vlog("No compilation database at {0}", Path);
+  }
+
+  // Fall back to generic handling of compilation databases.
+  // We don't know what files they read, so can't efficiently check whether
+  // they need to be reloaded. So we never do that.
+  // FIXME: the interface doesn't provide a way to virtualize FS access.
+
+  // Don't try these more than once. If we've scanned before, we're done.
+  if (CachePopulatedAt > stopwatch::time_point::min())
+    return true;
+  for (const auto &Entry :
+       tooling::CompilationDatabasePluginRegistry::entries()) {
+    // Avoid duplicating the special cases handled above.
+    if (Entry.getName() == "fixed-compilation-database" ||
+        Entry.getName() == "json-compilation-database")
+      continue;
+    auto Plugin = Entry.instantiate();
+    if (auto CDB = Plugin->loadFromDirectory(Path, Error)) {
+      log("Loaded compilation database from {0} with plugin {1}", Path,
+          Entry.getName());
+      this->CDB = std::move(CDB);
+      return true;
     }
+    // Don't log Error here, it's usually just "couldn't find <file>".
   }
-};
+  vlog("No compilation database at {0}", Path);
+  return true;
+}
 
 DirectoryBasedGlobalCompilationDatabase::
-    DirectoryBasedGlobalCompilationDatabase(
-        llvm::Optional<Path> CompileCommandsDir) {
-  if (CompileCommandsDir)
-    OnlyDirCache = std::make_unique<DirectoryCache>(*CompileCommandsDir);
+    DirectoryBasedGlobalCompilationDatabase(const Options &Opts)
+    : Opts(Opts) {
+  if (Opts.CompileCommandsDir)
+    OnlyDirCache = std::make_unique<DirectoryCache>(*Opts.CompileCommandsDir);
 }
 
 DirectoryBasedGlobalCompilationDatabase::
@@ -194,6 +369,9 @@ DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const {
   CDBLookupRequest Req;
   Req.FileName = File;
   Req.ShouldBroadcast = true;
+  auto Now = std::chrono::steady_clock::now();
+  Req.FreshTime = Now - Opts.RevalidateAfter;
+  Req.FreshTimeMissing = Now - Opts.RevalidateMissingAfter;
 
   auto Res = lookupCDB(Req);
   if (!Res) {
@@ -264,7 +442,8 @@ DirectoryBasedGlobalCompilationDatabase::lookupCDB(
   if (OnlyDirCache) {
     DirCache = OnlyDirCache.get();
     ShouldBroadcast = Request.ShouldBroadcast;
-    CDB = DirCache->get(ShouldBroadcast);
+    CDB = DirCache->get(Opts.TFS, ShouldBroadcast, Request.FreshTime,
+                        Request.FreshTimeMissing);
   } else {
     // Traverse the canonical version to prevent false positives. i.e.:
     // src/build/../a.cc can detect a CDB in /src/build if not canonicalized.
@@ -276,7 +455,8 @@ DirectoryBasedGlobalCompilationDatabase::lookupCDB(
     });
     for (DirectoryCache *Candidate : getDirectoryCaches(SearchDirs)) {
       bool CandidateShouldBroadcast = Request.ShouldBroadcast;
-      if ((CDB = Candidate->get(CandidateShouldBroadcast))) {
+      if ((CDB = Candidate->get(Opts.TFS, CandidateShouldBroadcast,
+                                Request.FreshTime, Request.FreshTimeMissing))) {
         DirCache = Candidate;
         ShouldBroadcast = CandidateShouldBroadcast;
         break;
@@ -289,7 +469,7 @@ DirectoryBasedGlobalCompilationDatabase::lookupCDB(
 
   CDBLookupResult Result;
   Result.CDB = std::move(CDB);
-  Result.PI.SourceRoot = DirCache->path().str();
+  Result.PI.SourceRoot = DirCache->Path;
 
   // FIXME: Maybe make the following part async, since this can block
   // retrieval of compile commands.
@@ -300,13 +480,14 @@ DirectoryBasedGlobalCompilationDatabase::lookupCDB(
 
 void DirectoryBasedGlobalCompilationDatabase::broadcastCDB(
     CDBLookupResult Result) const {
+  vlog("Broadcasting compilation database from {0}", Result.PI.SourceRoot);
   assert(Result.CDB && "Trying to broadcast an invalid CDB!");
 
   std::vector<std::string> AllFiles = Result.CDB->getAllFiles();
   // We assume CDB in CompileCommandsDir owns all of its entries, since we don't
   // perform any search in parent paths whenever it is set.
   if (OnlyDirCache) {
-    assert(OnlyDirCache->path() == Result.PI.SourceRoot &&
+    assert(OnlyDirCache->Path == Result.PI.SourceRoot &&
            "Trying to broadcast a CDB outside of CompileCommandsDir!");
     OnCommandChanged.broadcast(std::move(AllFiles));
     return;
@@ -327,10 +508,14 @@ void DirectoryBasedGlobalCompilationDatabase::broadcastCDB(
     });
   }
   // Work out which ones have CDBs in them.
+  // Given that we know that CDBs have been moved/generated, don't trust caches.
+  // (This should be rare, so it's OK to add a little latency).
+  constexpr auto IgnoreCache = std::chrono::steady_clock::time_point::max();
   for (DirectoryCache *Dir : getDirectoryCaches(FileAncestors)) {
     bool ShouldBroadcast = false;
-    if (Dir->get(ShouldBroadcast))
-      DirectoryHasCDB.find(Dir->path())->setValue(true);
+    if (Dir->get(Opts.TFS, ShouldBroadcast, /*FreshTime=*/IgnoreCache,
+                 /*FreshTimeMissing=*/IgnoreCache))
+      DirectoryHasCDB.find(Dir->Path)->setValue(true);
   }
 
   std::vector<std::string> GovernedFiles;
@@ -357,6 +542,8 @@ DirectoryBasedGlobalCompilationDatabase::getProjectInfo(PathRef File) const {
   CDBLookupRequest Req;
   Req.FileName = File;
   Req.ShouldBroadcast = false;
+  Req.FreshTime = Req.FreshTimeMissing =
+      std::chrono::steady_clock::time_point::min();
   auto Res = lookupCDB(Req);
   if (!Res)
     return llvm::None;

diff  --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.h b/clang-tools-extra/clangd/GlobalCompilationDatabase.h
index e654ccbd12c6..9fb6f15f13d2 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -12,6 +12,7 @@
 #include "CompileCommands.h"
 #include "support/Function.h"
 #include "support/Path.h"
+#include "support/ThreadsafeFS.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/Optional.h"
@@ -66,8 +67,22 @@ class GlobalCompilationDatabase {
 class DirectoryBasedGlobalCompilationDatabase
     : public GlobalCompilationDatabase {
 public:
-  DirectoryBasedGlobalCompilationDatabase(
-      llvm::Optional<Path> CompileCommandsDir);
+  struct Options {
+    Options(const ThreadsafeFS &TFS) : TFS(TFS) {}
+
+    const ThreadsafeFS &TFS;
+    // Frequency to check whether e.g. compile_commands.json has changed.
+    std::chrono::steady_clock::duration RevalidateAfter =
+        std::chrono::seconds(5);
+    // Frequency to check whether e.g. compile_commands.json has been created.
+    // (This is more expensive to check frequently, as we check many locations).
+    std::chrono::steady_clock::duration RevalidateMissingAfter =
+        std::chrono::seconds(30);
+    // Only look for a compilation database in this one fixed directory.
+    llvm::Optional<Path> CompileCommandsDir;
+  };
+
+  DirectoryBasedGlobalCompilationDatabase(const Options &Opts);
   ~DirectoryBasedGlobalCompilationDatabase() override;
 
   /// Scans File's parents looking for compilation databases.
@@ -81,6 +96,8 @@ class DirectoryBasedGlobalCompilationDatabase
   llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override;
 
 private:
+  Options Opts;
+
   class DirectoryCache;
   // If there's an explicit CompileCommandsDir, cache of the CDB found there.
   mutable std::unique_ptr<DirectoryCache> OnlyDirCache;
@@ -99,6 +116,10 @@ class DirectoryBasedGlobalCompilationDatabase
     PathRef FileName;
     // Whether this lookup should trigger discovery of the CDB found.
     bool ShouldBroadcast = false;
+    // Cached results newer than this are considered fresh and not checked
+    // against disk.
+    std::chrono::steady_clock::time_point FreshTime;
+    std::chrono::steady_clock::time_point FreshTimeMissing;
   };
   struct CDBLookupResult {
     std::shared_ptr<const tooling::CompilationDatabase> CDB;
@@ -108,6 +129,9 @@ class DirectoryBasedGlobalCompilationDatabase
 
   // Performs broadcast on governed files.
   void broadcastCDB(CDBLookupResult Res) const;
+
+  // cache test calls lookupCDB directly to ensure valid/invalid times.
+  friend class DirectoryBasedGlobalCompilationDatabaseCacheTest;
 };
 
 /// Extracts system include search path from drivers matching QueryDriverGlobs

diff  --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp
index 9b40e840478e..18d2837b89c2 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -90,11 +90,12 @@ class Checker {
       : File(File), Opts(Opts) {}
 
   // Read compilation database and choose a compile command for the file.
-  bool buildCommand() {
+  bool buildCommand(const ThreadsafeFS &TFS) {
     log("Loading compilation database...");
+    DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS);
+    CDBOpts.CompileCommandsDir = Opts.CompileCommandsDir;
     std::unique_ptr<GlobalCompilationDatabase> BaseCDB =
-        std::make_unique<DirectoryBasedGlobalCompilationDatabase>(
-            Opts.CompileCommandsDir);
+        std::make_unique<DirectoryBasedGlobalCompilationDatabase>(CDBOpts);
     BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs),
                                      std::move(BaseCDB));
     auto Mangler = CommandMangler::detect();
@@ -244,7 +245,8 @@ bool check(llvm::StringRef File, const ThreadsafeFS &TFS,
   log("Testing on source file {0}", File);
 
   Checker C(File, Opts);
-  if (!C.buildCommand() || !C.buildInvocation(TFS, Contents) || !C.buildAST())
+  if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
+      !C.buildAST())
     return false;
   C.testLocationFeatures();
 

diff  --git a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
index ef9a299483f6..d9fccb835c96 100644
--- a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -11,8 +11,10 @@
 #include "Matchers.h"
 #include "TestFS.h"
 #include "support/Path.h"
+#include "support/ThreadsafeFS.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
@@ -23,6 +25,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include <chrono>
 #include <fstream>
 #include <string>
 
@@ -40,7 +43,8 @@ using ::testing::StartsWith;
 using ::testing::UnorderedElementsAre;
 
 TEST(GlobalCompilationDatabaseTest, FallbackCommand) {
-  DirectoryBasedGlobalCompilationDatabase DB(None);
+  MockFS TFS;
+  DirectoryBasedGlobalCompilationDatabase DB(TFS);
   auto Cmd = DB.getFallbackCommand(testPath("foo/bar.cc"));
   EXPECT_EQ(Cmd.Directory, testPath("foo"));
   EXPECT_THAT(Cmd.CommandLine, ElementsAre("clang", testPath("foo/bar.cc")));
@@ -166,6 +170,7 @@ TEST_F(OverlayCDBTest, Adjustments) {
 }
 
 // Allows placement of files for tests and cleans them up after.
+// FIXME: GlobalCompilationDatabase is mostly VFS-clean now, switch to MockFS?
 class ScratchFS {
   llvm::SmallString<128> Root;
 
@@ -238,13 +243,14 @@ TEST(GlobalCompilationDatabaseTest, DiscoveryWithNestedCDBs) {
       ]
       )cdb";
   ScratchFS FS;
+  RealThreadsafeFS TFS;
   FS.write("compile_commands.json", CDBOuter);
   FS.write("build/compile_commands.json", CDBInner);
 
   // Note that gen2.cc goes missing with our following model, not sure this
   // happens in practice though.
   {
-    DirectoryBasedGlobalCompilationDatabase DB(llvm::None);
+    DirectoryBasedGlobalCompilationDatabase DB(TFS);
     std::vector<std::string> DiscoveredFiles;
     auto Sub =
         DB.watch([&DiscoveredFiles](const std::vector<std::string> Changes) {
@@ -262,7 +268,9 @@ TEST(GlobalCompilationDatabaseTest, DiscoveryWithNestedCDBs) {
 
   // With a custom compile commands dir.
   {
-    DirectoryBasedGlobalCompilationDatabase DB(FS.root().str());
+    DirectoryBasedGlobalCompilationDatabase::Options Opts(TFS);
+    Opts.CompileCommandsDir = FS.root().str();
+    DirectoryBasedGlobalCompilationDatabase DB(Opts);
     std::vector<std::string> DiscoveredFiles;
     auto Sub =
         DB.watch([&DiscoveredFiles](const std::vector<std::string> Changes) {
@@ -282,17 +290,34 @@ TEST(GlobalCompilationDatabaseTest, DiscoveryWithNestedCDBs) {
 
 TEST(GlobalCompilationDatabaseTest, BuildDir) {
   ScratchFS FS;
+  RealThreadsafeFS TFS;
   auto Command = [&](llvm::StringRef Relative) {
-    return DirectoryBasedGlobalCompilationDatabase(llvm::None)
+    DirectoryBasedGlobalCompilationDatabase::Options Opts(TFS);
+    return DirectoryBasedGlobalCompilationDatabase(Opts)
         .getCompileCommand(FS.path(Relative))
         .getValueOr(tooling::CompileCommand())
         .CommandLine;
   };
   EXPECT_THAT(Command("x/foo.cc"), IsEmpty());
-  FS.write("x/build/compile_flags.txt", "-DXYZZY");
+  const char *const CDB =
+      R"cdb(
+      [
+        {
+          "file": "{0}/x/foo.cc",
+          "command": "clang -DXYZZY {0}/x/foo.cc",
+          "directory": "{0}",
+        },
+        {
+          "file": "{0}/bar.cc",
+          "command": "clang -DXYZZY {0}/bar.cc",
+          "directory": "{0}",
+        }
+      ]
+      )cdb";
+  FS.write("x/build/compile_commands.json", CDB);
   EXPECT_THAT(Command("x/foo.cc"), Contains("-DXYZZY"));
   EXPECT_THAT(Command("bar.cc"), IsEmpty())
-      << "x/build/compile_flags.txt only applicable to x/";
+      << "x/build/compile_flags.json only applicable to x/";
 }
 
 TEST(GlobalCompilationDatabaseTest, NonCanonicalFilenames) {
@@ -330,5 +355,108 @@ TEST_F(OverlayCDBTest, GetProjectInfo) {
   EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
 }
 } // namespace
+
+// Friend test has access to internals.
+class DirectoryBasedGlobalCompilationDatabaseCacheTest
+    : public ::testing::Test {
+protected:
+  std::shared_ptr<const tooling::CompilationDatabase>
+  lookupCDB(const DirectoryBasedGlobalCompilationDatabase &GDB,
+            llvm::StringRef Path,
+            std::chrono::steady_clock::time_point FreshTime) {
+    DirectoryBasedGlobalCompilationDatabase::CDBLookupRequest Req;
+    Req.FileName = Path;
+    Req.FreshTime = Req.FreshTimeMissing = FreshTime;
+    if (auto Result = GDB.lookupCDB(Req))
+      return std::move(Result->CDB);
+    return nullptr;
+  }
+};
+
+// Matches non-null CDBs which include the specified flag.
+MATCHER_P2(hasFlag, Flag, Path, "") {
+  if (arg == nullptr)
+    return false;
+  auto Cmds = arg->getCompileCommands(Path);
+  if (Cmds.empty()) {
+    *result_listener << "yields no commands";
+    return false;
+  }
+  if (!llvm::is_contained(Cmds.front().CommandLine, Flag)) {
+    *result_listener << "flags are: "
+                     << llvm::join(Cmds.front().CommandLine, " ");
+    return false;
+  }
+  return true;
+}
+
+auto hasFlag(llvm::StringRef Flag) { return hasFlag(Flag, "dummy.cc"); }
+
+TEST_F(DirectoryBasedGlobalCompilationDatabaseCacheTest, Cacheable) {
+  MockFS FS;
+  auto Stale = std::chrono::steady_clock::now() - std::chrono::minutes(1);
+  auto Fresh = std::chrono::steady_clock::now() + std::chrono::hours(24);
+
+  DirectoryBasedGlobalCompilationDatabase GDB(FS);
+  FS.Files["compile_flags.txt"] = "-DROOT";
+  auto Root = lookupCDB(GDB, testPath("foo/test.cc"), Stale);
+  EXPECT_THAT(Root, hasFlag("-DROOT"));
+
+  // Add a compilation database to a subdirectory - CDB loaded.
+  FS.Files["foo/compile_flags.txt"] = "-DFOO";
+  EXPECT_EQ(Root, lookupCDB(GDB, testPath("foo/test.cc"), Stale))
+      << "cache still valid";
+  auto Foo = lookupCDB(GDB, testPath("foo/test.cc"), Fresh);
+  EXPECT_THAT(Foo, hasFlag("-DFOO")) << "new cdb loaded";
+  EXPECT_EQ(Foo, lookupCDB(GDB, testPath("foo/test.cc"), Stale))
+      << "new cdb in cache";
+
+  // Mtime changed, but no content change - CDB not reloaded.
+  ++FS.Timestamps["foo/compile_flags.txt"];
+  auto FooAgain = lookupCDB(GDB, testPath("foo/test.cc"), Fresh);
+  EXPECT_EQ(Foo, FooAgain) << "Same content, read but not reloaded";
+  // Content changed, but not size or mtime - CDB not reloaded.
+  FS.Files["foo/compile_flags.txt"] = "-DBAR";
+  auto FooAgain2 = lookupCDB(GDB, testPath("foo/test.cc"), Fresh);
+  EXPECT_EQ(Foo, FooAgain2) << "Same filesize, change not detected";
+  // Mtime change forces a re-read, and we notice the 
diff erent content.
+  ++FS.Timestamps["foo/compile_flags.txt"];
+  auto Bar = lookupCDB(GDB, testPath("foo/test.cc"), Fresh);
+  EXPECT_THAT(Bar, hasFlag("-DBAR")) << "refreshed with mtime change";
+
+  // Size and content both change - CDB reloaded.
+  FS.Files["foo/compile_flags.txt"] = "-DFOOBAR";
+  EXPECT_EQ(Bar, lookupCDB(GDB, testPath("foo/test.cc"), Stale))
+      << "cache still valid";
+  auto FooBar = lookupCDB(GDB, testPath("foo/test.cc"), Fresh);
+  EXPECT_THAT(FooBar, hasFlag("-DFOOBAR")) << "cdb reloaded";
+
+  // compile_commands.json takes precedence over compile_flags.txt.
+  FS.Files["foo/compile_commands.json"] = llvm::formatv(R"json([{
+    "file": "{0}/foo/dummy.cc",
+    "command": "clang -DBAZ dummy.cc",
+    "directory": "{0}/foo",
+  }])json",
+                                                        testRoot());
+  EXPECT_EQ(FooBar, lookupCDB(GDB, testPath("foo/test.cc"), Stale))
+      << "cache still valid";
+  auto Baz = lookupCDB(GDB, testPath("foo/test.cc"), Fresh);
+  EXPECT_THAT(Baz, hasFlag("-DBAZ", testPath("foo/dummy.cc")))
+      << "compile_commands overrides compile_flags";
+
+  // Removing compile_commands.json reveals compile_flags.txt again.
+  // However this *does* cause a CDB reload (we cache only one CDB per dir).
+  FS.Files.erase("foo/compile_commands.json");
+  auto FoobarAgain = lookupCDB(GDB, testPath("foo/test.cc"), Fresh);
+  EXPECT_THAT(FoobarAgain, hasFlag("-DFOOBAR")) << "reloaded compile_flags";
+  EXPECT_NE(FoobarAgain, FooBar) << "CDB discarded (shadowed within directory)";
+
+  // Removing the directory's CDB leaves the parent CDB active.
+  // The parent CDB is *not* reloaded (we cache the CDB per-directory).
+  FS.Files.erase("foo/compile_flags.txt");
+  EXPECT_EQ(Root, lookupCDB(GDB, testPath("foo/test.cc"), Fresh))
+      << "CDB retained (shadowed by another directory)";
+}
+
 } // namespace clangd
 } // namespace clang


        


More information about the cfe-commits mailing list