[clang-tools-extra] 67d16b6 - [clangd] Cache .clang-tidy files again.

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Sun Nov 29 04:29:09 PST 2020


Author: Sam McCall
Date: 2020-11-29T13:28:53+01:00
New Revision: 67d16b6da4bef1ee174514148854e77151a62605

URL: https://github.com/llvm/llvm-project/commit/67d16b6da4bef1ee174514148854e77151a62605
DIFF: https://github.com/llvm/llvm-project/commit/67d16b6da4bef1ee174514148854e77151a62605.diff

LOG: [clangd] Cache .clang-tidy files again.

This cache went away in 73fdd998701cce3aa6c4d8d2a73ab97351a0313b

This time, the cache is periodically validated against disk, so config
is still mostly "live".

The per-file cache reuses FileCache, but the tree-of-file-caches is
duplicated from ConfigProvider. .clangd, .clang-tidy, .clang-format, and
compile_commands.json all have this pattern, we should extract it at some point.
TODO for now though.

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/TidyProvider.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/TidyProvider.cpp b/clang-tools-extra/clangd/TidyProvider.cpp
index 1fb79f3abbce3..687ae6501d25f 100644
--- a/clang-tools-extra/clangd/TidyProvider.cpp
+++ b/clang-tools-extra/clangd/TidyProvider.cpp
@@ -8,7 +8,9 @@
 
 #include "TidyProvider.h"
 #include "Config.h"
+#include "support/FileCache.h"
 #include "support/Logger.h"
