Nebiroth updated this revision to Diff 124024.
Nebiroth marked 19 inline comments as done.
Nebiroth added a comment.

Fixed DraftStore thread-safe API being broken
Removed superfluous getCompilationDatabase call
Changed name of struct to ClangDConfigurationParamsChange
Removed operator ! overload for struct
Minor code cleanup


https://reviews.llvm.org/D39571

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/DraftStore.cpp
  clangd/DraftStore.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  test/clangd/did-change-configuration.test
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===================================================================
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -18,6 +18,7 @@
 # CHECK-NEXT:          ":"
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
+# CHECK-NEXT:      "configurationChangeProvider": true,
 # CHECK-NEXT:      "definitionProvider": true,
 # CHECK-NEXT:      "documentFormattingProvider": true,
 # CHECK-NEXT:      "documentOnTypeFormattingProvider": {
@@ -48,4 +49,4 @@
 # CHECK-NEXT:  "result": null
 Content-Length: 33
 
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0":"method":"exit"}
\ No newline at end of file
Index: test/clangd/initialize-params-invalid.test
===================================================================
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -1,3 +1,4 @@
+
 # RUN: clangd -pretty -run-synchronously < %s | FileCheck -strict-whitespace %s
 # It is absolutely vital that this file has CRLF line endings.
 #
@@ -18,6 +19,7 @@
 # CHECK-NEXT:          ":"
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
+# CHECK-NEXT:      "configurationChangeProvider": true,
 # CHECK-NEXT:      "definitionProvider": true,
 # CHECK-NEXT:      "documentFormattingProvider": true,
 # CHECK-NEXT:      "documentOnTypeFormattingProvider": {
@@ -45,4 +47,4 @@
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
 Content-Length: 33
 
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0":"method":"exit"}
\ No newline at end of file
Index: test/clangd/did-change-configuration.test
===================================================================
--- /dev/null
+++ test/clangd/did-change-configuration.test
@@ -0,0 +1,46 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 150
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"voidmain(){}"}}}
+
+Content-Length: 86
+
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":""}}
+#Failed to decode workspace/didChangeConfiguration request.
+#Incorrect mapping node
+
+Content-Length: 114
+
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":{}}}}
+#Failed to decode workspace/didChangeConfiguration request.
+#compilationDatabasePath is not a scalar node
+
+Content-Length: 140
+
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":"/","testBadValue":"foo1234"}}}
+#Ignored unknown field "testBadValue"
+#Failed to find compilation database for / in overriden directory /
+#Bad field, bad compilation database
+
+Content-Length: 722
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///compile_commands.json","languageId":"json","version":1,"text":"[\n{\n"directory":"/",\n"command":"/usr/bin/c++-DGTEST_HAS_RTTI=0-D_GNU_SOURCE-D__STDC_CONSTANT_MACROS-D__STDC_FORMAT_MACROS-D__STDC_LIMIT_MACROS-Ilib/Demangle-I../lib/Demangle-I/usr/include/libxml2-Iinclude-I../include-fPIC-fvisibility-inlines-hidden-Werror=date-time-std=c++11-Wall-W-Wno-unused-parameter-Wwrite-strings-Wcast-qual-Wno-missing-field-initializers-pedantic-Wno-long-long-Wno-maybe-uninitialized-Wdelete-non-virtual-dtor-Wno-comment-O0-g-fno-exceptions-fno-rtti-o/foo.c.o-c/foo.c",\n"file":"/foo.c"\n},"}}}
+
+Content-Length: 115
+
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":"/"}}}
+#CHECK:{"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"diagnostics":[{"message":"type specifier missing, defaults to 'int'","range":{"end":{"character":1,"line":0},"start":{"character":1,"line":0}},"severity":2},{"message":"control reaches end of non-void function","range":{"end":{"character":12,"line":0},"start":{"character":12,"line":0}},"severity":2}],"uri":"file:///foo.c"}}
+
+Content-Length: 48
+
+{"jsonrpc":"2.0","id":10000,"method":"shutdown"}
+
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}
\ No newline at end of file
Index: clangd/ProtocolHandlers.h
===================================================================
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -53,7 +53,9 @@
   virtual void onSwitchSourceHeader(Ctx C, TextDocumentIdentifier &Params) = 0;
   virtual void onFileEvent(Ctx C, DidChangeWatchedFilesParams &Params) = 0;
   virtual void onCommand(Ctx C, ExecuteCommandParams &Params) = 0;
-  virtual void onRename(Ctx C, RenameParams &Parames) = 0;
+  virtual void onRename(Ctx C, RenameParams &Params) = 0;
+  virtual void onChangeConfiguration(Ctx C,
+                                     DidChangeConfigurationParams &Params) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, JSONOutput &Out,
Index: clangd/ProtocolHandlers.cpp
===================================================================
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -74,4 +74,6 @@
   Register("textDocument/rename", &ProtocolCallbacks::onRename);
   Register("workspace/didChangeWatchedFiles", &ProtocolCallbacks::onFileEvent);
   Register("workspace/executeCommand", &ProtocolCallbacks::onCommand);
