-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[lldb] NFC Moving mcp::Transport into its own file. #155711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Moving `lldb_protocol::mcp::MCPTransport` out of Server.h and into its own file and simplifying the name to `Transport`.
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesMoving Full diff: https://github.com/llvm/llvm-project/pull/155711.diff 9 Files Affected:
diff --git a/lldb/include/lldb/Protocol/MCP/MCPError.h b/lldb/include/lldb/Protocol/MCP/MCPError.h
index 55dd40f124a15..52c5a78fa23c0 100644
--- a/lldb/include/lldb/Protocol/MCP/MCPError.h
+++ b/lldb/include/lldb/Protocol/MCP/MCPError.h
@@ -19,7 +19,7 @@ class MCPError : public llvm::ErrorInfo<MCPError> {
public:
static char ID;
- MCPError(std::string message, int64_t error_code = kInternalError);
+ MCPError(std::string message, int64_t error_code = eErrorCodeInternalError);
void log(llvm::raw_ostream &OS) const override;
std::error_code convertToErrorCode() const override;
@@ -28,9 +28,6 @@ class MCPError : public llvm::ErrorInfo<MCPError> {
lldb_protocol::mcp::Error toProtocolError() const;
- static constexpr int64_t kResourceNotFound = -32002;
- static constexpr int64_t kInternalError = -32603;
-
private:
std::string m_message;
int64_t m_error_code;
diff --git a/lldb/include/lldb/Protocol/MCP/Protocol.h b/lldb/include/lldb/Protocol/MCP/Protocol.h
index 6e1ffcbe1f3e3..295f1f6e1b037 100644
--- a/lldb/include/lldb/Protocol/MCP/Protocol.h
+++ b/lldb/include/lldb/Protocol/MCP/Protocol.h
@@ -55,6 +55,11 @@ enum ErrorCode : signed {
eErrorCodeInvalidParams = -32602,
/// Internal JSON-RPC error.
eErrorCodeInternalError = -32603,
+
+ /// Additional MCP error codes.
+
+ /// Resource related uri not found.
+ eErrorCodeResourceNotFound = -32002,
};
struct Error {
diff --git a/lldb/include/lldb/Protocol/MCP/Server.h b/lldb/include/lldb/Protocol/MCP/Server.h
index aa5714e45755e..009c574fde92f 100644
--- a/lldb/include/lldb/Protocol/MCP/Server.h
+++ b/lldb/include/lldb/Protocol/MCP/Server.h
@@ -9,43 +9,20 @@
#ifndef LLDB_PROTOCOL_MCP_SERVER_H
#define LLDB_PROTOCOL_MCP_SERVER_H
-#include "lldb/Host/JSONTransport.h"
#include "lldb/Host/MainLoop.h"
#include "lldb/Protocol/MCP/Protocol.h"
#include "lldb/Protocol/MCP/Resource.h"
#include "lldb/Protocol/MCP/Tool.h"
+#include "lldb/Protocol/MCP/Transport.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/Error.h"
-#include <mutex>
namespace lldb_protocol::mcp {
-class MCPTransport
- : public lldb_private::JSONRPCTransport<Request, Response, Notification> {
-public:
- using LogCallback = std::function<void(llvm::StringRef message)>;
-
- MCPTransport(lldb::IOObjectSP in, lldb::IOObjectSP out,
- std::string client_name, LogCallback log_callback = {})
- : JSONRPCTransport(in, out), m_client_name(std::move(client_name)),
- m_log_callback(log_callback) {}
- virtual ~MCPTransport() = default;
-
- void Log(llvm::StringRef message) override {
- if (m_log_callback)
- m_log_callback(llvm::formatv("{0}: {1}", m_client_name, message).str());
- }
-
-private:
- std::string m_client_name;
- LogCallback m_log_callback;
-};
-
-class Server : public MCPTransport::MessageHandler {
+class Server : public Transport::MessageHandler {
public:
Server(std::string name, std::string version,
- std::unique_ptr<MCPTransport> transport_up,
- lldb_private::MainLoop &loop);
+ std::unique_ptr<Transport> transport_up, lldb_private::MainLoop &loop);
~Server() = default;
using NotificationHandler = std::function<void(const Notification &)>;
@@ -92,7 +69,7 @@ class Server : public MCPTransport::MessageHandler {
const std::string m_name;
const std::string m_version;
- std::unique_ptr<MCPTransport> m_transport_up;
+ std::unique_ptr<Transport> m_transport_up;
lldb_private::MainLoop &m_loop;
llvm::StringMap<std::unique_ptr<Tool>> m_tools;
diff --git a/lldb/include/lldb/Protocol/MCP/Transport.h b/lldb/include/lldb/Protocol/MCP/Transport.h
new file mode 100644
index 0000000000000..3cb725131f3b0
--- /dev/null
+++ b/lldb/include/lldb/Protocol/MCP/Transport.h
@@ -0,0 +1,39 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_PROTOCOL_MCP_TRANSPORT_H
+#define LLDB_PROTOCOL_MCP_TRANSPORT_H
+
+#include "lldb/Host/JSONTransport.h"
+#include "lldb/Protocol/MCP/Protocol.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringRef.h"
+#include <functional>
+#include <string>
+
+namespace lldb_protocol::mcp {
+
+class Transport
+ : public lldb_private::JSONRPCTransport<Request, Response, Notification> {
+public:
+ using LogCallback = std::function<void(llvm::StringRef message)>;
+
+ Transport(lldb::IOObjectSP in, lldb::IOObjectSP out, std::string client_name,
+ LogCallback log_callback = {});
+ virtual ~Transport() = default;
+
+ void Log(llvm::StringRef message) override;
+
+private:
+ std::string m_client_name;
+ LogCallback m_log_callback;
+};
+
+} // namespace lldb_protocol::mcp
+
+#endif
diff --git a/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp b/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp
index 57132534cf680..a9c4164313a6d 100644
--- a/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp
+++ b/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp
@@ -67,7 +67,7 @@ void ProtocolServerMCP::AcceptCallback(std::unique_ptr<Socket> socket) {
LLDB_LOG(log, "New MCP client connected: {0}", client_name);
lldb::IOObjectSP io_sp = std::move(socket);
- auto transport_up = std::make_unique<lldb_protocol::mcp::MCPTransport>(
+ auto transport_up = std::make_unique<lldb_protocol::mcp::Transport>(
io_sp, io_sp, std::move(client_name), [&](llvm::StringRef message) {
LLDB_LOG(GetLog(LLDBLog::Host), "{0}", message);
});
diff --git a/lldb/source/Protocol/MCP/CMakeLists.txt b/lldb/source/Protocol/MCP/CMakeLists.txt
index a73e7e6a7cab1..23da62085537e 100644
--- a/lldb/source/Protocol/MCP/CMakeLists.txt
+++ b/lldb/source/Protocol/MCP/CMakeLists.txt
@@ -3,6 +3,7 @@ add_lldb_library(lldbProtocolMCP NO_PLUGIN_DEPENDENCIES
Protocol.cpp
Server.cpp
Tool.cpp
+ Transport.cpp
LINK_COMPONENTS
Support
diff --git a/lldb/source/Protocol/MCP/Server.cpp b/lldb/source/Protocol/MCP/Server.cpp
index 63c2d01d17922..fb317083b015e 100644
--- a/lldb/source/Protocol/MCP/Server.cpp
+++ b/lldb/source/Protocol/MCP/Server.cpp
@@ -15,7 +15,7 @@ using namespace lldb_protocol::mcp;
using namespace llvm;
Server::Server(std::string name, std::string version,
- std::unique_ptr<MCPTransport> transport_up,
+ std::unique_ptr<Transport> transport_up,
lldb_private::MainLoop &loop)
: m_name(std::move(name)), m_version(std::move(version)),
m_transport_up(std::move(transport_up)), m_loop(loop) {
@@ -180,7 +180,7 @@ llvm::Expected<Response> Server::ResourcesReadHandler(const Request &request) {
return make_error<MCPError>(
llvm::formatv("no resource handler for uri: {0}", uri_str).str(),
- MCPError::kResourceNotFound);
+ eErrorCodeResourceNotFound);
}
ServerCapabilities Server::GetCapabilities() {
@@ -219,7 +219,7 @@ void Server::Received(const Request &request) {
response.takeError(),
[&](const MCPError &err) { protocol_error = err.toProtocolError(); },
[&](const llvm::ErrorInfoBase &err) {
- protocol_error.code = MCPError::kInternalError;
+ protocol_error.code = eErrorCodeInternalError;
protocol_error.message = err.message();
});
Response error_response;
diff --git a/lldb/source/Protocol/MCP/Transport.cpp b/lldb/source/Protocol/MCP/Transport.cpp
new file mode 100644
index 0000000000000..58c66af7320ed
--- /dev/null
+++ b/lldb/source/Protocol/MCP/Transport.cpp
@@ -0,0 +1,32 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Protocol/MCP/Transport.h"
+#include "lldb/Host/JSONTransport.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FormatVariadic.h"
+#include <string>
+#include <utility>
+
+using namespace llvm;
+using namespace lldb;
+
+namespace lldb_protocol::mcp {
+
+Transport::Transport(IOObjectSP in, IOObjectSP out, std::string client_name,
+ LogCallback log_callback)
+ : JSONRPCTransport(in, out), m_client_name(std::move(client_name)),
+ m_log_callback(log_callback) {}
+
+void Transport::Log(StringRef message) {
+ if (m_log_callback)
+ m_log_callback(formatv("{0}: {1}", m_client_name, message).str());
+}
+
+} // namespace lldb_protocol::mcp
diff --git a/lldb/unittests/Protocol/ProtocolMCPServerTest.cpp b/lldb/unittests/Protocol/ProtocolMCPServerTest.cpp
index 9fa446133d46f..846582ab9a74e 100644
--- a/lldb/unittests/Protocol/ProtocolMCPServerTest.cpp
+++ b/lldb/unittests/Protocol/ProtocolMCPServerTest.cpp
@@ -21,6 +21,7 @@
#include "lldb/Protocol/MCP/Resource.h"
#include "lldb/Protocol/MCP/Server.h"
#include "lldb/Protocol/MCP/Tool.h"
+#include "lldb/Protocol/MCP/Transport.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/JSON.h"
@@ -36,12 +37,12 @@ using namespace lldb_private;
using namespace lldb_protocol::mcp;
namespace {
-class TestMCPTransport final : public MCPTransport {
+class TestMCPTransport final : public lldb_protocol::mcp::Transport {
public:
TestMCPTransport(lldb::IOObjectSP in, lldb::IOObjectSP out)
- : lldb_protocol::mcp::MCPTransport(in, out, "unittest") {}
+ : lldb_protocol::mcp::Transport(in, out, "unittest") {}
- using MCPTransport::Write;
+ using Transport::Write;
void Log(llvm::StringRef message) override {
log_messages.emplace_back(message);
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/29889 Here is the relevant piece of the build log for the reference
|
This reverts commit 71a065e.
i had to revert it |
FYI #155811 was the fix for the build error... |
…oject into bugprone-method-hiding * 'bugprone-method-hiding' of github.com:t-a-james/llvm-project: (230 commits) [SimplifyCFG] Move token type check into canReplaceOperandWithVariable() [ADT] Fix signed integer overflow (llvm#155826) [Offload] Update LIBOMPTARGET_INFO text for `attach` map-type. (llvm#155509) [CMake][AIX] Enable CMP0182: Create shared library archives by default (llvm#155686) AMDGPU: Add tests for atomics with AGPR operands (llvm#155820) [AArch64] Split zero cycle zeoring per register class (llvm#154561) [gn build] Port fa883e1 [mlir][tosa] Allow shift operand of tosa::MulOp as non-constant (llvm#155197) [AArch64][NFC] Add MCInstrAnalysis unittests (llvm#155609) [Offload][OpenMP] Tests require libc on GPU for printf (llvm#155785) AMDGPU: Add missing verifier tests for load/store AGPR case (llvm#155815) [lldb-mcp] Fix building for Windows Revert "[lldb] Correct a usage after a rename was merged. (llvm#155720)" Revert "[lldb] NFC Moving mcp::Transport into its own file. (llvm#155711)" [lldb][test] Run ranges::ref_vew test only for libc++ (llvm#155813) [SCCP][FuncSpec] Poison unreachable constant global variable user (llvm#155753) [LoongArch] Lowering v32i8 vector mask generation to `VMSKLTZ` (llvm#149953) [flang][docs][NFC] Remove stray backtick (llvm#154974) [MLIR] Apply clang-tidy fixes for misc-use-internal-linkage in LinalgOps.cpp (NFC) [MLIR] Apply clang-tidy fixes for performance-move-const-arg in VariantValue.cpp (NFC) ...
Moving
lldb_protocol::mcp::MCPTransport
out of Server.h and into its own file and simplifying the name toTransport
.