[clangd] Support config over LSP.

[sammccall]

Summary

We support sending fragments in initialize and workspace/didChangeConfiguration.
Sadly no workspace/configuration yet, we'd want to send that request on
didOpen and we'd have to block on getting the answer, which is much more
invasive.

[kadircet]

clang-tools-extra/clangd/ClangdLSPServer.cpp

1220

nit: maybe just set ReparseAllFiles in here too, and change the condition below to only return ReparseAllFiles

clang-tools-extra/clangd/Protocol.h

494

it feels like clients will only be using two types of keys, a "global" one for sending over (likely project-specific) user preferences and another for directory/file specific configs. The latter is likely to have the path as a key, whereas the former will likely be a special identifier. I am glad that ! comes before both / and any capital letters(letter drives).

I think it is only important to have a distinction between these two types (a subset of what you describe here). Current design makes it really easy on our side, I am just afraid of confusing clients. Maybe we should just have an enum/boolean tag saying if the fragment is global or not instead of relying on the key orderings ?

[sammccall]

clang-tools-extra/clangd/ClangdLSPServer.cpp

1220

Is the idea here that triggering spurious reparses is cheap enough?
Or that if it's cheap enough for our "future-facing" extension, we should be able to stop optimizing it for the existing one?

clang-tools-extra/clangd/Protocol.h

494

If we go down this path, where do we draw the line?
(this isn't a slipper-slope argument - I think it's a reasonable idea and we should work out how far we want to take it)

If we're encoding this file/directory use case in the protocol somehow, why wouldn't we *require* the key to be a path, and no longer require a condition to be set for that? This is like how .clangd files are handled.

Then we'd have a more natural way of dealing with priority between file/dir-specific entries (actually, maybe it coincides with sort order, ha!).

But this model starts to look something much closer to the workspace/configuration server->client request, which has a scope URI attached. The recommendation from LSP folks is that actual configuration flow over that endpoint, and didChangeConfiguration just be used to notify of changes.

That hasn't seemed feasible for us as we imagine all the requests issues by background index, and the bother of making everything consuming config asynchronous. But:

• if we restricted per-URI config to per-directory, not per-file, the number of requests aren't overwhelming (assuming caching). It's still possible to achieve per-file config with conditions...
• async seems fundamentally solvable, given that we only create config on TUScheduler threads and background-index threads now, never the main thread (unless in sync mode...). Technical details to work out of course (if the server doesn't get a response it'll want to time out, but what thread does the timeout handler run on?)

It would help there were some clearer practical goal we were driving towards. Propagating, say, vscode extension settings live into clangd can be done this way, but it's a sledgehammer for a peanut. Maybe live-configuring *workspace* properties is an interesting thing, but I'm not sure how much value there is when .clangd exists.

[kadircet]

clang-tools-extra/clangd/ClangdLSPServer.cpp

1220

