[clangd] Config: loading and caching config from disk.

Summary:
The Provider extension point is designed to also be implemented by
ClangdLSPServer (to inject config-over-lsp) and likely by embedders.

Reviewers: kadircet

Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, jfb, usaxena95, cfe-commits

Tags: #clang

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

clang-tools-extra/clangd/CMakeLists.txt

@@ -38,6 +38,7 @@ add_clang_library(clangDaemon
   Compiler.cpp
   Config.cpp
   ConfigCompile.cpp
+  ConfigProvider.cpp
   ConfigYAML.cpp
   Diagnostics.cpp
   DraftStore.cpp

clang-tools-extra/clangd/ConfigProvider.cpp

@@ -0,0 +1,207 @@
+//===--- ConfigProvider.cpp - Loading of user configuration ---------------===//
+//
+// 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 "ConfigProvider.h"
+#include "Config.h"
+#include "ConfigFragment.h"
+#include "support/ThreadsafeFS.h"
+#include "support/Trace.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/Path.h"
+#include <mutex>
+
+namespace clang {
+namespace clangd {
+namespace config {
+
+// Threadsafe cache around reading a YAML config file from disk.
+class FileConfigCache {
+  std::mutex Mu;
+  llvm::SmallVector<CompiledFragment, 1> CachedValue;
+  llvm::sys::TimePoint<> MTime = {};
+  unsigned Size = -1;
+
+  void updateCacheLocked(const llvm::vfs::Status &Stat,
+                         llvm::vfs::FileSystem &FS, DiagnosticCallback DC) {
+    if (Size == Stat.getSize() && MTime == Stat.getLastModificationTime())
+      return; // Already valid.
+
+    Size = Stat.getSize();
+    MTime = Stat.getLastModificationTime();
+    CachedValue.clear();
+
+    auto Buf = FS.getBufferForFile(Path);
+    // If stat() succeeds but we failed to read, don't cache failure.
+    if (!Buf) {
+      Size = -1;
+      MTime = {};
+      return;
+    }
+
+    // If file changed between stat and open, we don't know its mtime.
+    // For simplicity, don't cache the value in this case (use a bad key).
+    if (Buf->get()->getBufferSize() != Size) {
+      Size = -1;
+      MTime = {};
+    }
+
+    // Finally parse and compile the actual fragments.
+    for (auto &Fragment :
+         Fragment::parseYAML(Buf->get()->getBuffer(), Path, DC))
+      CachedValue.push_back(std::move(Fragment).compile(DC));
+  }
+
+public:
+  // Must be set before the cache is used. Not a constructor param to allow
+  // computing ancestor-relative paths to be deferred.
+  std::string Path;
+
+  // Retrieves up-to-date config fragments from disk.
+  // A cached result may be reused if the mtime and size are unchanged.
+  // (But several concurrent read()s can miss the cache after a single change).
+  // Future performance ideas:
+  // - allow caches to be reused based on short elapsed walltime
+  // - allow latency-sensitive operations to skip revalidating the cache
+  void read(const ThreadsafeFS &TFS, DiagnosticCallback DC,
+            std::vector<CompiledFragment> &Out) {
+    assert(llvm::sys::path::is_absolute(Path));
+    auto FS = TFS.view(/*CWD=*/llvm::None);
+    auto Stat = FS->status(Path);
+    if (!Stat || !Stat->isRegularFile()) {
+      // No point taking the lock to clear the cache. We know what to return.
+      // If the file comes back we'll invalidate the cache at that point.
+      return;
+    }
+
+    std::lock_guard<std::mutex> Lock(Mu);
+    updateCacheLocked(*Stat, *FS, DC);
+    llvm::copy(CachedValue, std::back_inserter(Out));
+  }
+};
+
+std::unique_ptr<Provider> Provider::fromYAMLFile(llvm::StringRef AbsPath,
+                                                 const ThreadsafeFS &FS) {
+  class AbsFileProvider : public Provider {
+    mutable FileConfigCache Cache; // threadsafe
+    const ThreadsafeFS &FS;
+
+    std::vector<CompiledFragment>
+    getFragments(const Params &P, DiagnosticCallback DC) const override {
+      std::vector<CompiledFragment> Result;
+      Cache.read(FS, DC, Result);
+      return Result;
+    };
+
+  public:
+    AbsFileProvider(llvm::StringRef Path, const ThreadsafeFS &FS) : FS(FS) {
+      assert(llvm::sys::path::is_absolute(Path));
+      Cache.Path = Path.str();
+    }
+  };
+
+  return std::make_unique<AbsFileProvider>(AbsPath, FS);
+}
+
+std::unique_ptr<Provider>
+Provider::fromAncestorRelativeYAMLFiles(llvm::StringRef RelPath,
+                                        const ThreadsafeFS &FS) {
+  class RelFileProvider : public Provider {
+    std::string RelPath;
+    const ThreadsafeFS &FS;
+
+    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<FileConfigCache> Cache;
+
+    std::vector<CompiledFragment>
+    getFragments(const Params &P, DiagnosticCallback DC) const override {
+      namespace path = llvm::sys::path;
+
+      if (P.Path.empty())
+        return {};
+
+      // Compute absolute paths to all ancestors (substrings of P.Path).
+      llvm::StringRef Parent = path::parent_path(P.Path);
+      llvm::SmallVector<llvm::StringRef, 8> Ancestors;
+      for (auto I = path::begin(Parent, path::Style::posix),
+                E = path::end(Parent);
+           I != E; ++I) {
+        // Avoid weird non-substring cases like phantom "." components.
+        // In practice, Component is a substring for all "normal" ancestors.
+        if (I->end() < Parent.begin() && I->end() > Parent.end())
+          continue;
+        Ancestors.emplace_back(Parent.begin(), I->end() - Parent.begin());
+      }
+      // Ensure corresponding cache entries exist in the map.
+      llvm::SmallVector<FileConfigCache *, 8> Caches;
+      {
+        std::lock_guard<std::mutex> Lock(Mu);
+        for (llvm::StringRef Ancestor : Ancestors) {
+          auto R = Cache.try_emplace(Ancestor);
+          // Assemble the actual config file path only once.
+          if (R.second) {
+            llvm::SmallString<256> ConfigPath = Ancestor;
+            path::append(ConfigPath, RelPath);
+            R.first->second.Path = ConfigPath.str().str();
+          }
+          Caches.push_back(&R.first->second);
+        }
+      }
+      // Finally query each individual file.
+      // This will take a (per-file) lock for each file that actually exists.
+      std::vector<CompiledFragment> Result;
+      for (FileConfigCache *Cache : Caches)
+        Cache->read(FS, DC, Result);
+      return Result;
+    };
+
+  public:
+    RelFileProvider(llvm::StringRef RelPath, const ThreadsafeFS &FS)
+        : RelPath(RelPath), FS(FS) {
+      assert(llvm::sys::path::is_relative(RelPath));
+    }
+  };
+
+  return std::make_unique<RelFileProvider>(RelPath, FS);
+}
+
+std::unique_ptr<Provider>
+Provider::combine(std::vector<std::unique_ptr<Provider>> Providers) {
+  struct CombinedProvider : Provider {
+    std::vector<std::unique_ptr<Provider>> Providers;
+
+    std::vector<CompiledFragment>
+    getFragments(const Params &P, DiagnosticCallback DC) const override {
+      std::vector<CompiledFragment> Result;
+      for (const auto &Provider : Providers) {
+        for (auto &Fragment : Provider->getFragments(P, DC))
+          Result.push_back(std::move(Fragment));
+      }
+      return Result;
+    }
+  };
+  auto Result = std::make_unique<CombinedProvider>();
+  Result->Providers = std::move(Providers);
+  return Result;
+}
+
+Config Provider::getConfig(const Params &P, DiagnosticCallback DC) const {
+  trace::Span Tracer("getConfig");
+  if (!P.Path.empty())
+    SPAN_ATTACH(Tracer, "path", P.Path);
+  Config C;
+  for (const auto &Fragment : getFragments(P, DC))
+    Fragment(P, C);
+  return C;
+}
+
+} // namespace config
+} // namespace clangd
+} // namespace clang

clang-tools-extra/clangd/ConfigProvider.h

@@ -26,6 +26,7 @@
 namespace clang {
 namespace clangd {
 struct Config;
+class ThreadsafeFS;
 namespace config {
 
 /// Describes the context used to evaluate configuration fragments.
@@ -47,6 +48,48 @@ using DiagnosticCallback = llvm::function_ref<void(const llvm::SMDiagnostic &)>;
 /// Returns true if the condition was met and the settings were used.
 using CompiledFragment = std::function<bool(const Params &, Config &)>;
 
+/// A source of configuration fragments.
+/// Generally these providers reflect a fixed policy for obtaining config,
+/// but return different concrete configuration over time.
+/// e.g. a provider that reads config from files is responsive to file changes.
+class Provider {
+public:
+  virtual ~Provider() = default;
+
+  // Reads fragments from a single YAML file with a fixed path.
+  static std::unique_ptr<Provider> fromYAMLFile(llvm::StringRef AbsPathPath,
+                                                const ThreadsafeFS &);
+  // Reads fragments from YAML files found relative to ancestors of Params.Path.
+  //
+  // All fragments that exist are returned, starting from distant ancestors.
+  // For instance, given RelPath of ".clangd", then for source file /foo/bar.cc,
+  // the searched fragments are [/.clangd, /foo/.clangd].
+  //
+  // If Params does not specify a path, no fragments are returned.
+  static std::unique_ptr<Provider>
+  fromAncestorRelativeYAMLFiles(llvm::StringRef RelPath, const ThreadsafeFS &);
+
+  /// A provider that includes fragments from all the supplied providers.
+  /// Order is preserved; later providers take precedence over earlier ones.
+  static std::unique_ptr<Provider>
+      combine(std::vector<std::unique_ptr<Provider>>);
+
+  /// Build a config based on this provider.
+  Config getConfig(const Params &, DiagnosticCallback) const;
+
+private:
+  /// Provide fragments that may be relevant to the file.
+  /// The configuration provider is not responsible for testing conditions.
+  ///
+  /// Providers are expected to cache compiled fragments, and only
+  /// reparse/recompile when the source data has changed.
+  /// Despite the need for caching, this function must be threadsafe.
+  ///
+  /// When parsing/compiling, the DiagnosticCallback is used to report errors.
+  virtual std::vector<CompiledFragment>
+  getFragments(const Params &, DiagnosticCallback) const = 0;
+};
+
 } // namespace config
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/unittests/CMakeLists.txt

@@ -42,6 +42,7 @@ add_unittest(ClangdUnitTests ClangdTests
   CompileCommandsTests.cpp
   CompilerTests.cpp
   ConfigCompileTests.cpp
+  ConfigProviderTests.cpp
   ConfigYAMLTests.cpp
   DexTests.cpp
   DiagnosticsTests.cpp

clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp

@@ -0,0 +1,156 @@
+//===-- ConfigProviderTests.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 "Config.h"
+#include "ConfigProvider.h"
+#include "ConfigTesting.h"
+#include "TestFS.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include "llvm/Support/SourceMgr.h"
+#include <atomic>
+
+namespace clang {
+namespace clangd {
+namespace config {
+namespace {
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+
+// Provider that appends an arg to compile flags.
+// The arg is prefix<N>, where N is the times getFragments() was called.
+// It also yields a diagnostic each time it's called.
+class FakeProvider : public Provider {
+  std::string Prefix;
+  mutable std::atomic<unsigned> Index = {0};
+
+  std::vector<CompiledFragment>
+  getFragments(const Params &, DiagnosticCallback DC) const override {
+    DC(llvm::SMDiagnostic("", llvm::SourceMgr::DK_Error, Prefix));
+    CompiledFragment F =
+        [Arg(Prefix + std::to_string(++Index))](const Params &P, Config &C) {
+          C.CompileFlags.Edits.push_back(
+              [Arg](std::vector<std::string> &Argv) { Argv.push_back(Arg); });
+          return true;
+        };
+    return {F};
+  }
+
+public:
+  FakeProvider(llvm::StringRef Prefix) : Prefix(Prefix) {}
+};
+
+std::vector<std::string> getAddedArgs(Config &C) {
+  std::vector<std::string> Argv;
+  for (auto &Edit : C.CompileFlags.Edits)
+    Edit(Argv);
+  return Argv;
+}
+
+// The provider from combine() should invoke its providers in order, and not
+// cache their results.
+TEST(ProviderTest, Combine) {
+  CapturedDiags Diags;
+  std::vector<std::unique_ptr<Provider>> Providers;
+  Providers.push_back(std::make_unique<FakeProvider>("foo"));
+  Providers.push_back(std::make_unique<FakeProvider>("bar"));
+  auto Combined = Provider::combine(std::move(Providers));
+  Config Cfg = Combined->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics,
+              ElementsAre(DiagMessage("foo"), DiagMessage("bar")));
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo1", "bar1"));
+  Diags.Diagnostics.clear();
+
+  Cfg = Combined->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics,
+              ElementsAre(DiagMessage("foo"), DiagMessage("bar")));
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo2", "bar2"));
+}
+
+const char *AddFooWithErr = R"yaml(
+CompileFlags:
+  Add: foo
+  Unknown: 42
+)yaml";
+
+const char *AddBarBaz = R"yaml(
+CompileFlags:
+  Add: bar
+---
+CompileFlags:
+  Add: baz
+)yaml";
+
+TEST(ProviderTest, FromYAMLFile) {
+  MockFS FS;
+  FS.Files["foo.yaml"] = AddFooWithErr;
+
+  CapturedDiags Diags;
+  auto P = Provider::fromYAMLFile(testPath("foo.yaml"), FS);
+  auto Cfg = P->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics,
+              ElementsAre(DiagMessage("Unknown CompileFlags key Unknown")));
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo"));
+  Diags.Diagnostics.clear();
+
+  Cfg = P->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "Cached, not re-parsed";
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo"));
+
+  FS.Files["foo.yaml"] = AddBarBaz;
+  Cfg = P->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "New config, no errors";
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("bar", "baz"));
+
+  FS.Files.erase("foo.yaml");
+  Cfg = P->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "Missing file is not an error";
+  EXPECT_THAT(getAddedArgs(Cfg), IsEmpty());
+}
+
+TEST(ProviderTest, FromAncestorRelativeYAMLFiles) {
+  MockFS FS;
+  FS.Files["a/b/c/foo.yaml"] = AddBarBaz;
+  FS.Files["a/foo.yaml"] = AddFooWithErr;
+
+  std::string ABCPath =
+      testPath("a/b/c/d/test.cc", llvm::sys::path::Style::posix);
+  Params ABCParams;
+  ABCParams.Path = ABCPath;
+  std::string APath =
+      testPath("a/b/e/f/test.cc", llvm::sys::path::Style::posix);
+  Params AParams;
+  AParams.Path = APath;
+
+  CapturedDiags Diags;
+  auto P = Provider::fromAncestorRelativeYAMLFiles("foo.yaml", FS);
+
+  auto Cfg = P->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+  EXPECT_THAT(getAddedArgs(Cfg), IsEmpty());
+
+  Cfg = P->getConfig(ABCParams, Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics,
+              ElementsAre(DiagMessage("Unknown CompileFlags key Unknown")));
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo", "bar", "baz"));
+  Diags.Diagnostics.clear();
+
+  Cfg = P->getConfig(AParams, Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "Cached config";
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo"));
+
+  FS.Files.erase("a/foo.yaml");
+  Cfg = P->getConfig(ABCParams, Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("bar", "baz"));
+}
+
+} // namespace
+} // namespace config
+} // namespace clangd
+} // namespace clang

clang-tools-extra/clangd/unittests/TestFS.cpp

@@ -79,13 +79,13 @@ const char *testRoot() {
 #endif
 }
 
-std::string testPath(PathRef File) {
+std::string testPath(PathRef File, llvm::sys::path::Style Style) {
   assert(llvm::sys::path::is_relative(File) && "FileName should be relative");
 
   llvm::SmallString<32> NativeFile = File;
-  llvm::sys::path::native(NativeFile);
+  llvm::sys::path::native(NativeFile, Style);
   llvm::SmallString<32> Path;
-  llvm::sys::path::append(Path, testRoot(), NativeFile);
+  llvm::sys::path::append(Path, Style, testRoot(), NativeFile);
   return std::string(Path.str());
 }
 

clang-tools-extra/clangd/unittests/TestFS.h

@@ -69,7 +69,8 @@ private:
 const char *testRoot();
 
 // Returns a suitable absolute path for this OS.
-std::string testPath(PathRef File);
+std::string testPath(PathRef File,
+                     llvm::sys::path::Style = llvm::sys::path::Style::native);
 
 // unittest: is a scheme that refers to files relative to testRoot()
 // This anchor is used to force the linker to link in the generated object file