[Lldb-commits] [lldb] [lldb] Move Transport class into lldb_private (NFC) (PR #143806)
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Wed Jun 11 18:09:11 PDT 2025
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/143806
>From 7fad13d3dba1974ece6f4e7da6fe4f48d7f3b4b0 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Wed, 11 Jun 2025 16:17:21 -0700
Subject: [PATCH 1/2] [lldb] Move Transport class into lldb_private (NFC)
Move lldb-dap's Transport class into lldb_private so the code can be
shared between the "JSON with header" protocol used by DAP and the JSON
RPC protocol used by MCP (see [1]).
[1]: https://discourse.llvm.org/t/rfc-adding-mcp-support-to-lldb/86798
---
lldb/include/lldb/Host/JSONTransport.h | 131 +++++++++++++++++++
lldb/source/Host/CMakeLists.txt | 3 +-
lldb/source/Host/common/JSONTransport.cpp | 149 ++++++++++++++++++++++
lldb/tools/lldb-dap/DAP.cpp | 7 +-
lldb/tools/lldb-dap/Transport.cpp | 144 +--------------------
lldb/tools/lldb-dap/Transport.h | 70 +---------
lldb/unittests/DAP/DAPTest.cpp | 7 +-
lldb/unittests/DAP/TestBase.cpp | 3 +-
lldb/unittests/DAP/TransportTest.cpp | 16 ++-
9 files changed, 313 insertions(+), 217 deletions(-)
create mode 100644 lldb/include/lldb/Host/JSONTransport.h
create mode 100644 lldb/source/Host/common/JSONTransport.cpp
diff --git a/lldb/include/lldb/Host/JSONTransport.h b/lldb/include/lldb/Host/JSONTransport.h
new file mode 100644
index 0000000000000..907351abf8f74
--- /dev/null
+++ b/lldb/include/lldb/Host/JSONTransport.h
@@ -0,0 +1,131 @@
+//===-- JSONTransport.h ---------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// Transport layer for encoding and decoding JSON protocol messages.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_HOST_JSONTRANSPORT_H
+#define LLDB_HOST_JSONTRANSPORT_H
+
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/JSON.h"
+#include <chrono>
+#include <system_error>
+
+namespace lldb_private {
+
+class TransportEOFError : public llvm::ErrorInfo<TransportEOFError> {
+public:
+ static char ID;
+
+ TransportEOFError() = default;
+
+ void log(llvm::raw_ostream &OS) const override {
+ OS << "transport end of file reached";
+ }
+ std::error_code convertToErrorCode() const override {
+ return llvm::inconvertibleErrorCode();
+ }
+};
+
+class TransportTimeoutError : public llvm::ErrorInfo<TransportTimeoutError> {
+public:
+ static char ID;
+
+ TransportTimeoutError() = default;
+
+ void log(llvm::raw_ostream &OS) const override {
+ OS << "transport operation timed out";
+ }
+ std::error_code convertToErrorCode() const override {
+ return std::make_error_code(std::errc::timed_out);
+ }
+};
+
+class TransportClosedError : public llvm::ErrorInfo<TransportClosedError> {
+public:
+ static char ID;
+
+ TransportClosedError() = default;
+
+ void log(llvm::raw_ostream &OS) const override {
+ OS << "transport is closed";
+ }
+ std::error_code convertToErrorCode() const override {
+ return llvm::inconvertibleErrorCode();
+ }
+};
+
+/// A transport class that uses JSON for communication.
+class JSONTransport {
+public:
+ JSONTransport(llvm::StringRef client_name, lldb::IOObjectSP input,
+ lldb::IOObjectSP output);
+ virtual ~JSONTransport() = default;
+
+ /// Transport is not copyable.
+ /// @{
+ JSONTransport(const JSONTransport &rhs) = delete;
+ void operator=(const JSONTransport &rhs) = delete;
+ /// @}
+
+ /// Writes a message to the output stream.
+ template <typename T> llvm::Error Write(const T &t) {
+ const std::string message = llvm::formatv("{0}", toJSON(t)).str();
+ return WriteImpl(message);
+ }
+
+ /// Reads the next message from the input stream.
+ template <typename T>
+ llvm::Expected<T> Read(const std::chrono::microseconds &timeout) {
+ llvm::Expected<std::string> message = ReadImpl(timeout);
+ if (!message)
+ return message.takeError();
+ return llvm::json::parse<T>(/*JSON=*/*message,
+ /*RootName=*/"transport_message");
+ }
+
+ /// Returns the name of this transport client, for example `stdin/stdout` or
+ /// `client_1`.
+ llvm::StringRef GetClientName() { return m_client_name; }
+
+protected:
+ virtual void Log(llvm::StringRef message);
+ virtual llvm::Error WriteImpl(const std::string &message) = 0;
+ virtual llvm::Expected<std::string>
+ ReadImpl(const std::chrono::microseconds &timeout) = 0;
+
+ llvm::StringRef m_client_name;
+ lldb::IOObjectSP m_input;
+ lldb::IOObjectSP m_output;
+};
+
+/// A transport class that uses JSON with a header for communication.
+class JSONWithHeaderTransport : public JSONTransport {
+public:
+ JSONWithHeaderTransport(llvm::StringRef client_name, lldb::IOObjectSP input,
+ lldb::IOObjectSP output)
+ : JSONTransport(client_name, input, output) {}
+ virtual ~JSONWithHeaderTransport() = default;
+
+ virtual llvm::Error WriteImpl(const std::string &message) override;
+ virtual llvm::Expected<std::string>
+ ReadImpl(const std::chrono::microseconds &timeout) override;
+
+ static constexpr llvm::StringLiteral kHeaderContentLength =
+ "Content-Length: ";
+ static constexpr llvm::StringLiteral kHeaderSeparator = "\r\n\r\n";
+};
+
+} // namespace lldb_private
+
+#endif
diff --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt
index 5b713133afeaf..b15d72e61b6e5 100644
--- a/lldb/source/Host/CMakeLists.txt
+++ b/lldb/source/Host/CMakeLists.txt
@@ -27,8 +27,9 @@ add_host_subdirectory(common
common/HostNativeThreadBase.cpp
common/HostProcess.cpp
common/HostThread.cpp
- common/LockFileBase.cpp
+ common/JSONTransport.cpp
common/LZMA.cpp
+ common/LockFileBase.cpp
common/MainLoopBase.cpp
common/MemoryMonitor.cpp
common/MonitoringProcessLauncher.cpp
diff --git a/lldb/source/Host/common/JSONTransport.cpp b/lldb/source/Host/common/JSONTransport.cpp
new file mode 100644
index 0000000000000..32d1889f69c02
--- /dev/null
+++ b/lldb/source/Host/common/JSONTransport.cpp
@@ -0,0 +1,149 @@
+//===-- JSONTransport.cpp -------------------------------------------------===//
+//
+// 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/Host/JSONTransport.h"
+#include "lldb/Utility/IOObject.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/SelectHelper.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+#include <optional>
+#include <string>
+#include <utility>
+
+using namespace llvm;
+using namespace lldb;
+using namespace lldb_private;
+
+/// ReadFull attempts to read the specified number of bytes. If EOF is
+/// encountered, an empty string is returned.
+static Expected<std::string>
+ReadFull(IOObject &descriptor, size_t length,
+ std::optional<std::chrono::microseconds> timeout = std::nullopt) {
+ if (!descriptor.IsValid())
+ return llvm::make_error<TransportClosedError>();
+
+ bool timeout_supported = true;
+ // FIXME: SelectHelper does not work with NativeFile on Win32.
+#if _WIN32
+ timeout_supported = descriptor.GetFdType() == IOObject::eFDTypeSocket;
+#endif
+
+ if (timeout && timeout_supported) {
+ SelectHelper sh;
+ sh.SetTimeout(*timeout);
+ sh.FDSetRead(descriptor.GetWaitableHandle());
+ Status status = sh.Select();
+ if (status.Fail()) {
+ // Convert timeouts into a specific error.
+ if (status.GetType() == lldb::eErrorTypePOSIX &&
+ status.GetError() == ETIMEDOUT)
+ return make_error<TransportTimeoutError>();
+ return status.takeError();
+ }
+ }
+
+ std::string data;
+ data.resize(length);
+ Status status = descriptor.Read(data.data(), length);
+ if (status.Fail())
+ return status.takeError();
+
+ // Read returns '' on EOF.
+ if (length == 0)
+ return make_error<TransportEOFError>();
+
+ // Return the actual number of bytes read.
+ return data.substr(0, length);
+}
+
+static Expected<std::string>
+ReadUntil(IOObject &descriptor, StringRef delimiter,
+ std::optional<std::chrono::microseconds> timeout = std::nullopt) {
+ std::string buffer;
+ buffer.reserve(delimiter.size() + 1);
+ while (!llvm::StringRef(buffer).ends_with(delimiter)) {
+ Expected<std::string> next =
+ ReadFull(descriptor, buffer.empty() ? delimiter.size() : 1, timeout);
+ if (auto Err = next.takeError())
+ return std::move(Err);
+ buffer += *next;
+ }
+ return buffer.substr(0, buffer.size() - delimiter.size());
+}
+
+JSONTransport::JSONTransport(StringRef client_name, IOObjectSP input,
+ IOObjectSP output)
+ : m_client_name(client_name), m_input(std::move(input)),
+ m_output(std::move(output)) {}
+
+void JSONTransport::Log(llvm::StringRef message) {
+ LLDB_LOG(GetLog(LLDBLog::Host), "{0}", message);
+}
+
+Expected<std::string>
+JSONWithHeaderTransport::ReadImpl(const std::chrono::microseconds &timeout) {
+ if (!m_input || !m_input->IsValid())
+ return createStringError("transport output is closed");
+
+ IOObject *input = m_input.get();
+ Expected<std::string> message_header =
+ ReadFull(*input, kHeaderContentLength.size(), timeout);
+ if (!message_header)
+ return message_header.takeError();
+ if (*message_header != kHeaderContentLength)
+ return createStringError(formatv("expected '{0}' and got '{1}'",
+ kHeaderContentLength, *message_header)
+ .str());
+
+ Expected<std::string> raw_length = ReadUntil(*input, kHeaderSeparator);
+ if (!raw_length)
+ return handleErrors(raw_length.takeError(),
+ [&](const TransportEOFError &E) -> llvm::Error {
+ return createStringError(
+ "unexpected EOF while reading header separator");
+ });
+
+ size_t length;
+ if (!to_integer(*raw_length, length))
+ return createStringError(
+ formatv("invalid content length {0}", *raw_length).str());
+
+ Expected<std::string> raw_json = ReadFull(*input, length);
+ if (!raw_json)
+ return handleErrors(
+ raw_json.takeError(), [&](const TransportEOFError &E) -> llvm::Error {
+ return createStringError("unexpected EOF while reading JSON");
+ });
+
+ Log(llvm::formatv("--> ({0}) {1}", m_client_name, *raw_json).str());
+
+ return raw_json;
+}
+
+Error JSONWithHeaderTransport::WriteImpl(const std::string &message) {
+ if (!m_output || !m_output->IsValid())
+ return llvm::make_error<TransportClosedError>();
+
+ Log(llvm::formatv("<-- ({0}) {1}", m_client_name, message).str());
+
+ std::string Output;
+ raw_string_ostream OS(Output);
+ OS << kHeaderContentLength << message.length() << kHeaderSeparator << message;
+ size_t num_bytes = Output.size();
+ return m_output->Write(Output.data(), num_bytes).takeError();
+}
+
+char TransportEOFError::ID;
+char TransportTimeoutError::ID;
+char TransportClosedError::ID;
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index b034c967594ba..9fe8227cd2d6f 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -70,6 +70,7 @@
using namespace lldb_dap;
using namespace lldb_dap::protocol;
+using namespace lldb_private;
namespace {
#ifdef _WIN32
@@ -893,14 +894,14 @@ llvm::Error DAP::Loop() {
while (!disconnecting) {
llvm::Expected<Message> next =
- transport.Read(std::chrono::seconds(1));
- if (next.errorIsA<EndOfFileError>()) {
+ transport.Read<protocol::Message>(std::chrono::seconds(1));
+ if (next.errorIsA<TransportEOFError>()) {
consumeError(next.takeError());
break;
}
// If the read timed out, continue to check if we should disconnect.
- if (next.errorIsA<TimeoutError>()) {
+ if (next.errorIsA<TransportTimeoutError>()) {
consumeError(next.takeError());
continue;
}
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
index 4e322e9ff1358..90fc895badc25 100644
--- a/lldb/tools/lldb-dap/Transport.cpp
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -8,152 +8,16 @@
#include "Transport.h"
#include "DAPLog.h"
-#include "Protocol/ProtocolBase.h"
-#include "lldb/Utility/IOObject.h"
-#include "lldb/Utility/SelectHelper.h"
-#include "lldb/Utility/Status.h"
#include "lldb/lldb-forward.h"
-#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/Error.h"
-#include "llvm/Support/raw_ostream.h"
-#include <optional>
-#include <string>
-#include <utility>
using namespace llvm;
using namespace lldb;
using namespace lldb_private;
using namespace lldb_dap;
-using namespace lldb_dap::protocol;
-/// ReadFull attempts to read the specified number of bytes. If EOF is
-/// encountered, an empty string is returned.
-static Expected<std::string>
-ReadFull(IOObject &descriptor, size_t length,
- std::optional<std::chrono::microseconds> timeout = std::nullopt) {
- if (!descriptor.IsValid())
- return createStringError("transport output is closed");
+Transport::Transport(llvm::StringRef client_name, lldb_dap::Log *log,
+ lldb::IOObjectSP input, lldb::IOObjectSP output)
+ : JSONWithHeaderTransport(client_name, input, output), m_log(log) {}
- bool timeout_supported = true;
- // FIXME: SelectHelper does not work with NativeFile on Win32.
-#if _WIN32
- timeout_supported = descriptor.GetFdType() == IOObject::eFDTypeSocket;
-#endif
-
- if (timeout && timeout_supported) {
- SelectHelper sh;
- sh.SetTimeout(*timeout);
- sh.FDSetRead(descriptor.GetWaitableHandle());
- Status status = sh.Select();
- if (status.Fail()) {
- // Convert timeouts into a specific error.
- if (status.GetType() == lldb::eErrorTypePOSIX &&
- status.GetError() == ETIMEDOUT)
- return make_error<TimeoutError>();
- return status.takeError();
- }
- }
-
- std::string data;
- data.resize(length);
- Status status = descriptor.Read(data.data(), length);
- if (status.Fail())
- return status.takeError();
-
- // Read returns '' on EOF.
- if (length == 0)
- return make_error<EndOfFileError>();
-
- // Return the actual number of bytes read.
- return data.substr(0, length);
-}
-
-static Expected<std::string>
-ReadUntil(IOObject &descriptor, StringRef delimiter,
- std::optional<std::chrono::microseconds> timeout = std::nullopt) {
- std::string buffer;
- buffer.reserve(delimiter.size() + 1);
- while (!llvm::StringRef(buffer).ends_with(delimiter)) {
- Expected<std::string> next =
- ReadFull(descriptor, buffer.empty() ? delimiter.size() : 1, timeout);
- if (auto Err = next.takeError())
- return std::move(Err);
- buffer += *next;
- }
- return buffer.substr(0, buffer.size() - delimiter.size());
-}
-
-/// DAP message format
-/// ```
-/// Content-Length: (?<length>\d+)\r\n\r\n(?<content>.{\k<length>})
-/// ```
-static constexpr StringLiteral kHeaderContentLength = "Content-Length: ";
-static constexpr StringLiteral kHeaderSeparator = "\r\n\r\n";
-
-namespace lldb_dap {
-
-char EndOfFileError::ID;
-char TimeoutError::ID;
-
-Transport::Transport(StringRef client_name, Log *log, IOObjectSP input,
- IOObjectSP output)
- : m_client_name(client_name), m_log(log), m_input(std::move(input)),
- m_output(std::move(output)) {}
-
-Expected<Message> Transport::Read(const std::chrono::microseconds &timeout) {
- if (!m_input || !m_input->IsValid())
- return createStringError("transport output is closed");
-
- IOObject *input = m_input.get();
- Expected<std::string> message_header =
- ReadFull(*input, kHeaderContentLength.size(), timeout);
- if (!message_header)
- return message_header.takeError();
- if (*message_header != kHeaderContentLength)
- return createStringError(formatv("expected '{0}' and got '{1}'",
- kHeaderContentLength, *message_header)
- .str());
-
- Expected<std::string> raw_length = ReadUntil(*input, kHeaderSeparator);
- if (!raw_length)
- return handleErrors(raw_length.takeError(),
- [&](const EndOfFileError &E) -> llvm::Error {
- return createStringError(
- "unexpected EOF while reading header separator");
- });
-
- size_t length;
- if (!to_integer(*raw_length, length))
- return createStringError(
- formatv("invalid content length {0}", *raw_length).str());
-
- Expected<std::string> raw_json = ReadFull(*input, length);
- if (!raw_json)
- return handleErrors(
- raw_json.takeError(), [&](const EndOfFileError &E) -> llvm::Error {
- return createStringError("unexpected EOF while reading JSON");
- });
-
- DAP_LOG(m_log, "--> ({0}) {1}", m_client_name, *raw_json);
-
- return json::parse<Message>(/*JSON=*/*raw_json,
- /*RootName=*/"protocol_message");
-}
-
-Error Transport::Write(const Message &message) {
- if (!m_output || !m_output->IsValid())
- return createStringError("transport output is closed");
-
- std::string json = formatv("{0}", toJSON(message)).str();
-
- DAP_LOG(m_log, "<-- ({0}) {1}", m_client_name, json);
-
- std::string Output;
- raw_string_ostream OS(Output);
- OS << kHeaderContentLength << json.length() << kHeaderSeparator << json;
- size_t num_bytes = Output.size();
- return m_output->Write(Output.data(), num_bytes).takeError();
-}
-
-} // end namespace lldb_dap
+void Transport::Log(llvm::StringRef message) { DAP_LOG(m_log, "{0}", message); }
diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h
index 4e347eaa51314..0b083dd90dcd0 100644
--- a/lldb/tools/lldb-dap/Transport.h
+++ b/lldb/tools/lldb-dap/Transport.h
@@ -15,80 +15,24 @@
#define LLDB_TOOLS_LLDB_DAP_TRANSPORT_H
#include "DAPForward.h"
-#include "Protocol/ProtocolBase.h"
+#include "lldb/Host/JSONTransport.h"
#include "lldb/lldb-forward.h"
#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/Error.h"
-#include <chrono>
-#include <system_error>
namespace lldb_dap {
-class EndOfFileError : public llvm::ErrorInfo<EndOfFileError> {
-public:
- static char ID;
-
- EndOfFileError() = default;
-
- void log(llvm::raw_ostream &OS) const override {
- OS << "end of file reached";
- }
- std::error_code convertToErrorCode() const override {
- return llvm::inconvertibleErrorCode();
- }
-};
-
-class TimeoutError : public llvm::ErrorInfo<TimeoutError> {
-public:
- static char ID;
-
- TimeoutError() = default;
-
- void log(llvm::raw_ostream &OS) const override {
- OS << "operation timed out";
- }
- std::error_code convertToErrorCode() const override {
- return std::make_error_code(std::errc::timed_out);
- }
-};
-
/// A transport class that performs the Debug Adapter Protocol communication
/// with the client.
-class Transport {
+class Transport : public lldb_private::JSONWithHeaderTransport {
public:
- Transport(llvm::StringRef client_name, Log *log, lldb::IOObjectSP input,
- lldb::IOObjectSP output);
- ~Transport() = default;
-
- /// Transport is not copyable.
- /// @{
- Transport(const Transport &rhs) = delete;
- void operator=(const Transport &rhs) = delete;
- /// @}
-
- /// Writes a Debug Adater Protocol message to the output stream.
- llvm::Error Write(const protocol::Message &M);
-
- /// Reads the next Debug Adater Protocol message from the input stream.
- ///
- /// \param timeout[in]
- /// A timeout to wait for reading the initial header. Once a message
- /// header is recieved, this will block until the full message is
- /// read.
- ///
- /// \returns Returns the next protocol message.
- llvm::Expected<protocol::Message>
- Read(const std::chrono::microseconds &timeout);
+ Transport(llvm::StringRef client_name, lldb_dap::Log *log,
+ lldb::IOObjectSP input, lldb::IOObjectSP output);
+ virtual ~Transport() = default;
- /// Returns the name of this transport client, for example `stdin/stdout` or
- /// `client_1`.
- llvm::StringRef GetClientName() { return m_client_name; }
+ virtual void Log(llvm::StringRef message) override;
private:
- llvm::StringRef m_client_name;
- Log *m_log;
- lldb::IOObjectSP m_input;
- lldb::IOObjectSP m_output;
+ lldb_dap::Log *m_log;
};
} // namespace lldb_dap
diff --git a/lldb/unittests/DAP/DAPTest.cpp b/lldb/unittests/DAP/DAPTest.cpp
index 5fb6bf7e564ab..40ffaf87c9c45 100644
--- a/lldb/unittests/DAP/DAPTest.cpp
+++ b/lldb/unittests/DAP/DAPTest.cpp
@@ -32,7 +32,8 @@ TEST_F(DAPTest, SendProtocolMessages) {
/*transport=*/*to_dap,
};
dap.Send(Event{/*event=*/"my-event", /*body=*/std::nullopt});
- ASSERT_THAT_EXPECTED(from_dap->Read(std::chrono::milliseconds(1)),
- HasValue(testing::VariantWith<Event>(testing::FieldsAre(
- /*event=*/"my-event", /*body=*/std::nullopt))));
+ ASSERT_THAT_EXPECTED(
+ from_dap->Read<protocol::Message>(std::chrono::milliseconds(1)),
+ HasValue(testing::VariantWith<Event>(testing::FieldsAre(
+ /*event=*/"my-event", /*body=*/std::nullopt))));
}
diff --git a/lldb/unittests/DAP/TestBase.cpp b/lldb/unittests/DAP/TestBase.cpp
index 388d1b901507e..4063b34250312 100644
--- a/lldb/unittests/DAP/TestBase.cpp
+++ b/lldb/unittests/DAP/TestBase.cpp
@@ -122,7 +122,8 @@ std::vector<Message> DAPTestBase::DrainOutput() {
std::vector<Message> msgs;
output.CloseWriteFileDescriptor();
while (true) {
- Expected<Message> next = from_dap->Read(std::chrono::milliseconds(1));
+ Expected<Message> next =
+ from_dap->Read<protocol::Message>(std::chrono::milliseconds(1));
if (!next) {
consumeError(next.takeError());
break;
diff --git a/lldb/unittests/DAP/TransportTest.cpp b/lldb/unittests/DAP/TransportTest.cpp
index e6dab42e30941..aaf257993af23 100644
--- a/lldb/unittests/DAP/TransportTest.cpp
+++ b/lldb/unittests/DAP/TransportTest.cpp
@@ -26,6 +26,8 @@ using namespace lldb_dap::protocol;
using lldb_private::File;
using lldb_private::NativeFile;
using lldb_private::Pipe;
+using lldb_private::TransportEOFError;
+using lldb_private::TransportTimeoutError;
class TransportTest : public PipeBase {
protected:
@@ -50,7 +52,7 @@ TEST_F(TransportTest, MalformedRequests) {
input.Write(malformed_header.data(), malformed_header.size()),
Succeeded());
ASSERT_THAT_EXPECTED(
- transport->Read(std::chrono::milliseconds(1)),
+ transport->Read<protocol::Message>(std::chrono::milliseconds(1)),
FailedWithMessage(
"expected 'Content-Length: ' and got 'COnTent-LenGth: '"));
}
@@ -63,20 +65,22 @@ TEST_F(TransportTest, Read) {
ASSERT_THAT_EXPECTED(input.Write(message.data(), message.size()),
Succeeded());
ASSERT_THAT_EXPECTED(
- transport->Read(std::chrono::milliseconds(1)),
+ transport->Read<protocol::Message>(std::chrono::milliseconds(1)),
HasValue(testing::VariantWith<Request>(testing::FieldsAre(
/*seq=*/1, /*command=*/"abc", /*arguments=*/std::nullopt))));
}
TEST_F(TransportTest, ReadWithTimeout) {
- ASSERT_THAT_EXPECTED(transport->Read(std::chrono::milliseconds(1)),
- Failed<TimeoutError>());
+ ASSERT_THAT_EXPECTED(
+ transport->Read<protocol::Message>(std::chrono::milliseconds(1)),
+ Failed<TransportTimeoutError>());
}
TEST_F(TransportTest, ReadWithEOF) {
input.CloseWriteFileDescriptor();
- ASSERT_THAT_EXPECTED(transport->Read(std::chrono::milliseconds(1)),
- Failed<EndOfFileError>());
+ ASSERT_THAT_EXPECTED(
+ transport->Read<protocol::Message>(std::chrono::milliseconds(1)),
+ Failed<TransportEOFError>());
}
TEST_F(TransportTest, Write) {
>From ef1dbae85322c4595da10992bf90165726645286 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Wed, 11 Jun 2025 18:08:20 -0700
Subject: [PATCH 2/2] Address John's feedback
- Move client name out of the base class
- Use HTTPDelimitedJSONTransport
- Don't set the root name
---
lldb/include/lldb/Host/JSONTransport.h | 25 +++++++++--------------
lldb/source/Host/common/JSONTransport.cpp | 14 ++++++-------
lldb/tools/lldb-dap/Transport.cpp | 7 +++++--
lldb/tools/lldb-dap/Transport.h | 7 ++++++-
4 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/lldb/include/lldb/Host/JSONTransport.h b/lldb/include/lldb/Host/JSONTransport.h
index 907351abf8f74..4db5e417ea852 100644
--- a/lldb/include/lldb/Host/JSONTransport.h
+++ b/lldb/include/lldb/Host/JSONTransport.h
@@ -68,8 +68,7 @@ class TransportClosedError : public llvm::ErrorInfo<TransportClosedError> {
/// A transport class that uses JSON for communication.
class JSONTransport {
public:
- JSONTransport(llvm::StringRef client_name, lldb::IOObjectSP input,
- lldb::IOObjectSP output);
+ JSONTransport(lldb::IOObjectSP input, lldb::IOObjectSP output);
virtual ~JSONTransport() = default;
/// Transport is not copyable.
@@ -90,37 +89,33 @@ class JSONTransport {
llvm::Expected<std::string> message = ReadImpl(timeout);
if (!message)
return message.takeError();
- return llvm::json::parse<T>(/*JSON=*/*message,
- /*RootName=*/"transport_message");
+ return llvm::json::parse<T>(/*JSON=*/*message);
}
- /// Returns the name of this transport client, for example `stdin/stdout` or
- /// `client_1`.
- llvm::StringRef GetClientName() { return m_client_name; }
-
protected:
virtual void Log(llvm::StringRef message);
+
virtual llvm::Error WriteImpl(const std::string &message) = 0;
virtual llvm::Expected<std::string>
ReadImpl(const std::chrono::microseconds &timeout) = 0;
- llvm::StringRef m_client_name;
lldb::IOObjectSP m_input;
lldb::IOObjectSP m_output;
};
-/// A transport class that uses JSON with a header for communication.
-class JSONWithHeaderTransport : public JSONTransport {
+/// A transport class for JSON with a HTTP header.
+class HTTPDelimitedJSONTransport : public JSONTransport {
public:
- JSONWithHeaderTransport(llvm::StringRef client_name, lldb::IOObjectSP input,
- lldb::IOObjectSP output)
- : JSONTransport(client_name, input, output) {}
- virtual ~JSONWithHeaderTransport() = default;
+ HTTPDelimitedJSONTransport(lldb::IOObjectSP input, lldb::IOObjectSP output)
+ : JSONTransport(input, output) {}
+ virtual ~HTTPDelimitedJSONTransport() = default;
+protected:
virtual llvm::Error WriteImpl(const std::string &message) override;
virtual llvm::Expected<std::string>
ReadImpl(const std::chrono::microseconds &timeout) override;
+ // FIXME: Support any header.
static constexpr llvm::StringLiteral kHeaderContentLength =
"Content-Length: ";
static constexpr llvm::StringLiteral kHeaderSeparator = "\r\n\r\n";
diff --git a/lldb/source/Host/common/JSONTransport.cpp b/lldb/source/Host/common/JSONTransport.cpp
index 32d1889f69c02..103c76d25daf7 100644
--- a/lldb/source/Host/common/JSONTransport.cpp
+++ b/lldb/source/Host/common/JSONTransport.cpp
@@ -82,17 +82,15 @@ ReadUntil(IOObject &descriptor, StringRef delimiter,
return buffer.substr(0, buffer.size() - delimiter.size());
}
-JSONTransport::JSONTransport(StringRef client_name, IOObjectSP input,
- IOObjectSP output)
- : m_client_name(client_name), m_input(std::move(input)),
- m_output(std::move(output)) {}
+JSONTransport::JSONTransport(IOObjectSP input, IOObjectSP output)
+ : m_input(std::move(input)), m_output(std::move(output)) {}
void JSONTransport::Log(llvm::StringRef message) {
LLDB_LOG(GetLog(LLDBLog::Host), "{0}", message);
}
Expected<std::string>
-JSONWithHeaderTransport::ReadImpl(const std::chrono::microseconds &timeout) {
+HTTPDelimitedJSONTransport::ReadImpl(const std::chrono::microseconds &timeout) {
if (!m_input || !m_input->IsValid())
return createStringError("transport output is closed");
@@ -126,16 +124,16 @@ JSONWithHeaderTransport::ReadImpl(const std::chrono::microseconds &timeout) {
return createStringError("unexpected EOF while reading JSON");
});
- Log(llvm::formatv("--> ({0}) {1}", m_client_name, *raw_json).str());
+ Log(llvm::formatv("--> {0}", *raw_json).str());
return raw_json;
}
-Error JSONWithHeaderTransport::WriteImpl(const std::string &message) {
+Error HTTPDelimitedJSONTransport::WriteImpl(const std::string &message) {
if (!m_output || !m_output->IsValid())
return llvm::make_error<TransportClosedError>();
- Log(llvm::formatv("<-- ({0}) {1}", m_client_name, message).str());
+ Log(llvm::formatv("<-- {0}", message).str());
std::string Output;
raw_string_ostream OS(Output);
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
index 90fc895badc25..d602920da34e3 100644
--- a/lldb/tools/lldb-dap/Transport.cpp
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -18,6 +18,9 @@ using namespace lldb_dap;
Transport::Transport(llvm::StringRef client_name, lldb_dap::Log *log,
lldb::IOObjectSP input, lldb::IOObjectSP output)
- : JSONWithHeaderTransport(client_name, input, output), m_log(log) {}
+ : HTTPDelimitedJSONTransport(input, output), m_client_name(client_name),
+ m_log(log) {}
-void Transport::Log(llvm::StringRef message) { DAP_LOG(m_log, "{0}", message); }
+void Transport::Log(llvm::StringRef message) {
+ DAP_LOG(m_log, "({0}) {1}", m_client_name, message);
+}
diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h
index 0b083dd90dcd0..51f62e718a0d0 100644
--- a/lldb/tools/lldb-dap/Transport.h
+++ b/lldb/tools/lldb-dap/Transport.h
@@ -23,7 +23,7 @@ namespace lldb_dap {
/// A transport class that performs the Debug Adapter Protocol communication
/// with the client.
-class Transport : public lldb_private::JSONWithHeaderTransport {
+class Transport : public lldb_private::HTTPDelimitedJSONTransport {
public:
Transport(llvm::StringRef client_name, lldb_dap::Log *log,
lldb::IOObjectSP input, lldb::IOObjectSP output);
@@ -31,7 +31,12 @@ class Transport : public lldb_private::JSONWithHeaderTransport {
virtual void Log(llvm::StringRef message) override;
+ /// Returns the name of this transport client, for example `stdin/stdout` or
+ /// `client_1`.
+ llvm::StringRef GetClientName() { return m_client_name; }
+
private:
+ llvm::StringRef m_client_name;
lldb_dap::Log *m_log;
};
More information about the lldb-commits
mailing list