+  Register("workspace/didChangeConfiguration",
+           &ProtocolCallbacks::onChangeConfiguration);
 }
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -289,6 +289,30 @@
   parse(llvm::yaml::MappingNode *Params, clangd::Logger &Logger);
 };
 
+/// Clangd extension to manage a workspace/didChangeConfiguration notification
+/// since the data received is described as 'any' type in LSP.
+
+struct ClangdConfigurationParamsChange {
+
+  llvm::Optional<std::string> compilationDatabasePath;
+  static llvm::Optional<ClangdConfigurationParamsChange>
+  parse(llvm::yaml::MappingNode *Params, clangd::Logger &Logger);
+};
+
+struct DidChangeConfigurationParams {
+
+  DidChangeConfigurationParams() {}
+  DidChangeConfigurationParams(ClangdConfigurationParamsChange settings) {}
+
+  // We use this predefined struct because it is easier to use
+  // than the protocol specified type of 'any'.
+  ClangdConfigurationParamsChange settings;
+
+  static llvm::Optional<DidChangeConfigurationParams>
+  parse(llvm::yaml::MappingNode *Params, clangd::Logger &Logger);
+  static std::string unparse(const DidChangeConfigurationParams &P);
+};
+
 struct FormattingOptions {
   /// Size of a tab in spaces.
   int tabSize;
Index: clangd/Protocol.cpp
===================================================================
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -528,6 +528,63 @@
   return Result;
 }
 
+llvm::Optional<DidChangeConfigurationParams>
+DidChangeConfigurationParams::parse(llvm::yaml::MappingNode *Params,
+                                    clangd::Logger &Logger) {
+  DidChangeConfigurationParams Result;
+  for (auto &NextKeyValue : *Params) {
+    auto *KeyString = dyn_cast<llvm::yaml::ScalarNode>(NextKeyValue.getKey());
+    if (!KeyString)
+      return llvm::None;
+
+    llvm::SmallString<10> KeyStorage;
+    StringRef KeyValue = KeyString->getValue(KeyStorage);
+    auto *Value =
+        dyn_cast_or_null<llvm::yaml::MappingNode>(NextKeyValue.getValue());
+    if (!Value)
+      return llvm::None;
+
+    llvm::SmallString<10> Storage;
+    if (KeyValue == "settings") {
+      auto Parsed = ClangdConfigurationParamsChange::parse(Value, Logger);
+      if (!Parsed.hasValue())
+        return llvm::None;
+      Result.settings = Parsed.getValue();
+    } else {
+      logIgnoredField(KeyValue, Logger);
+    }
+  }
+  if (!Result.settings.compilationDatabasePath.hasValue())
+    return llvm::None;
+
+  return Result;
+}
+
+llvm::Optional<ClangdConfigurationParamsChange>
+ClangdConfigurationParamsChange::parse(llvm::yaml::MappingNode *Params,
+                                       clangd::Logger &Logger) {
+  ClangdConfigurationParamsChange Result;
+  for (auto &NextKeyValue : *Params) {
+    auto *KeyString = dyn_cast<llvm::yaml::ScalarNode>(NextKeyValue.getKey());
+    if (!KeyString)
+      return llvm::None;
+
+    llvm::SmallString<10> KeyStorage;
+    StringRef KeyValue = KeyString->getValue(KeyStorage);
+    auto *Value =
+        dyn_cast_or_null<llvm::yaml::ScalarNode>(NextKeyValue.getValue());
+    if (!Value)
+      return llvm::None;
+
+    if (KeyValue == "compilationDatabasePath") {
+      Result.compilationDatabasePath = Value->getValue(KeyStorage);
+    } else {
+      logIgnoredField(KeyValue, Logger);
+    }
+  }
+  return Result;
+}
+
 llvm::Optional<TextDocumentContentChangeEvent>
 TextDocumentContentChangeEvent::parse(llvm::yaml::MappingNode *Params,
                                       clangd::Logger &Logger) {
Index: clangd/GlobalCompilationDatabase.h
===================================================================
--- clangd/GlobalCompilationDatabase.h
+++ clangd/GlobalCompilationDatabase.h
@@ -52,7 +52,10 @@
 
   std::vector<tooling::CompileCommand>
   getCompileCommands(PathRef File) override;
-
+  void setCompileCommandsDir(Path P) {
+    CompileCommandsDir = P;
+    CompilationDatabases.clear();
+  }
   void setExtraFlagsForFile(PathRef File, std::vector<std::string> ExtraFlags);
 
 private:
Index: clangd/GlobalCompilationDatabase.cpp
===================================================================
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -10,6 +10,7 @@
 #include "GlobalCompilationDatabase.h"
 #include "Logger.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 
@@ -100,7 +101,7 @@
         tryLoadDatabaseFromPath(CompileCommandsDir.getValue());
     if (ReturnValue == nullptr)
       Logger.log("Failed to find compilation database for " + Twine(File) +
-                 "in overriden directory " + CompileCommandsDir.getValue() +
+                 " in overriden directory " + CompileCommandsDir.getValue() +
                  "\n");
     return ReturnValue;
   }
