[clang-tools-extra] r345235 - [clangd] Clean up LSP structs around configuration. NFC, no protocol changes.

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 24 21:22:52 PDT 2018


Author: sammccall
Date: Wed Oct 24 21:22:52 2018
New Revision: 345235

URL: http://llvm.org/viewvc/llvm-project?rev=345235&view=rev
Log:
[clangd] Clean up LSP structs around configuration. NFC, no protocol changes.

 - align struct names/comments with LSP, remove redundant "clangd" prefixes.
 - don't map config structs as Optional<> when their presence/absence
   doesn't signal anything and all fields must have sensible "absent" values
 - be more lax around parsing of 'any'-typed messages

Modified:
    clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
    clang-tools-extra/trunk/clangd/ClangdLSPServer.h
    clang-tools-extra/trunk/clangd/Protocol.cpp
    clang-tools-extra/trunk/clangd/Protocol.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=345235&r1=345234&r2=345235&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Oct 24 21:22:52 2018
@@ -303,15 +303,14 @@ void ClangdLSPServer::onInitialize(const
   if (Server)
     return Reply(make_error<LSPError>("server already initialized",
                                       ErrorCode::InvalidRequest));
-  if (Params.initializationOptions)
-    CompileCommandsDir = Params.initializationOptions->compilationDatabasePath;
+  if (const auto &Dir = Params.initializationOptions.compilationDatabasePath)
+    CompileCommandsDir = Dir;
   CDB.emplace(UseInMemoryCDB
                   ? CompilationDB::makeInMemory()
                   : CompilationDB::makeDirectoryBased(CompileCommandsDir));
   Server.emplace(CDB->getCDB(), FSProvider,
                  static_cast<DiagnosticsConsumer &>(*this), ClangdServerOpts);
-  if (Params.initializationOptions)
-    applyConfiguration(Params.initializationOptions->ParamsChange);
+  applyConfiguration(Params.initializationOptions.ConfigSettings);
 
   CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets;
   DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes;
@@ -654,25 +653,22 @@ void ClangdLSPServer::onHover(const Text
 }
 
 void ClangdLSPServer::applyConfiguration(
-    const ClangdConfigurationParamsChange &Params) {
+    const ConfigurationSettings &Settings) {
   // 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
-      /// entries are changed.
-      PathRef File = Entry.first;
-      if (!CDB->setCompilationCommandForFile(
-              File, tooling::CompileCommand(
-                        std::move(Entry.second.workingDirectory), File,
-                        std::move(Entry.second.compilationCommand),
-                        /*Output=*/"")))
-        ShouldReparseOpenFiles = true;
-    }
-    if (ShouldReparseOpenFiles)
-      reparseOpenedFiles();
+  bool ShouldReparseOpenFiles = false;
+  for (auto &Entry : Settings.compilationDatabaseChanges) {
+    /// The opened files need to be reparsed only when some existing
+    /// entries are changed.
+    PathRef File = Entry.first;
+    if (!CDB->setCompilationCommandForFile(
+            File, tooling::CompileCommand(
+                      std::move(Entry.second.workingDirectory), File,
+                      std::move(Entry.second.compilationCommand),
+                      /*Output=*/"")))
+      ShouldReparseOpenFiles = true;
   }
+  if (ShouldReparseOpenFiles)
+    reparseOpenedFiles();
 }
 
 // FIXME: This function needs to be properly tested.

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=345235&r1=345234&r2=345235&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Oct 24 21:22:52 2018
@@ -93,7 +93,7 @@ private:
   /// may be very expensive.  This method is normally called when the
   /// compilation database is changed.
   void reparseOpenedFiles();
-  void applyConfiguration(const ClangdConfigurationParamsChange &Settings);
+  void applyConfiguration(const ConfigurationSettings &Settings);
 
   /// Used to indicate that the 'shutdown' request was received from the
   /// Language Server client.

Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=345235&r1=345234&r2=345235&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Wed Oct 24 21:22:52 2018
@@ -663,20 +663,22 @@ bool fromJSON(const json::Value &Params,
          O.map("compilationCommand", CDbUpdate.compilationCommand);
 }
 
