[llvm-branch-commits] [clang-tools-extra] 67d16b6 - [clangd] Cache .clang-tidy files again.
Sam McCall via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sun Nov 29 04:33:41 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 llvm-branch-commits
mailing list