Index: clangd/DraftStore.h
===================================================================
--- clangd/DraftStore.h
+++ clangd/DraftStore.h
@@ -40,6 +40,9 @@
   /// \return version and contents of the stored document.
   /// For untracked files, a (0, None) pair is returned.
   VersionedDraft getDraft(PathRef File) const;
+
+  std::vector<Path> getActiveFiles() const;
+
   /// \return version of the tracked document.
   /// For untracked files, 0 is returned.
   DocVersion getVersion(PathRef File) const;
Index: clangd/DraftStore.cpp
===================================================================
--- clangd/DraftStore.cpp
+++ clangd/DraftStore.cpp
@@ -21,6 +21,16 @@
   return It->second;
 }
 
+std::vector<Path> DraftStore::getActiveFiles() const {
+  std::lock_guard<std::mutex> Lock(Mutex);
+  std::vector<Path> ResultVector;
+
+  for (auto Draft : Drafts.keys())
+    ResultVector.push_back(Draft);
+
+  return ResultVector;
+}
+
 DocVersion DraftStore::getVersion(PathRef File) const {
   std::lock_guard<std::mutex> Lock(Mutex);
 
@@ -32,6 +42,7 @@
 
 DocVersion DraftStore::updateDraft(PathRef File, StringRef Contents) {
   std::lock_guard<std::mutex> Lock(Mutex);
+  assert(!File.empty());
 
   auto &Entry = Drafts[File];
   DocVersion NewVersion = ++Entry.Version;
@@ -41,6 +52,7 @@
 
 DocVersion DraftStore::removeDraft(PathRef File) {
   std::lock_guard<std::mutex> Lock(Mutex);
+  assert(!File.empty());
 
   auto &Entry = Drafts[File];
   DocVersion NewVersion = ++Entry.Version;
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -238,6 +238,12 @@
   /// and AST and rebuild them from scratch.
   std::future<void> forceReparse(PathRef File);
 
+  /// Calls forceReparse() on all currently opened files.
+  /// As a result, this method may be very expensive.
+  /// This method is normally called when the compilation database is changed.
+
+  std::vector<std::future<void>> reparseOpenedFiles();
+
   /// DEPRECATED. Please use a callback-based version, this API is deprecated
   /// and will soon be removed.
   ///
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -575,6 +575,21 @@
   return DoneFuture;
 }
 
+std::vector<std::future<void>> ClangdServer::reparseOpenedFiles() {
+
+  std::promise<void> DonePromise;
+  std::future<void> DoneFuture = DonePromise.get_future();
+  std::vector<std::future<void>> FutureVector;
+  std::vector<Path> ActiveFilePaths;
+
+  ActiveFilePaths = DraftMgr.getActiveFiles();
+
+  for (auto FilePath : ActiveFilePaths)
+    FutureVector.push_back(forceReparse(FilePath));
+
+  return FutureVector;
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -71,7 +71,8 @@
   void onFileEvent(Ctx C, DidChangeWatchedFilesParams &Params) override;
   void onCommand(Ctx C, ExecuteCommandParams &Params) override;
   void onRename(Ctx C, RenameParams &Parames) override;
-
+  void onChangeConfiguration(Ctx C,
+                             DidChangeConfigurationParams &Params) override;
   std::vector<clang::tooling::Replacement>
   getFixIts(StringRef File, const clangd::Diagnostic &D);
 
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -9,6 +9,11 @@
 
 #include "ClangdLSPServer.h"
 #include "JSONRPCDispatcher.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "llvm/Support/raw_ostream.h"
 
 #include "llvm/Support/FormatVariadic.h"
 
@@ -57,6 +62,7 @@
                  {"triggerCharacters", {"(", ","}},
              }},
             {"definitionProvider", true},
+            {"configurationChangeProvider", true},
             {"renameProvider", true},
             {"executeCommandProvider",
              json::obj{
@@ -234,6 +240,20 @@
   C.reply(Result ? URI::fromFile(*Result).uri : "");
 }
 
+void ClangdLSPServer::onChangeConfiguration(
+    Ctx C, DidChangeConfigurationParams &Params) {
+
+  // Parse whatever settings might be found inside Params.
+  ClangdConfigurationParamsChange Settings = Params.settings;
+
+  // Verify if path has value and is a valid path
+  if (Params.settings.compilationDatabasePath.hasValue()) {
+    CDB.setCompileCommandsDir(
+        Params.settings.compilationDatabasePath.getValue());
+    Server.reparseOpenedFiles();
+  }
+}
+
 ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount,
                                  bool StorePreamblesInMemory,
                                  bool SnippetCompletions,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to