-bool fromJSON(const json::Value &Params,
-              ClangdConfigurationParamsChange &CCPC) {
+bool fromJSON(const json::Value &Params, ConfigurationSettings &S) {
   json::ObjectMapper O(Params);
-  return O &&
-         O.map("compilationDatabaseChanges", CCPC.compilationDatabaseChanges);
+  if (!O)
+    return true; // 'any' type in LSP.
+  O.map("compilationDatabaseChanges", S.compilationDatabaseChanges);
+  return true;
 }
 
-bool fromJSON(const json::Value &Params, ClangdInitializationOptions &Opts) {
-  if (!fromJSON(Params, Opts.ParamsChange)) {
-    return false;
-  }
-
+bool fromJSON(const json::Value &Params, InitializationOptions &Opts) {
   json::ObjectMapper O(Params);
-  return O && O.map("compilationDatabasePath", Opts.compilationDatabasePath);
+  if (!O)
+    return true; // 'any' type in LSP.
+
+  fromJSON(Params, Opts.ConfigSettings);
+  O.map("compilationDatabasePath", Opts.compilationDatabasePath);
+  return true;
 }
 
 bool fromJSON(const json::Value &Params, ReferenceParams &R) {

Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=345235&r1=345234&r2=345235&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Wed Oct 24 21:22:52 2018
@@ -355,25 +355,26 @@ struct ClangdCompileCommand {
 };
 bool fromJSON(const llvm::json::Value &, ClangdCompileCommand &);
 
-/// Clangd extension to set clangd-specific "initializationOptions" in the
-/// "initialize" request and for the "workspace/didChangeConfiguration"
-/// notification since the data received is described as 'any' type in LSP.
-struct ClangdConfigurationParamsChange {
-  // The changes that happened to the compilation database.
+/// 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.
-  llvm::Optional<std::map<std::string, ClangdCompileCommand>>
-      compilationDatabaseChanges;
+  std::map<std::string, ClangdCompileCommand> compilationDatabaseChanges;
 };
-bool fromJSON(const llvm::json::Value &, ClangdConfigurationParamsChange &);
+bool fromJSON(const llvm::json::Value &, ConfigurationSettings &);
 
-struct ClangdInitializationOptions {
+/// 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).
-  ClangdConfigurationParamsChange ParamsChange;
+  ConfigurationSettings ConfigSettings;
 
   llvm::Optional<std::string> compilationDatabasePath;
 };
-bool fromJSON(const llvm::json::Value &, ClangdInitializationOptions &);
+bool fromJSON(const llvm::json::Value &, InitializationOptions &);
 
 struct InitializeParams {
   /// The process Id of the parent process that started
@@ -402,9 +403,8 @@ struct InitializeParams {
   /// The initial trace setting. If omitted trace is disabled ('off').
   llvm::Optional<TraceLevel> trace;
 
-  // We use this predefined struct because it is easier to use
-  // than the protocol specified type of 'any'.
-  llvm::Optional<ClangdInitializationOptions> initializationOptions;
+  /// User-provided initialization options.
+  InitializationOptions initializationOptions;
 };
 bool fromJSON(const llvm::json::Value &, InitializeParams &);
 
@@ -477,9 +477,7 @@ struct DidChangeWatchedFilesParams {
 bool fromJSON(const llvm::json::Value &, DidChangeWatchedFilesParams &);
 
 struct DidChangeConfigurationParams {
-  // We use this predefined struct because it is easier to use
-  // than the protocol specified type of 'any'.
-  ClangdConfigurationParamsChange settings;
+  ConfigurationSettings settings;
 };
 bool fromJSON(const llvm::json::Value &, DidChangeConfigurationParams &);
 




More information about the cfe-commits mailing list