-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[lldb] Moving MCPTransport into its own file. #156712
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Moving `lldb_protocol::mcp::MCPTransport` into its own file and renaming to `lldb_protocol::mcp::Transport`.
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesMoving Full diff: https://github.com/llvm/llvm-project/pull/156712.diff 7 Files Affected:
diff --git a/lldb/include/lldb/Host/JSONTransport.h b/lldb/include/lldb/Host/JSONTransport.h
index 0be60a8f3f96a..210f33edace6e 100644
--- a/lldb/include/lldb/Host/JSONTransport.h
+++ b/lldb/include/lldb/Host/JSONTransport.h
@@ -100,7 +100,8 @@ template <typename Req, typename Resp, typename Evt> class Transport {
virtual llvm::Expected<MainLoop::ReadHandleUP>
RegisterMessageHandler(MainLoop &loop, MessageHandler &handler) = 0;
-protected:
+ // FIXME: Refactor mcp::Server to not directly access log on the transport.
+ // protected:
template <typename... Ts> inline auto Logv(const char *Fmt, Ts &&...Vals) {
Log(llvm::formatv(Fmt, std::forward<Ts>(Vals)...).str());
}
@@ -139,9 +140,7 @@ class JSONTransport : public Transport<Req, Resp, Evt> {
/// detail.
static constexpr size_t kReadBufferSize = 1024;
-protected:
- virtual llvm::Expected<std::vector<std::string>> Parse() = 0;
- virtual std::string Encode(const llvm::json::Value &message) = 0;
+ // FIXME: Write should be protected.
llvm::Error Write(const llvm::json::Value &message) {
this->Logv("<-- {0}", message);
std::string output = Encode(message);
@@ -149,6 +148,10 @@ class JSONTransport : public Transport<Req, Resp, Evt> {
return m_out->Write(output.data(), bytes_written).takeError();
}
+protected:
+ virtual llvm::Expected<std::vector<std::string>> Parse() = 0;
+ virtual std::string Encode(const llvm::json::Value &message) = 0;
+
llvm::SmallString<kReadBufferSize> m_buffer;
private:
diff --git a/lldb/include/lldb/Protocol/MCP/Server.h b/lldb/include/lldb/Protocol/MCP/Server.h
index c6e78a9ea0cff..254b7d9680cd8 100644
--- a/lldb/include/lldb/Protocol/MCP/Server.h
+++ b/lldb/include/lldb/Protocol/MCP/Server.h
@@ -14,33 +14,17 @@
#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>
+#include "llvm/Support/JSON.h"
+#include <functional>
+#include <memory>
+#include <string>
+#include <vector>
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;
-};
-
/// Information about this instance of lldb's MCP server for lldb-mcp to use to
/// coordinate connecting an lldb-mcp client.
struct ServerInfo {
diff --git a/lldb/include/lldb/Protocol/MCP/Transport.h b/lldb/include/lldb/Protocol/MCP/Transport.h
new file mode 100644
index 0000000000000..47c2ccfc44dfe
--- /dev/null
+++ b/lldb/include/lldb/Protocol/MCP/Transport.h
@@ -0,0 +1,48 @@
+//===----------------------------------------------------------------------===//
+//
+// 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/FunctionExtras.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace lldb_protocol::mcp {
+
+/// Generic transport that uses the MCP protocol.
+using MCPTransport = lldb_private::Transport<Request, Response, Notification>;
+
+/// Generic logging callback, to allow the MCP server / client / transport layer
+/// to be independent of the lldb log implementation.
+using LogCallback = llvm::unique_function<void(llvm::StringRef message)>;
+
+class Transport final
+ : public lldb_private::JSONRPCTransport<Request, Response, Notification> {
+public:
+ Transport(lldb::IOObjectSP in, lldb::IOObjectSP out,
+ LogCallback log_callback = {});
+ virtual ~Transport() = default;
+
+ /// Transport is not copyable.
+ /// @{
+ Transport(const Transport &) = delete;
+ void operator=(const Transport &) = delete;
+ /// @}
+
+ void Log(llvm::StringRef message) override;
+
+private:
+ 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 12cb25788534e..2b004c19e88a6 100644
--- a/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp
+++ b/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp
@@ -70,9 +70,9 @@ 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>(
- io_sp, io_sp, std::move(client_name), [&](llvm::StringRef message) {
- LLDB_LOG(GetLog(LLDBLog::Host), "{0}", message);
+ auto transport_up = std::make_unique<lldb_protocol::mcp::Transport>(
+ io_sp, io_sp, [client_name](llvm::StringRef message) {
+ LLDB_LOG(GetLog(LLDBLog::Host), "{0}: {1}", client_name, message);
});
auto instance_up = std::make_unique<lldb_protocol::mcp::Server>(
std::string(kName), std::string(kVersion), std::move(transport_up),
diff --git a/lldb/source/Protocol/MCP/CMakeLists.txt b/lldb/source/Protocol/MCP/CMakeLists.txt
index a4f270a83c43b..5258cb61a7d10 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/Transport.cpp b/lldb/source/Protocol/MCP/Transport.cpp
new file mode 100644
index 0000000000000..cccdc3b5bd65c
--- /dev/null
+++ b/lldb/source/Protocol/MCP/Transport.cpp
@@ -0,0 +1,23 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 "llvm/ADT/StringRef.h"
+#include <utility>
+
+using namespace lldb_protocol::mcp;
+using namespace llvm;
+
+Transport::Transport(lldb::IOObjectSP in, lldb::IOObjectSP out,
+ LogCallback log_callback)
+ : JSONRPCTransport(in, out), m_log_callback(std::move(log_callback)) {}
+
+void Transport::Log(StringRef message) {
+ if (m_log_callback)
+ m_log_callback(message);
+}
diff --git a/lldb/unittests/Protocol/ProtocolMCPServerTest.cpp b/lldb/unittests/Protocol/ProtocolMCPServerTest.cpp
index 9fa446133d46f..f686255c6d41d 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,19 +37,6 @@ using namespace lldb_private;
using namespace lldb_protocol::mcp;
namespace {
-class TestMCPTransport final : public MCPTransport {
-public:
- TestMCPTransport(lldb::IOObjectSP in, lldb::IOObjectSP out)
- : lldb_protocol::mcp::MCPTransport(in, out, "unittest") {}
-
- using MCPTransport::Write;
-
- void Log(llvm::StringRef message) override {
- log_messages.emplace_back(message);
- }
-
- std::vector<std::string> log_messages;
-};
class TestServer : public Server {
public:
@@ -134,7 +122,7 @@ class ProtocolServerMCPTest : public PipePairTest {
public:
SubsystemRAII<FileSystem, HostInfo, Socket> subsystems;
- std::unique_ptr<TestMCPTransport> transport_up;
+ std::unique_ptr<lldb_protocol::mcp::Transport> transport_up;
std::unique_ptr<TestServer> server_up;
MainLoop loop;
MockMessageHandler<Request, Response, Notification> message_handler;
@@ -163,7 +151,7 @@ class ProtocolServerMCPTest : public PipePairTest {
void SetUp() override {
PipePairTest::SetUp();
- transport_up = std::make_unique<TestMCPTransport>(
+ transport_up = std::make_unique<lldb_protocol::mcp::Transport>(
std::make_shared<NativeFile>(input.GetReadFileDescriptor(),
File::eOpenOptionReadOnly,
NativeFile::Unowned),
@@ -173,7 +161,7 @@ class ProtocolServerMCPTest : public PipePairTest {
server_up = std::make_unique<TestServer>(
"lldb-mcp", "0.1.0",
- std::make_unique<TestMCPTransport>(
+ std::make_unique<lldb_protocol::mcp::Transport>(
std::make_shared<NativeFile>(output.GetReadFileDescriptor(),
File::eOpenOptionReadOnly,
NativeFile::Unowned),
|
JDevlieghere
approved these changes
Sep 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Moving
lldb_protocol::mcp::MCPTransport
into its own file and renaming tolldb_protocol::mcp::Transport
.