[Mlir-commits] [mlir] ee7ccbe - [mlir:LSP][NFC] Make the LSPServer class private

River Riddle llvmlistbot at llvm.org
Wed Jul 20 01:46:23 PDT 2022


Author: River Riddle
Date: 2022-07-20T01:43:30-07:00
New Revision: ee7ccbeaa7d388dc9f7844f3d63ac07c3430d2ab

URL: https://github.com/llvm/llvm-project/commit/ee7ccbeaa7d388dc9f7844f3d63ac07c3430d2ab
DIFF: https://github.com/llvm/llvm-project/commit/ee7ccbeaa7d388dc9f7844f3d63ac07c3430d2ab.diff

LOG: [mlir:LSP][NFC] Make the LSPServer class private

There is no benefit to making it public, and the code is much
cleaner and easier to follow when inlined. This also matches
the pattern within the PDLL lsp server.

Added: 
    

Modified: 
    mlir/lib/Tools/mlir-lsp-server/LSPServer.cpp
    mlir/lib/Tools/mlir-lsp-server/LSPServer.h
    mlir/lib/Tools/mlir-lsp-server/MlirLspServerMain.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Tools/mlir-lsp-server/LSPServer.cpp b/mlir/lib/Tools/mlir-lsp-server/LSPServer.cpp
index 837514c503117..7f0727f237be2 100644
--- a/mlir/lib/Tools/mlir-lsp-server/LSPServer.cpp
+++ b/mlir/lib/Tools/mlir-lsp-server/LSPServer.cpp
@@ -20,12 +20,12 @@ using namespace mlir;
 using namespace mlir::lsp;
 
 //===----------------------------------------------------------------------===//
-// LSPServer::Impl
+// LSPServer
 //===----------------------------------------------------------------------===//
 
