simark created this revision.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, ioeric, 
ilya-biryukov.

This patch removes the possibility to change the compilation database
path at runtime using the didChangeConfiguration request.  Instead, it
is suggested to use the setting on the initialize request, and clangd
whenever the user wants to use a different build configuration.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53220

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  test/clangd/compile-commands-path-in-initialize.test
  test/clangd/compile-commands-path.test

Index: test/clangd/compile-commands-path.test
===================================================================
--- test/clangd/compile-commands-path.test
+++ /dev/null
@@ -1,42 +0,0 @@
-# Test that we can switch between configuration/build using the
-# workspace/didChangeConfiguration notification.
-
-# RUN: rm -rf %t.dir/* && mkdir -p %t.dir
-# RUN: mkdir %t.dir/build-1
-# RUN: mkdir %t.dir/build-2
-# RUN: echo '[{"directory": "%/t.dir", "command": "c++ the-file.cpp", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
-# RUN: echo '[{"directory": "%/t.dir/build-1", "command": "c++ -DMACRO=1 the-file.cpp", "file": "../the-file.cpp"}]' > %t.dir/build-1/compile_commands.json
-# RUN: echo '[{"directory": "%/t.dir/build-2", "command": "c++ -DMACRO=2 the-file.cpp", "file": "../the-file.cpp"}]' > %t.dir/build-2/compile_commands.json
-
-# RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
-
-# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
-# (with the extra slash in the front), so we add it here.
-# RUN: sed -e "s|file://\([A-Z]\):/|file:///\1:/|g" %t.test.1 > %t.test
-
-# RUN: clangd -lit-test < %t.test | FileCheck -strict-whitespace %t.test
-
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
----
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file://INPUT_DIR/the-file.cpp","languageId":"cpp","version":1,"text":"#if !defined(MACRO)\n#pragma message (\"MACRO is not defined\")\n#elif MACRO == 1\n#pragma message (\"MACRO is one\")\n#elif MACRO == 2\n#pragma message (\"MACRO is two\")\n#else\n#pragma message (\"woops\")\n#endif\nint main() {}\n"}}}
-# CHECK:   "method": "textDocument/publishDiagnostics",
-# CHECK-NEXT:   "params": {
-# CHECK-NEXT:     "diagnostics": [
-# CHECK-NEXT:       {
-# CHECK-NEXT:         "message": "MACRO is not defined",
----
-{"jsonrpc":"2.0","id":0,"method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":"INPUT_DIR/build-1"}}}
-# CHECK:   "method": "textDocument/publishDiagnostics",
-# CHECK-NEXT:   "params": {
-# CHECK-NEXT:     "diagnostics": [
-# CHECK-NEXT:       {
-# CHECK-NEXT:         "message": "MACRO is one",
----
-{"jsonrpc":"2.0","id":0,"method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":"INPUT_DIR/build-2"}}}
-# CHECK:   "method": "textDocument/publishDiagnostics",
-# CHECK-NEXT:   "params": {
-# CHECK-NEXT:     "diagnostics": [
-# CHECK-NEXT:       {
-# CHECK-NEXT:         "message": "MACRO is two",
----
-{"jsonrpc":"2.0","id":10000,"method":"shutdown"}
Index: test/clangd/compile-commands-path-in-initialize.test
===================================================================
--- test/clangd/compile-commands-path-in-initialize.test
+++ test/clangd/compile-commands-path-in-initialize.test
@@ -3,10 +3,8 @@
 
 # RUN: rm -rf %t.dir/* && mkdir -p %t.dir
 # RUN: mkdir %t.dir/build-1
-# RUN: mkdir %t.dir/build-2
 # RUN: echo '[{"directory": "%/t.dir", "command": "c++ the-file.cpp", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 # RUN: echo '[{"directory": "%/t.dir/build-1", "command": "c++ -DMACRO=1 the-file.cpp", "file": "../the-file.cpp"}]' > %t.dir/build-1/compile_commands.json
-# RUN: echo '[{"directory": "%/t.dir/build-2", "command": "c++ -DMACRO=2 the-file.cpp", "file": "../the-file.cpp"}]' > %t.dir/build-2/compile_commands.json
 
 # RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
 
