[clang-tools-extra] r344614 - Remove possibility to change compile database path at runtime

Simon Marchi via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 16 08:55:03 PDT 2018


Author: simark
Date: Tue Oct 16 08:55:03 2018
New Revision: 344614

URL: http://llvm.org/viewvc/llvm-project?rev=344614&view=rev
Log:
Remove possibility to change compile database path at runtime

Summary:
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.

Subscribers: ilya-biryukov, ioeric, jkorous, arphaman, kadircet, cfe-commits

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

Removed:
    clang-tools-extra/trunk/test/clangd/compile-commands-path.test
Modified:
    clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
    clang-tools-extra/trunk/clangd/Protocol.cpp
    clang-tools-extra/trunk/clangd/Protocol.h
    clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=344614&r1=344613&r2=344614&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue Oct 16 08:55:03 2018
@@ -81,8 +81,16 @@ CompletionItemKindBitset defaultCompleti
 } // namespace
 
 void ClangdLSPServer::onInitialize(InitializeParams &Params) {
-  if (Params.initializationOptions)
-    applyConfiguration(*Params.initializationOptions);
+  if (Params.initializationOptions) {
+    const ClangdInitializationOptions &Opts = *Params.initializationOptions;
+
+    // Explicit compilation database path.
+    if (Opts.compilationDatabasePath.hasValue()) {
+      CDB.setCompileCommandsDir(Opts.compilationDatabasePath.getValue());
+    }
+
+    applyConfiguration(Opts.ParamsChange);
+  }
 
   if (Params.rootUri && *Params.rootUri)
     Server->setRootPath(Params.rootUri->file());
@@ -425,17 +433,10 @@ void ClangdLSPServer::onHover(TextDocume
 }
 
 void ClangdLSPServer::applyConfiguration(
-    const ClangdConfigurationParamsChange &Settings) {
-  // Compilation database change.
-  if (Settings.compilationDatabasePath.hasValue()) {
-    CDB.setCompileCommandsDir(Settings.compilationDatabasePath.getValue());
-
-    reparseOpenedFiles();
-  }
-
-  // Update to the compilation database.
-  if (Settings.compilationDatabaseChanges) {
-    const auto &CompileCommandUpdates = *Settings.compilationDatabaseChanges;
+    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

Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=344614&r1=344613&r2=344614&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Tue Oct 16 08:55:03 2018
@@ -665,10 +665,19 @@ bool fromJSON(const llvm::json::Value &P
 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, Opts.ParamsChange)) {
+    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);

Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=344614&r1=344613&r2=344614&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Tue Oct 16 08:55:03 2018
@@ -411,8 +411,6 @@ bool fromJSON(const llvm::json::Value &,
 /// "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>>
@@ -420,7 +418,14 @@ struct ClangdConfigurationParamsChange {
 };
 bool fromJSON(const llvm::json::Value &, ClangdConfigurationParamsChange &);
 
-struct ClangdInitializationOptions : public ClangdConfigurationParamsChange {};
+struct ClangdInitializationOptions {
+  // What we can change throught the didChangeConfiguration request, we can
+  // also set through the initialize request (initializationOptions field).
+  ClangdConfigurationParamsChange ParamsChange;
+
+  llvm::Optional<std::string> compilationDatabasePath;
+};
+bool fromJSON(const llvm::json::Value &, ClangdInitializationOptions &);
 
 struct InitializeParams {
   /// The process Id of the parent process that started

Modified: clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test?rev=344614&r1=344613&r2=344614&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test (original)
+++ clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test Tue Oct 16 08:55:03 2018
@@ -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"}

Removed: clang-tools-extra/trunk/test/clangd/compile-commands-path.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/compile-commands-path.test?rev=344613&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clangd/compile-commands-path.test (original)
+++ clang-tools-extra/trunk/test/clangd/compile-commands-path.test (removed)
@@ -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"}




More information about the cfe-commits mailing list