-struct LSPServer::Impl {
-  Impl(MLIRServer &server, JSONTransport &transport)
-      : server(server), transport(transport) {}
+namespace {
+struct LSPServer {
+  LSPServer(MLIRServer &server) : server(server) {}
 
   //===--------------------------------------------------------------------===//
   // Initialization
@@ -73,7 +73,6 @@ struct LSPServer::Impl {
   //===--------------------------------------------------------------------===//
 
   MLIRServer &server;
-  JSONTransport &transport;
 
   /// An outgoing notification used to send diagnostics to the client when they
   /// are ready to be processed.
@@ -83,12 +82,13 @@ struct LSPServer::Impl {
   /// Language Server client.
   bool shutdownRequestReceived = false;
 };
+} // namespace
 
 //===----------------------------------------------------------------------===//
 // Initialization
 
-void LSPServer::Impl::onInitialize(const InitializeParams &params,
-                                   Callback<llvm::json::Value> reply) {
+void LSPServer::onInitialize(const InitializeParams &params,
+                             Callback<llvm::json::Value> reply) {
   // Send a response with the capabilities of this server.
   llvm::json::Object serverCaps{
       {"textDocumentSync",
@@ -121,9 +121,8 @@ void LSPServer::Impl::onInitialize(const InitializeParams &params,
        {"capabilities", std::move(serverCaps)}}};
   reply(std::move(result));
 }
-void LSPServer::Impl::onInitialized(const InitializedParams &) {}
-void LSPServer::Impl::onShutdown(const NoParams &,
-                                 Callback<std::nullptr_t> reply) {
+void LSPServer::onInitialized(const InitializedParams &) {}
+void LSPServer::onShutdown(const NoParams &, Callback<std::nullptr_t> reply) {
   shutdownRequestReceived = true;
   reply(nullptr);
 }
@@ -131,8 +130,7 @@ void LSPServer::Impl::onShutdown(const NoParams &,
 //===----------------------------------------------------------------------===//
 // Document Change
 
-void LSPServer::Impl::onDocumentDidOpen(
-    const DidOpenTextDocumentParams &params) {
+void LSPServer::onDocumentDidOpen(const DidOpenTextDocumentParams &params) {
   PublishDiagnosticsParams diagParams(params.textDocument.uri,
                                       params.textDocument.version);
   server.addOrUpdateDocument(params.textDocument.uri, params.textDocument.text,
@@ -142,8 +140,7 @@ void LSPServer::Impl::onDocumentDidOpen(
   // Publish any recorded diagnostics.
   publishDiagnostics(diagParams);
 }
-void LSPServer::Impl::onDocumentDidClose(
-    const DidCloseTextDocumentParams &params) {
+void LSPServer::onDocumentDidClose(const DidCloseTextDocumentParams &params) {
   Optional<int64_t> version = server.removeDocument(params.textDocument.uri);
   if (!version)
     return;
@@ -154,8 +151,7 @@ void LSPServer::Impl::onDocumentDidClose(
   publishDiagnostics(
       PublishDiagnosticsParams(params.textDocument.uri, *version));
 }
-void LSPServer::Impl::onDocumentDidChange(
-    const DidChangeTextDocumentParams &params) {
+void LSPServer::onDocumentDidChange(const DidChangeTextDocumentParams &params) {
   // TODO: We currently only support full document updates, we should refactor
   // to avoid this.
   if (params.contentChanges.size() != 1)
@@ -173,15 +169,15 @@ void LSPServer::Impl::onDocumentDidChange(
 //===----------------------------------------------------------------------===//
 // Definitions and References
 
-void LSPServer::Impl::onGoToDefinition(const TextDocumentPositionParams &params,
-                                       Callback<std::vector<Location>> reply) {
+void LSPServer::onGoToDefinition(const TextDocumentPositionParams &params,
+                                 Callback<std::vector<Location>> reply) {
   std::vector<Location> locations;
   server.getLocationsOf(params.textDocument.uri, params.position, locations);
   reply(std::move(locations));
 }
 
-void LSPServer::Impl::onReference(const ReferenceParams &params,
-                                  Callback<std::vector<Location>> reply) {
+void LSPServer::onReference(const ReferenceParams &params,
+                            Callback<std::vector<Location>> reply) {
   std::vector<Location> locations;
   server.findReferencesOf(params.textDocument.uri, params.position, locations);
   reply(std::move(locations));
@@ -190,17 +186,16 @@ void LSPServer::Impl::onReference(const ReferenceParams &params,
 //===----------------------------------------------------------------------===//
 // Hover
 
-void LSPServer::Impl::onHover(const TextDocumentPositionParams &params,
-                              Callback<Optional<Hover>> reply) {
+void LSPServer::onHover(const TextDocumentPositionParams &params,
+                        Callback<Optional<Hover>> reply) {
   reply(server.findHover(params.textDocument.uri, params.position));
 }
 
 //===----------------------------------------------------------------------===//
 // Document Symbols
 
-void LSPServer::Impl::onDocumentSymbol(
-    const DocumentSymbolParams &params,
-    Callback<std::vector<DocumentSymbol>> reply) {
+void LSPServer::onDocumentSymbol(const DocumentSymbolParams &params,
+                                 Callback<std::vector<DocumentSymbol>> reply) {
   std::vector<DocumentSymbol> symbols;
   server.findDocumentSymbols(params.textDocument.uri, symbols);
   reply(std::move(symbols));
@@ -209,65 +204,64 @@ void LSPServer::Impl::onDocumentSymbol(
 //===----------------------------------------------------------------------===//
 // Code Completion
 
-void LSPServer::Impl::onCompletion(const CompletionParams &params,
-                                   Callback<CompletionList> reply) {
+void LSPServer::onCompletion(const CompletionParams &params,
+                             Callback<CompletionList> reply) {
   reply(server.getCodeCompletion(params.textDocument.uri, params.position));
 }
 
 //===----------------------------------------------------------------------===//
-// LSPServer
+// Entry point
 //===----------------------------------------------------------------------===//
 
-LSPServer::LSPServer(MLIRServer &server, JSONTransport &transport)
-    : impl(std::make_unique<Impl>(server, transport)) {}
-LSPServer::~LSPServer() = default;
-
-LogicalResult LSPServer::run() {
-  MessageHandler messageHandler(impl->transport);
+LogicalResult lsp::runMlirLSPServer(MLIRServer &server,
+                                    JSONTransport &transport) {
+  LSPServer lspServer(server);
+  MessageHandler messageHandler(transport);
 
   // Initialization
-  messageHandler.method("initialize", impl.get(), &Impl::onInitialize);
-  messageHandler.notification("initialized", impl.get(), &Impl::onInitialized);
-  messageHandler.method("shutdown", impl.get(), &Impl::onShutdown);
+  messageHandler.method("initialize", &lspServer, &LSPServer::onInitialize);
+  messageHandler.notification("initialized", &lspServer,
+                              &LSPServer::onInitialized);
+  messageHandler.method("shutdown", &lspServer, &LSPServer::onShutdown);
 
   // Document Changes
-  messageHandler.notification("textDocument/didOpen", impl.get(),
-                              &Impl::onDocumentDidOpen);
-  messageHandler.notification("textDocument/didClose", impl.get(),
-                              &Impl::onDocumentDidClose);
-  messageHandler.notification("textDocument/didChange", impl.get(),
-                              &Impl::onDocumentDidChange);
+  messageHandler.notification("textDocument/didOpen", &lspServer,
+                              &LSPServer::onDocumentDidOpen);
+  messageHandler.notification("textDocument/didClose", &lspServer,
+                              &LSPServer::onDocumentDidClose);
+  messageHandler.notification("textDocument/didChange", &lspServer,
+                              &LSPServer::onDocumentDidChange);
 
   // Definitions and References
-  messageHandler.method("textDocument/definition", impl.get(),
-                        &Impl::onGoToDefinition);
-  messageHandler.method("textDocument/references", impl.get(),
-                        &Impl::onReference);
+  messageHandler.method("textDocument/definition", &lspServer,
+                        &LSPServer::onGoToDefinition);
+  messageHandler.method("textDocument/references", &lspServer,
+                        &LSPServer::onReference);
 
   // Hover
-  messageHandler.method("textDocument/hover", impl.get(), &Impl::onHover);
+  messageHandler.method("textDocument/hover", &lspServer, &LSPServer::onHover);
 
   // Document Symbols
-  messageHandler.method("textDocument/documentSymbol", impl.get(),
-                        &Impl::onDocumentSymbol);
+  messageHandler.method("textDocument/documentSymbol", &lspServer,
+                        &LSPServer::onDocumentSymbol);
 
   // Code Completion
-  messageHandler.method("textDocument/completion", impl.get(),
-                        &Impl::onCompletion);
+  messageHandler.method("textDocument/completion", &lspServer,
+                        &LSPServer::onCompletion);
 
   // Diagnostics
-  impl->publishDiagnostics =
+  lspServer.publishDiagnostics =
       messageHandler.outgoingNotification<PublishDiagnosticsParams>(
           "textDocument/publishDiagnostics");
 
   // Run the main loop of the transport.
   LogicalResult result = success();
-  if (llvm::Error error = impl->transport.run(messageHandler)) {
+  if (llvm::Error error = transport.run(messageHandler)) {
     Logger::error("Transport error: {0}", error);
     llvm::consumeError(std::move(error));
     result = failure();
   } else {
-    result = success(impl->shutdownRequestReceived);
+    result = success(lspServer.shutdownRequestReceived);
   }
   return result;
 }

diff  --git a/mlir/lib/Tools/mlir-lsp-server/LSPServer.h b/mlir/lib/Tools/mlir-lsp-server/LSPServer.h
index 09d2fd243a5bc..faf5936012776 100644
--- a/mlir/lib/Tools/mlir-lsp-server/LSPServer.h
+++ b/mlir/lib/Tools/mlir-lsp-server/LSPServer.h
@@ -18,22 +18,9 @@ namespace lsp {
 class JSONTransport;
 class MLIRServer;
 
-/// This class represents the main LSP server, and handles communication with
-/// the LSP client.
-class LSPServer {
-public:
-  /// Construct a new language server with the given MLIR server.
-  LSPServer(MLIRServer &server, JSONTransport &transport);
-  ~LSPServer();
-
-  /// Run the main loop of the server.
-  LogicalResult run();
-
-private:
-  struct Impl;
-
-  std::unique_ptr<Impl> impl;
-};
+/// Run the main loop of the LSP server using the given MLIR server and
+/// transport.
+LogicalResult runMlirLSPServer(MLIRServer &server, JSONTransport &transport);
 } // namespace lsp
 } // namespace mlir
 

diff  --git a/mlir/lib/Tools/mlir-lsp-server/MlirLspServerMain.cpp b/mlir/lib/Tools/mlir-lsp-server/MlirLspServerMain.cpp
index 3dc5d7cf2444d..ed586818954f4 100644
--- a/mlir/lib/Tools/mlir-lsp-server/MlirLspServerMain.cpp
+++ b/mlir/lib/Tools/mlir-lsp-server/MlirLspServerMain.cpp
@@ -70,6 +70,5 @@ LogicalResult mlir::MlirLspServerMain(int argc, char **argv,
 
   // Configure the servers and start the main language server.
   MLIRServer server(registry);
-  LSPServer lspServer(server, transport);
-  return lspServer.run();
+  return runMlirLSPServer(server, transport);
 }


        


More information about the Mlir-commits mailing list