@@ -18,18 +16,11 @@
 
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"initializationOptions":{"compilationDatabasePath":"INPUT_DIR/build-1"}}}
 ---
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file://INPUT_DIR/the-file.cpp","languageId":"cpp","version":1,"text":"#if !defined(MACRO)\n#pragma message (\"MACRO is not defined\")\n#elif MACRO == 1\n#pragma message (\"MACRO is one\")\n#elif MACRO == 2\n#pragma message (\"MACRO is two\")\n#else\n#pragma message (\"woops\")\n#endif\nint main() {}\n"}}}
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file://INPUT_DIR/the-file.cpp","languageId":"cpp","version":1,"text":"#if !defined(MACRO)\n#pragma message (\"MACRO is not defined\")\n#elif MACRO == 1\n#pragma message (\"MACRO is one\")\n#else\n#pragma message (\"woops\")\n#endif\nint main() {}\n"}}}
 # CHECK:   "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:   "params": {
 # CHECK-NEXT:     "diagnostics": [
 # CHECK-NEXT:       {
 # CHECK-NEXT:         "message": "MACRO is one",
 ---
-{"jsonrpc":"2.0","id":0,"method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":"INPUT_DIR/build-2"}}}
-# CHECK:   "method": "textDocument/publishDiagnostics",
-# CHECK-NEXT:   "params": {
-# CHECK-NEXT:     "diagnostics": [
-# CHECK-NEXT:       {
-# CHECK-NEXT:         "message": "MACRO is two",
----
 {"jsonrpc":"2.0","id":10000,"method":"shutdown"}
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -411,16 +411,17 @@
 /// "initialize" request and for the "workspace/didChangeConfiguration"
 /// notification since the data received is described as 'any' type in LSP.
 struct ClangdConfigurationParamsChange {
-  llvm::Optional<std::string> compilationDatabasePath;
-
   // The changes that happened to the compilation database.
   // The key of the map is a file name.
   llvm::Optional<std::map<std::string, ClangdCompileCommand>>
       compilationDatabaseChanges;
 };
 bool fromJSON(const llvm::json::Value &, ClangdConfigurationParamsChange &);
 
-struct ClangdInitializationOptions : public ClangdConfigurationParamsChange {};
+struct ClangdInitializationOptions : public ClangdConfigurationParamsChange {
+  llvm::Optional<std::string> compilationDatabasePath;
+};
+bool fromJSON(const llvm::json::Value &, ClangdInitializationOptions &);
 
 struct InitializeParams {
   /// The process Id of the parent process that started
Index: clangd/Protocol.cpp
===================================================================
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -665,10 +665,19 @@
 bool fromJSON(const json::Value &Params,
               ClangdConfigurationParamsChange &CCPC) {
   json::ObjectMapper O(Params);
-  return O && O.map("compilationDatabasePath", CCPC.compilationDatabasePath) &&
+  return O &&
          O.map("compilationDatabaseChanges", CCPC.compilationDatabaseChanges);
 }
 
+bool fromJSON(const json::Value &Params, ClangdInitializationOptions &Opts) {
+  if (!fromJSON(Params, static_cast<ClangdConfigurationParamsChange &>(Opts))) {
+    return false;
+  }
+
+  json::ObjectMapper O(Params);
+  return O && O.map("compilationDatabasePath", Opts.compilationDatabasePath);
+}
+
 bool fromJSON(const json::Value &Params, ReferenceParams &R) {
   TextDocumentPositionParams &Base = R;
   return fromJSON(Params, Base);
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -87,6 +87,7 @@
   /// may be very expensive.  This method is normally called when the
   /// compilation database is changed.
   void reparseOpenedFiles();
+  void applyConfiguration(const ClangdInitializationOptions &Settings);
   void applyConfiguration(const ClangdConfigurationParamsChange &Settings);
 
   JSONOutput &Out;
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -425,17 +425,23 @@
 }
 
 void ClangdLSPServer::applyConfiguration(
-    const ClangdConfigurationParamsChange &Settings) {
-  // Compilation database change.
-  if (Settings.compilationDatabasePath.hasValue()) {
-    CDB.setCompileCommandsDir(Settings.compilationDatabasePath.getValue());
+    const ClangdInitializationOptions &Opts) {
+  // Explicit compilation database path.
+  if (Opts.compilationDatabasePath.hasValue()) {
+    CDB.setCompileCommandsDir(Opts.compilationDatabasePath.getValue());
 
     reparseOpenedFiles();
   }
 
-  // Update to the compilation database.
-  if (Settings.compilationDatabaseChanges) {
-    const auto &CompileCommandUpdates = *Settings.compilationDatabaseChanges;
+  applyConfiguration(
+      static_cast<const ClangdConfigurationParamsChange &>(Opts));
+}
+
+void ClangdLSPServer::applyConfiguration(
+    const ClangdConfigurationParamsChange &Params) {
+  // Per-file update to the compilation database.
+  if (Params.compilationDatabaseChanges) {
+    const auto &CompileCommandUpdates = *Params.compilationDatabaseChanges;
     bool ShouldReparseOpenFiles = false;
     for (auto &Entry : CompileCommandUpdates) {
       /// The opened files need to be reparsed only when some existing
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to