+#include "support/ThreadsafeFS.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
@@ -19,53 +21,115 @@
 
 namespace clang {
 namespace clangd {
+namespace {
 
-static void mergeCheckList(llvm::Optional<std::string> &Checks,
-                           llvm::StringRef List) {
-  if (List.empty())
-    return;
-  if (!Checks || Checks->empty()) {
-    Checks.emplace(List);
-    return;
+// Access to config from a .clang-tidy file, caching IO and parsing.
+class DotClangTidyCache : private FileCache {
+  // We cache and expose shared_ptr to avoid copying the value on every lookup
+  // when we're ultimately just going to pass it to mergeWith.
+  mutable std::shared_ptr<const tidy::ClangTidyOptions> Value;
+
+public:
+  DotClangTidyCache(PathRef Path) : FileCache(Path) {}
+
+  std::shared_ptr<const tidy::ClangTidyOptions>
+  get(const ThreadsafeFS &TFS,
+      std::chrono::steady_clock::time_point FreshTime) const {
+    std::shared_ptr<const tidy::ClangTidyOptions> Result;
+    read(
+        TFS, FreshTime,
+        [this](llvm::Optional<llvm::StringRef> Data) {
+          Value.reset();
+          if (Data && !Data->empty()) {
+            if (auto Parsed = tidy::parseConfiguration(*Data))
+              Value = std::make_shared<const tidy::ClangTidyOptions>(
+                  std::move(*Parsed));
+            else
+              elog("Error parsing clang-tidy configuration in {0}: {1}", path(),
+                   Parsed.getError().message());
+          }
+        },
+        [&]() { Result = Value; });
+    return Result;
   }
-  *Checks = llvm::join_items(",", *Checks, List);
-}
+};
 
-static llvm::Optional<tidy::ClangTidyOptions>
-tryReadConfigFile(llvm::vfs::FileSystem *FS, llvm::StringRef Directory) {
-  assert(!Directory.empty());
-  // We guaranteed that child directories of Directory exist, so this assert
-  // should hopefully never fail.
-  assert(FS->exists(Directory));
+// Access to combined config from .clang-tidy files governing a source file.
+// Each config file is cached and the caches are shared for affected sources.
+//
+// FIXME: largely duplicates config::Provider::fromAncestorRelativeYAMLFiles.
+// Potentially useful for compile_commands.json too. Extract?
+class DotClangTidyTree {
+  const ThreadsafeFS &FS;
+  std::string RelPath;
+  std::chrono::steady_clock::duration MaxStaleness;
 
-  llvm::SmallString<128> ConfigFile(Directory);
-  llvm::sys::path::append(ConfigFile, ".clang-tidy");
+  mutable std::mutex Mu;
+  // Keys are the ancestor directory, not the actual config path within it.
+  // We only insert into this map, so pointers to values are stable forever.
+  // Mutex guards the map itself, not the values (which are threadsafe).
+  mutable llvm::StringMap<DotClangTidyCache> Cache;
 
-  llvm::ErrorOr<llvm::vfs::Status> FileStatus = FS->status(ConfigFile);
+public:
+  DotClangTidyTree(const ThreadsafeFS &FS)
+      : FS(FS), RelPath(".clang-tidy"), MaxStaleness(std::chrono::seconds(5)) {}
 
-  if (!FileStatus || !FileStatus->isRegularFile())
-    return llvm::None;
+  void apply(tidy::ClangTidyOptions &Result, PathRef AbsPath) {
+    namespace path = llvm::sys::path;
+    assert(path::is_absolute(AbsPath));
+
+    // Compute absolute paths to all ancestors (substrings of P.Path).
+    // Ensure cache entries for each ancestor exist in the map.
+    llvm::StringRef Parent = path::parent_path(AbsPath);
+    llvm::SmallVector<DotClangTidyCache *, 8> Caches;
+    {
+      std::lock_guard<std::mutex> Lock(Mu);
+      for (auto I = path::begin(Parent, path::Style::posix),
+                E = path::end(Parent);
+           I != E; ++I) {
+        assert(I->end() >= Parent.begin() && I->end() <= Parent.end() &&
+               "Canonical path components should be substrings");
+        llvm::StringRef Ancestor(Parent.begin(), I->end() - Parent.begin());
+        auto It = Cache.find(Ancestor);
 
-  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text =
-      FS->getBufferForFile(ConfigFile);
-  if (std::error_code EC = Text.getError()) {
-    elog("Can't read '{0}': {1}", ConfigFile, EC.message());
-    return llvm::None;
+        // Assemble the actual config file path only if needed.
+        if (It == Cache.end()) {
+          llvm::SmallString<256> ConfigPath = Ancestor;
+          path::append(ConfigPath, RelPath);
+          It = Cache.try_emplace(Ancestor, ConfigPath.str()).first;
+        }
+        Caches.push_back(&It->second);
+      }
+    }
+    // Finally query each individual file.
+    // This will take a (per-file) lock for each file that actually exists.
+    std::chrono::steady_clock::time_point FreshTime =
+        std::chrono::steady_clock::now() - MaxStaleness;
+    llvm::SmallVector<std::shared_ptr<const tidy::ClangTidyOptions>, 4>
+        OptionStack;
+    for (const DotClangTidyCache *Cache : Caches)
+      if (auto Config = Cache->get(FS, FreshTime)) {
+        OptionStack.push_back(std::move(Config));
+        if (!OptionStack.back()->InheritParentConfig.getValueOr(false))
+          break;
+      }
+    unsigned Order = 1u;
+    for (auto &Option : llvm::reverse(OptionStack))
+      Result.mergeWith(*Option, Order++);
   }
+};
 
-  // Skip empty files, e.g. files opened for writing via shell output
-  // redirection.
-  if ((*Text)->getBuffer().empty())
-    return llvm::None;
-  llvm::ErrorOr<tidy::ClangTidyOptions> ParsedOptions =
-      tidy::parseConfiguration((*Text)->getBuffer());
-  if (!ParsedOptions) {
-    if (ParsedOptions.getError())
-      elog("Error parsing clang-tidy configuration in '{0}': {1}", ConfigFile,
-           ParsedOptions.getError().message());
-    return llvm::None;
+} // namespace
+
+static void mergeCheckList(llvm::Optional<std::string> &Checks,
+                           llvm::StringRef List) {
+  if (List.empty())
+    return;
+  if (!Checks || Checks->empty()) {
+    Checks.emplace(List);
+    return;
   }
-  return std::move(*ParsedOptions);
+  *Checks = llvm::join_items(",", *Checks, List);
 }
 
 TidyProviderRef provideEnvironment() {
@@ -176,41 +240,9 @@ TidyProviderRef provideClangdConfig() {
 }
 
 TidyProvider provideClangTidyFiles(ThreadsafeFS &TFS) {
-  return [&TFS](tidy::ClangTidyOptions &Opts, llvm::StringRef Filename) {
-    llvm::SmallVector<tidy::ClangTidyOptions, 4> OptionStack;
-    auto FS(TFS.view(llvm::None));
-    llvm::SmallString<256> AbsolutePath(Filename);
-
-    assert(llvm::sys::path::is_absolute(AbsolutePath));
-
-    llvm::sys::path::remove_dots(AbsolutePath, true);
-    llvm::StringRef Directory = llvm::sys::path::parent_path(AbsolutePath);
-    {
-      auto Status = FS->status(Directory);
-
-      if (!Status || !Status->isDirectory()) {
-        elog("Error reading configuration from {0}: directory doesn't exist",
-             Directory);
-        return;
-      }
-    }
-
-    // FIXME: Store options in a cache that validates itself against changes
-    // during the clangd session.
-    for (llvm::StringRef CurrentDirectory = Directory;
-         !CurrentDirectory.empty();
-         CurrentDirectory = llvm::sys::path::parent_path(CurrentDirectory)) {
-      auto ConfigFile = tryReadConfigFile(FS.get(), CurrentDirectory);
-      if (!ConfigFile)
-        continue;
-      OptionStack.push_back(std::move(*ConfigFile));
-      // Should we search for a parent config to merge
-      if (!OptionStack.back().InheritParentConfig.getValueOr(false))
-        break;
-    }
-    unsigned Order = 1U;
-    for (auto &Option : llvm::reverse(OptionStack))
-      Opts.mergeWith(Option, Order++);
+  return [Tree = std::make_unique<DotClangTidyTree>(TFS)](
+             tidy::ClangTidyOptions &Opts, llvm::StringRef Filename) {
+    Tree->apply(Opts, Filename);
   };
 }
 


        


More information about the cfe-commits mailing list