none :( sorry i misread this and thought the code below was doing ModifiedFiles.size() != 0 (that's way the comment was a nit)

OTOH, as you noted spurious reparses are cheap(still some IO and a task in the queue). We expect the usage of updating compile commands to migrate towards configs in the future, so in theory we won't be using that optimization most of the time.

Even though we've got enough info to prevent spurious reparses here, we might not in the future (e.g. reparses resulting from a config change). So I would drop the optimization(the whole Old-New comparison logic) to increase readability.

but definitely not something we should do now (probably ever), so feel free to ignore.

[sammccall]

There are lots of choices and overlapping ideas for how the protocol should look, little urgency (it isn't going to make clangd 11), and few concrete use cases to evaluate the options.

Let's table this for now and let the ideas sink in and more use cases crop up. I definitely think we should have this, probably for clangd 12, but it's worth getting right.

clang-tools-extra/clangd/ClangdLSPServer.h

 //===--- ClangdLSPServer.h - LSP server --------------------------*- C++-*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDLSPSERVER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDLSPSERVER_H
 
 #include "ClangdServer.h"
+#include "ConfigProvider.h"
 #include "DraftStore.h"
 #include "Features.inc"
 #include "FindSymbols.h"
 #include "GlobalCompilationDatabase.h"
 #include "Protocol.h"
 #include "Transport.h"
 #include "support/Context.h"
 #include "support/Path.h"
private:
   void notify(StringRef Method, llvm::json::Value Params);
   template <typename T> void progress(const llvm::json::Value &Token, T Value) {
     ProgressParams<T> Params;
     Params.token = Token;
     Params.value = std::move(Value);
     notify("$/progress", Params);
   }
 
+  /// Stores clangd config set over LSP.
+  class LSPConfigProvider : public config::Provider {
+    mutable std::mutex Mu;
+    mutable std::map<std::string, config::CompiledFragment> Cached;
+    llvm::StringMap<llvm::json::Value> Pending;
+
+    std::vector<config::CompiledFragment>
+    getFragments(const config::Params &,
+                 config::DiagnosticCallback) const override;
+
+  public:
+    void update(const std::map<std::string, llvm::json::Value> &);
+  };
+  LSPConfigProvider LSPConfig;
+  std::unique_ptr<config::Provider> ConfigProviderStorage;
+
   const ThreadsafeFS &TFS;
   /// Options used for code completion
   clangd::CodeCompleteOptions CCOpts;
   /// Options used for rename.
   clangd::RenameOptions RenameOpts;
   /// Options used for diagnostics.
   ClangdDiagnosticOptions DiagOpts;
   /// The supported kinds of the client.

clang-tools-extra/clangd/ClangdLSPServer.cpp

 //===--- ClangdLSPServer.cpp - LSP server ------------------------*- C++-*-===//
 //
 // 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 "ClangdLSPServer.h"
 #include "CodeComplete.h"
+#include "ConfigFragment.h"
 #include "Diagnostics.h"
 #include "DraftStore.h"
 #include "GlobalCompilationDatabase.h"
 #include "Protocol.h"
 #include "SemanticHighlighting.h"
 #include "SourceCode.h"
 #include "TUScheduler.h"
 #include "URI.h"
void ClangdLSPServer::onResolveTypeHierarchy(
     const ResolveTypeHierarchyItemParams &Params,
     Callback<Optional<TypeHierarchyItem>> Reply) {
   Server->resolveTypeHierarchy(Params.item, Params.resolve, Params.direction,
                                std::move(Reply));
 }
 
 void ClangdLSPServer::applyConfiguration(
     const ConfigurationSettings &Settings) {
+  bool ReparseAllFiles = false;
+  if (!Settings.fragments.empty()) {
+    LSPConfig.update(Settings.fragments);
+    ReparseAllFiles = true; // Config may affect any open file.
+  }
+
   // Per-file update to the compilation database.
   llvm::StringSet<> ModifiedFiles;
   for (auto &Entry : Settings.compilationDatabaseChanges) {
     PathRef File = Entry.first;
     auto Old = CDB->getCompileCommand(File);
     auto New =
         tooling::CompileCommand(std::move(Entry.second.workingDirectory), File,
                                 std::move(Entry.second.compilationCommand),
                                 /*Output=*/"");
     if (Old != New) {
       CDB->setCompileCommand(File, std::move(New));
       ModifiedFiles.insert(File);
     }
   }
 
-  reparseOpenFilesIfNeeded(
-      [&](llvm::StringRef File) { return ModifiedFiles.count(File) != 0; });
+  reparseOpenFilesIfNeeded([&](llvm::StringRef File) {
+    return ReparseAllFiles || ModifiedFiles.count(File) != 0;
+  });
 }
 
 void ClangdLSPServer::publishTheiaSemanticHighlighting(
     const TheiaSemanticHighlightingParams &Params) {
   notify("textDocument/semanticHighlighting", Params);
 }
 
 void ClangdLSPServer::publishDiagnostics(
     const PublishDiagnosticsParams &Params) {
   notify("textDocument/publishDiagnostics", Params);
 }
 
 // FIXME: This function needs to be properly tested.
 void ClangdLSPServer::onChangeConfiguration(
     const DidChangeConfigurationParams &Params) {
   applyConfiguration(Params.settings);
 }
 
+std::vector<config::CompiledFragment>
+ClangdLSPServer::LSPConfigProvider::getFragments(
+    const config::Params &P, config::DiagnosticCallback DC) const {
+  std::lock_guard<std::mutex> Lock(Mu);
+  // Compile any pending configs, and merge them into the cache.
+  for (const auto &Entry : Pending) {
+    llvm::Optional<config::Fragment> Fragment;
+    if (!Entry.second.getAsNull())
+      Fragment = config::Fragment::parseJSON(
+          Entry.second, ("LSP:" + Entry.first()).str(), DC);
+    if (Fragment)
+      Cached[Entry.first().str()] = std::move(*Fragment).compile(DC);
+    else
+      Cached.erase(Entry.first().str());
+  }
+  // Now extract all fragments from the cache.
+  std::vector<config::CompiledFragment> Results;
+  for (const auto &Entry : Cached) // Sorted by key, as desired.
+    Results.push_back(Entry.second);
+  return Results;
+}
+
+void ClangdLSPServer::LSPConfigProvider::update(
+    const std::map<std::string, llvm::json::Value> &M) {
+  std::lock_guard<std::mutex> Lock(Mu);
+  for (const auto &Entry : M)
+    Pending.insert_or_assign(Entry.first, Entry.second);
+}
+
 void ClangdLSPServer::onReference(const ReferenceParams &Params,
                                   Callback<std::vector<Location>> Reply) {
   Server->findReferences(Params.textDocument.uri.file(), Params.position,
                          CCOpts.Limit,
                          [Reply = std::move(Reply)](
                              llvm::Expected<ReferencesResult> Refs) mutable {
                            if (!Refs)
                              return Reply(Refs.takeError());
ClangdLSPServer::ClangdLSPServer(
   MsgHandler->bind("typeHierarchy/resolve", &ClangdLSPServer::onResolveTypeHierarchy);
   MsgHandler->bind("textDocument/selectionRange", &ClangdLSPServer::onSelectionRange);
   MsgHandler->bind("textDocument/documentLink", &ClangdLSPServer::onDocumentLink);
   MsgHandler->bind("textDocument/semanticTokens/full", &ClangdLSPServer::onSemanticTokens);
   MsgHandler->bind("textDocument/semanticTokens/full/delta", &ClangdLSPServer::onSemanticTokensDelta);
   if (Opts.FoldingRanges)
     MsgHandler->bind("textDocument/foldingRange", &ClangdLSPServer::onFoldingRange);
   // clang-format on
+  if (Opts.ConfigProvider) {
+    ConfigProviderStorage =
+        config::Provider::combine({&LSPConfig, Opts.ConfigProvider});
+    ClangdServerOpts.ConfigProvider = ConfigProviderStorage.get();
+  } else {
+    ClangdServerOpts.ConfigProvider = &LSPConfig;
+  }
 }
 
 ClangdLSPServer::~ClangdLSPServer() {
   IsBeingDestroyed = true;
   // Explicitly destroy ClangdServer first, blocking on threads it owns.
   // This ensures they don't access any other members.
   Server.reset();
 }

clang-tools-extra/clangd/ConfigFragment.h

 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGFRAGMENT_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGFRAGMENT_H
 
 #include "ConfigProvider.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SourceMgr.h"
 #include <string>
 #include <vector>
 
 namespace clang {
 namespace clangd {
 namespace config {

 struct Fragment {
   /// Parses fragments from a YAML file (one from each --- delimited document).
   /// Documents that contained fatal errors are omitted from the results.
   /// BufferName is used for the SourceMgr and diagnostics.
   static std::vector<Fragment> parseYAML(llvm::StringRef YAML,
                                          llvm::StringRef BufferName,
                                          DiagnosticCallback);
 
+  /// Parses a fragment from a JSON value (an Object).
+  /// Returns None if the value contained a fatal error and could not be parsed.
+  /// Locations are not reported with the diagnostics.
+  static llvm::Optional<Fragment> parseJSON(const llvm::json::Value &JSON,
+                                            llvm::StringRef SourceName,
+                                            DiagnosticCallback);
+
   /// Analyzes and consumes this fragment, possibly yielding more diagnostics.
   /// This always produces a usable result (errors are recovered).
   ///
   /// Typically, providers will compile a Fragment once when it's first loaded,
   /// caching the result for reuse.
   /// Like a compiled program, this is good for performance and also encourages
   /// errors to be reported early and only once.
   ///

clang-tools-extra/clangd/ConfigYAML.cpp

 //===--- ConfigYAML.cpp - Loading configuration fragments from YAML files -===//
 //
 // 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 "ConfigFragment.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/YAMLParser.h"
 #include <system_error>
 
 namespace clang {
 namespace clangd {
 namespace config {
 namespace {
for (auto &Doc : llvm::yaml::Stream(*Buf, *SM)) {
     }
   }
   // Hack: stash the buffer in the SourceMgr to keep it alive.
   // SM has two entries: "main" non-owning buffer, and ignored owning buffer.
   SM->AddNewSourceBuffer(std::move(Buf), llvm::SMLoc());
   return Result;
 }
 
+llvm::Optional<Fragment> Fragment::parseJSON(const llvm::json::Value &Value,
+                                             llvm::StringRef Source,
+                                             DiagnosticCallback Diags) {
+  // This is a horrible hack: JSON is a subset of YAML, so use the YAML parser.
+  // FIXME: The diagnostics produced will use YAML terms etc. Do this properly.
+  auto NoLocDiags = [&](const llvm::SMDiagnostic &D) {
+    Diags(llvm::SMDiagnostic(Source, D.getKind(), D.getMessage()));
+  };
+  auto Result = parseYAML(llvm::to_string(Value), "", NoLocDiags);
+
+  assert(Result.size() <= 1 && "JSON yielded multiple YAML documents?");
+  if (Result.empty())
+    return llvm::None;
+  return std::move(Result.front());
+}
+
 } // namespace config
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/Protocol.h

 
 /// Clangd extension: parameters configurable at any time, via the
 /// `workspace/didChangeConfiguration` notification.
 /// LSP defines this type as `any`.
 struct ConfigurationSettings {
   // Changes to the in-memory compilation database.
   // The key of the map is a file name.
   std::map<std::string, ClangdCompileCommand> compilationDatabaseChanges;
+
+  // clangd configuration fragments: see https://clangd.llvm.org/config.html.
+  // Each value is an Object or null, and replaces any fragment with that key.
+  // Fragments are used in key order ("!foo" is low-priority, "~foo" is high).
+  std::map<std::string, llvm::json::Value> fragments;
 };
 bool fromJSON(const llvm::json::Value &, ConfigurationSettings &);
 
 /// Clangd extension: parameters configurable at `initialize` time.
 /// LSP defines this type as `any`.
 struct InitializationOptions {
   // What we can change throught the didChangeConfiguration request, we can
   // also set through the initialize request (initializationOptions field).
struct InitializeParams {
   /// @deprecated in favour of rootUri.
   llvm::Optional<std::string> rootPath;
 
   /// The rootUri of the workspace. Is null if no
   /// folder is open. If both `rootPath` and `rootUri` are set
   /// `rootUri` wins.
   llvm::Optional<URIForFile> rootUri;
 
-  // User provided initialization options.
-  // initializationOptions?: any;
-
   /// The capabilities provided by the client (editor or tool)
   ClientCapabilities capabilities;
 
   /// The initial trace setting. If omitted trace is disabled ('off').
   llvm::Optional<TraceLevel> trace;
 
   /// User-provided initialization options.
+  /// LSP defines this type as `any`.
   InitializationOptions initializationOptions;
 };
 bool fromJSON(const llvm::json::Value &, InitializeParams &);
 
 struct WorkDoneProgressCreateParams {
   /// The token to be used to report progress.
   llvm::json::Value token = nullptr;
 };

clang-tools-extra/clangd/Protocol.cpp

return O && O.map("workingDirectory", CDbUpdate.workingDirectory) &&
          O.map("compilationCommand", CDbUpdate.compilationCommand);
 }
 
 bool fromJSON(const llvm::json::Value &Params, ConfigurationSettings &S) {
   llvm::json::ObjectMapper O(Params);
   if (!O)
     return true; // 'any' type in LSP.
   O.map("compilationDatabaseChanges", S.compilationDatabaseChanges);
+  if (auto *Clangd = Params.getAsObject()->getObject("fragments"))
+    for (const auto &Entry : *Clangd)
+      S.fragments.emplace(Entry.first.str(), llvm::json::Value(Entry.second));
   return true;
 }
 
 bool fromJSON(const llvm::json::Value &Params, InitializationOptions &Opts) {
   llvm::json::ObjectMapper O(Params);
   if (!O)
     return true; // 'any' type in LSP.
 

clang-tools-extra/clangd/test/config.test

+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+# Set some config fragments on startup.
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","initializationOptions": {
+  "fragments": {
+    "!global": {
+      "CompileFlags": {"Add": "-DFOO=A"}
+    },
+    "bar": {
+      "If": {"PathMatch":".*/bar.c"},
+      "CompileFlags": {"Add": "-DFOO=B"}
+    }
+  }
+}}}
+---
+# foo.c gets the global configuration.
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c",
+  "text":"int main() { return FOO; }"
+}}}
+#      CHECK:  "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:    "diagnostics": [
+# CHECK-NEXT:      {
+# CHECK-NEXT:        "code": "undeclared_var_use",
+# CHECK-NEXT:        "message": "Use of undeclared identifier 'A'",
+#      CHECK:      }
+#      CHECK:    ],
+# CHECK-NEXT:    "uri": "file://{{.*}}/foo.c",
+# CHECK-NEXT:    "version": 0
+# CHECK-NEXT:  }
+---
+# bar.c (also) gets the conditional configuration.
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///bar.c","languageId":"c",
+  "text":"int main() { return FOO; }"
+}}}
+#      CHECK:  "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:    "diagnostics": [
+# CHECK-NEXT:      {
+# CHECK-NEXT:        "code": "undeclared_var_use",
+# CHECK-NEXT:        "message": "Use of undeclared identifier 'B'",
+#      CHECK:      }
+#      CHECK:    ],
+# CHECK-NEXT:    "uri": "file://{{.*}}/bar.c",
+# CHECK-NEXT:    "version": 0
+# CHECK-NEXT:  }
+---
+# Replace global configuration.
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{
+  "fragments": {
+    "!global": {
+      "CompileFlags": {"Add": "-DFOO=C"}
+    }
+  }
+}}}
+# Both files get rebuilt, and use the new config.
+#      CHECK: "method": "textDocument/publishDiagnostics",
+#  CHECK-DAG: "message": "Use of undeclared identifier 'C'",
+#  CHECK-DAG: "message": "Use of undeclared identifier 'B'",
+---
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
+
+
+

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

- 1
   )yaml";
   auto Results = Fragment::parseYAML(YAML, "config.yaml", Diags.callback());
   EXPECT_THAT(Diags.Diagnostics,
               ElementsAre(DiagMessage("If should be a dictionary"),
                           DiagMessage("Config should be a dictionary")));
   ASSERT_THAT(Results, IsEmpty());
 }
 
+TEST(ParseYAML, JSON) {
+  CapturedDiags Diags;
+  llvm::json::Object Config{
+      {"If", llvm::json::Object{{"UnknownCondition", "foo"}}},
+      {"CompileFlags", llvm::json::Object{{"Add", "first"}}},
+  };
+  auto Result =
+      Fragment::parseJSON(std::move(Config), "config.json", Diags.callback());
+
+  ASSERT_THAT(Diags.Diagnostics,
+              ElementsAre(AllOf(DiagMessage("Unknown If key UnknownCondition"),
+                                DiagKind(llvm::SourceMgr::DK_Warning))));
+  EXPECT_EQ(Diags.Diagnostics.front().Rng, llvm::None);
+
+  ASSERT_NE(Result, llvm::None);
+  EXPECT_TRUE(Result->If.HasUnrecognizedCondition);
+  EXPECT_THAT(Result->CompileFlags.Add, ElementsAre(Val("first")));
+}
+
 } // namespace
 } // namespace config
 } // namespace clangd
 } // namespace clang