[lldb] Adjusting the base MCP protocol types per the spec. #153297
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
…he spec. * This adjusts the Request/Response types to have an 'id' that is either a string or a number. * Merges 'Error' into 'Response' to have a single reponse type that represents both errors and results. * Adjusts the Error.data field to by any JSON value. * Adds `operator==` support to the base protocol types and simplifies the tests.
|
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) Changes
Full diff: https://github.com/llvm/llvm-project/pull/153297.diff 8 Files Affected:
diff --git a/lldb/include/lldb/Protocol/MCP/MCPError.h b/lldb/include/lldb/Protocol/MCP/MCPError.h
index 2bdbb9b7a6874..55dd40f124a15 100644
--- a/lldb/include/lldb/Protocol/MCP/MCPError.h
+++ b/lldb/include/lldb/Protocol/MCP/MCPError.h
@@ -26,7 +26,7 @@ class MCPError : public llvm::ErrorInfo<MCPError> {
const std::string &getMessage() const { return m_message; }
- lldb_protocol::mcp::Error toProtcolError() const;
+ lldb_protocol::mcp::Error toProtocolError() const;
static constexpr int64_t kResourceNotFound = -32002;
static constexpr int64_t kInternalError = -32603;
diff --git a/lldb/include/lldb/Protocol/MCP/Protocol.h b/lldb/include/lldb/Protocol/MCP/Protocol.h
index 6448416eee08f..46edf67c723f9 100644
--- a/lldb/include/lldb/Protocol/MCP/Protocol.h
+++ b/lldb/include/lldb/Protocol/MCP/Protocol.h
@@ -23,50 +23,72 @@ namespace lldb_protocol::mcp {
static llvm::StringLiteral kProtocolVersion = "2024-11-05";
+/// A Request or Response 'id'.
+///
+/// NOTE: This differs from the JSONRPC-2.0 spec. The MCP spec says this must be
+/// a string or number, excluding a json 'null' as a valid id.
+using Id = std::variant<int64_t, std::string>;
+
/// A request that expects a response.
struct Request {
- uint64_t id = 0;
+ /// The request id.
+ Id id = 0;
+ /// The method to be invoked.
std::string method;
+ /// The method's params.
std::optional<llvm::json::Value> params;
};
llvm::json::Value toJSON(const Request &);
bool fromJSON(const llvm::json::Value &, Request &, llvm::json::Path);
+bool operator==(const Request &, const Request &);
-struct ErrorInfo {
+struct Error {
+ /// The error type that occurred.
int64_t code = 0;
+ /// A short description of the error. The message SHOULD be limited to a
+ /// concise single sentence.
std::string message;
- std::string data;
-};
-
-llvm::json::Value toJSON(const ErrorInfo &);
-bool fromJSON(const llvm::json::Value &, ErrorInfo &, llvm::json::Path);
-
-struct Error {
- uint64_t id = 0;
- ErrorInfo error;
+ /// Additional information about the error. The value of this member is
+ /// defined by the sender (e.g. detailed error information, nested errors
+ /// etc.).
+ std::optional<llvm::json::Value> data;
};
llvm::json::Value toJSON(const Error &);
bool fromJSON(const llvm::json::Value &, Error &, llvm::json::Path);
+bool operator==(const Error &, const Error &);
+/// A response to a request, either an error or a result.
struct Response {
- uint64_t id = 0;
- std::optional<llvm::json::Value> result;
- std::optional<ErrorInfo> error;
+ /// The request id.
+ Id id = 0;
+ /// The result of the request, either an Error or the JSON value of the
+ /// response.
+ std::variant<Error, llvm::json::Value> result;
};
llvm::json::Value toJSON(const Response &);
bool fromJSON(const llvm::json::Value &, Response &, llvm::json::Path);
+bool operator==(const Response &, const Response &);
/// A notification which does not expect a response.
struct Notification {
+ /// The method to be invoked.
std::string method;
+ /// The notification's params.
std::optional<llvm::json::Value> params;
};
llvm::json::Value toJSON(const Notification &);
bool fromJSON(const llvm::json::Value &, Notification &, llvm::json::Path);
+bool operator==(const Notification &, const Notification &);
+
+/// A general message as defined by the JSON-RPC 2.0 spec.
+using Message = std::variant<Request, Response, Notification>;
+
+bool fromJSON(const llvm::json::Value &, Message &, llvm::json::Path);
+llvm::json::Value toJSON(const Message &);
struct ToolCapability {
/// Whether this server supports notifications for changes to the tool list.
@@ -176,11 +198,6 @@ struct ToolDefinition {
llvm::json::Value toJSON(const ToolDefinition &);
bool fromJSON(const llvm::json::Value &, ToolDefinition &, llvm::json::Path);
-using Message = std::variant<Request, Response, Notification, Error>;
-
-bool fromJSON(const llvm::json::Value &, Message &, llvm::json::Path);
-llvm::json::Value toJSON(const Message &);
-
using ToolArguments = std::variant<std::monostate, llvm::json::Value>;
} // namespace lldb_protocol::mcp
diff --git a/lldb/source/Protocol/MCP/MCPError.cpp b/lldb/source/Protocol/MCP/MCPError.cpp
index c610e882abf51..e140d11e12cfe 100644
--- a/lldb/source/Protocol/MCP/MCPError.cpp
+++ b/lldb/source/Protocol/MCP/MCPError.cpp
@@ -25,10 +25,10 @@ std::error_code MCPError::convertToErrorCode() const {
return llvm::inconvertibleErrorCode();
}
-lldb_protocol::mcp::Error MCPError::toProtcolError() const {
+lldb_protocol::mcp::Error MCPError::toProtocolError() const {
lldb_protocol::mcp::Error error;
- error.error.code = m_error_code;
- error.error.message = m_message;
+ error.code = m_error_code;
+ error.message = m_message;
return error;
}
diff --git a/lldb/source/Protocol/MCP/Protocol.cpp b/lldb/source/Protocol/MCP/Protocol.cpp
index d579b88037e63..d9b11bd766686 100644
--- a/lldb/source/Protocol/MCP/Protocol.cpp
+++ b/lldb/source/Protocol/MCP/Protocol.cpp
@@ -26,8 +26,45 @@ static bool mapRaw(const json::Value &Params, StringLiteral Prop,
return true;
}
+static llvm::json::Value toJSON(const Id &Id) {
+ if (const int64_t *I = std::get_if<int64_t>(&Id))
+ return json::Value(*I);
+ if (const std::string *S = std::get_if<std::string>(&Id))
+ return json::Value(*S);
+ llvm_unreachable("unexpected type in protocol::Id");
+}
+
+static bool mapId(const llvm::json::Value &V, StringLiteral Prop, Id &Id,
+ llvm::json::Path P) {
+ const auto *O = V.getAsObject();
+ if (!O) {
+ P.report("expected object");
+ return false;
+ }
+
+ const auto *E = O->get(Prop);
+ if (!E) {
+ P.field(Prop).report("not found");
+ return false;
+ }
+
+ if (auto S = E->getAsString()) {
+ Id = S->str();
+ return true;
+ }
+
+ if (auto I = E->getAsInteger()) {
+ Id = *I;
+ return true;
+ }
+
+ P.report("expected string or number");
+ return false;
+}
+
llvm::json::Value toJSON(const Request &R) {
- json::Object Result{{"jsonrpc", "2.0"}, {"id", R.id}, {"method", R.method}};
+ json::Object Result{
+ {"jsonrpc", "2.0"}, {"id", toJSON(R.id)}, {"method", R.method}};
if (R.params)
Result.insert({"params", R.params});
return Result;
@@ -35,47 +72,75 @@ llvm::json::Value toJSON(const Request &R) {
bool fromJSON(const llvm::json::Value &V, Request &R, llvm::json::Path P) {
llvm::json::ObjectMapper O(V, P);
- if (!O || !O.map("id", R.id) || !O.map("method", R.method))
- return false;
- return mapRaw(V, "params", R.params, P);
-}
-
-llvm::json::Value toJSON(const ErrorInfo &EI) {
- llvm::json::Object Result{{"code", EI.code}, {"message", EI.message}};
- if (!EI.data.empty())
- Result.insert({"data", EI.data});
- return Result;
+ return O && mapId(V, "id", R.id, P) && O.map("method", R.method) &&
+ mapRaw(V, "params", R.params, P);
}
-bool fromJSON(const llvm::json::Value &V, ErrorInfo &EI, llvm::json::Path P) {
- llvm::json::ObjectMapper O(V, P);
- return O && O.map("code", EI.code) && O.map("message", EI.message) &&
- O.mapOptional("data", EI.data);
+bool operator==(const Request &a, const Request &b) {
+ return a.id == b.id && a.method == b.method && a.params == b.params;
}
llvm::json::Value toJSON(const Error &E) {
- return json::Object{{"jsonrpc", "2.0"}, {"id", E.id}, {"error", E.error}};
+ llvm::json::Object Result{{"code", E.code}, {"message", E.message}};
+ if (E.data)
+ Result.insert({"data", *E.data});
+ return Result;
}
bool fromJSON(const llvm::json::Value &V, Error &E, llvm::json::Path P) {
llvm::json::ObjectMapper O(V, P);
- return O && O.map("id", E.id) && O.map("error", E.error);
+ return O && O.map("code", E.code) && O.map("message", E.message) &&
+ mapRaw(V, "data", E.data, P);
+}
+
+bool operator==(const Error &a, const Error &b) {
+ return a.code == b.code && a.message == b.message && a.data == b.data;
}
llvm::json::Value toJSON(const Response &R) {
- llvm::json::Object Result{{"jsonrpc", "2.0"}, {"id", R.id}};
- if (R.result)
- Result.insert({"result", R.result});
- if (R.error)
- Result.insert({"error", R.error});
+ llvm::json::Object Result{{"jsonrpc", "2.0"}, {"id", toJSON(R.id)}};
+
+ if (const Error *error = std::get_if<Error>(&R.result))
+ Result.insert({"error", *error});
+ if (const json::Value *result = std::get_if<json::Value>(&R.result))
+ Result.insert({"result", *result});
return Result;
}
bool fromJSON(const llvm::json::Value &V, Response &R, llvm::json::Path P) {
- llvm::json::ObjectMapper O(V, P);
- if (!O || !O.map("id", R.id) || !O.map("error", R.error))
+ const json::Object *E = V.getAsObject();
+ if (!E) {
+ P.report("expected object");
+ return false;
+ }
+
+ const json::Value *result = E->get("result");
+ const json::Value *raw_error = E->get("error");
+
+ if (result && raw_error) {
+ P.report("'result' and 'error' fields are mutually exclusive");
return false;
- return mapRaw(V, "result", R.result, P);
+ }
+
+ if (!result && !raw_error) {
+ P.report("'result' or 'error' fields are required'");
+ return false;
+ }
+
+ if (result) {
+ R.result = std::move(*result);
+ } else {
+ Error error;
+ if (!fromJSON(*raw_error, error, P))
+ return false;
+ R.result = std::move(error);
+ }
+
+ return mapId(V, "id", R.id, P);
+}
+
+bool operator==(const Response &a, const Response &b) {
+ return a.id == b.id && a.result == b.result;
}
llvm::json::Value toJSON(const Notification &N) {
@@ -97,6 +162,10 @@ bool fromJSON(const llvm::json::Value &V, Notification &N, llvm::json::Path P) {
return true;
}
+bool operator==(const Notification &a, const Notification &b) {
+ return a.method == b.method && a.params == b.params;
+}
+
llvm::json::Value toJSON(const ToolCapability &TC) {
return llvm::json::Object{{"listChanged", TC.listChanged}};
}
@@ -235,24 +304,16 @@ bool fromJSON(const llvm::json::Value &V, Message &M, llvm::json::Path P) {
return true;
}
- if (O->get("error")) {
- Error E;
- if (!fromJSON(V, E, P))
- return false;
- M = std::move(E);
- return true;
- }
-
- if (O->get("result")) {
- Response R;
+ if (O->get("method")) {
+ Request R;
if (!fromJSON(V, R, P))
return false;
M = std::move(R);
return true;
}
- if (O->get("method")) {
- Request R;
+ if (O->get("result") || O->get("error")) {
+ Response R;
if (!fromJSON(V, R, P))
return false;
M = std::move(R);
diff --git a/lldb/source/Protocol/MCP/Server.cpp b/lldb/source/Protocol/MCP/Server.cpp
index 4ec127fe75bdd..a9c1482e3e378 100644
--- a/lldb/source/Protocol/MCP/Server.cpp
+++ b/lldb/source/Protocol/MCP/Server.cpp
@@ -66,13 +66,15 @@ Server::HandleData(llvm::StringRef data) {
Error protocol_error;
llvm::handleAllErrors(
response.takeError(),
- [&](const MCPError &err) { protocol_error = err.toProtcolError(); },
+ [&](const MCPError &err) { protocol_error = err.toProtocolError(); },
[&](const llvm::ErrorInfoBase &err) {
- protocol_error.error.code = MCPError::kInternalError;
- protocol_error.error.message = err.message();
+ protocol_error.code = MCPError::kInternalError;
+ protocol_error.message = err.message();
});
- protocol_error.id = request->id;
- return protocol_error;
+ Response error_response;
+ error_response.id = request->id;
+ error_response.result = std::move(protocol_error);
+ return error_response;
}
return *response;
@@ -84,9 +86,6 @@ Server::HandleData(llvm::StringRef data) {
return std::nullopt;
}
- if (std::get_if<Error>(&(*message)))
- return llvm::createStringError("unexpected MCP message: error");
-
if (std::get_if<Response>(&(*message)))
return llvm::createStringError("unexpected MCP message: response");
@@ -123,11 +122,11 @@ void Server::AddNotificationHandler(llvm::StringRef method,
llvm::Expected<Response> Server::InitializeHandler(const Request &request) {
Response response;
- response.result.emplace(llvm::json::Object{
+ response.result = llvm::json::Object{
{"protocolVersion", mcp::kProtocolVersion},
{"capabilities", GetCapabilities()},
{"serverInfo",
- llvm::json::Object{{"name", m_name}, {"version", m_version}}}});
+ llvm::json::Object{{"name", m_name}, {"version", m_version}}}};
return response;
}
@@ -138,7 +137,7 @@ llvm::Expected<Response> Server::ToolsListHandler(const Request &request) {
for (const auto &tool : m_tools)
tools.emplace_back(toJSON(tool.second->GetDefinition()));
- response.result.emplace(llvm::json::Object{{"tools", std::move(tools)}});
+ response.result = llvm::json::Object{{"tools", std::move(tools)}};
return response;
}
@@ -173,7 +172,7 @@ llvm::Expected<Response> Server::ToolsCallHandler(const Request &request) {
if (!text_result)
return text_result.takeError();
- response.result.emplace(toJSON(*text_result));
+ response.result = toJSON(*text_result);
return response;
}
@@ -189,8 +188,7 @@ llvm::Expected<Response> Server::ResourcesListHandler(const Request &request) {
for (const Resource &resource : resource_provider_up->GetResources())
resources.push_back(resource);
}
- response.result.emplace(
- llvm::json::Object{{"resources", std::move(resources)}});
+ response.result = llvm::json::Object{{"resources", std::move(resources)}};
return response;
}
@@ -226,7 +224,7 @@ llvm::Expected<Response> Server::ResourcesReadHandler(const Request &request) {
return result.takeError();
Response response;
- response.result.emplace(std::move(*result));
+ response.result = std::move(*result);
return response;
}
diff --git a/lldb/unittests/Protocol/ProtocolMCPTest.cpp b/lldb/unittests/Protocol/ProtocolMCPTest.cpp
index 1dca0e5fc5bb8..a81cd65db8cca 100644
--- a/lldb/unittests/Protocol/ProtocolMCPTest.cpp
+++ b/lldb/unittests/Protocol/ProtocolMCPTest.cpp
@@ -149,9 +149,7 @@ TEST(ProtocolMCPTest, MessageWithRequest) {
const Request &deserialized_request =
std::get<Request>(*deserialized_message);
- EXPECT_EQ(request.id, deserialized_request.id);
- EXPECT_EQ(request.method, deserialized_request.method);
- EXPECT_EQ(request.params, deserialized_request.params);
+ EXPECT_EQ(request, deserialized_request);
}
TEST(ProtocolMCPTest, MessageWithResponse) {
@@ -168,8 +166,7 @@ TEST(ProtocolMCPTest, MessageWithResponse) {
const Response &deserialized_response =
std::get<Response>(*deserialized_message);
- EXPECT_EQ(response.id, deserialized_response.id);
- EXPECT_EQ(response.result, deserialized_response.result);
+ EXPECT_EQ(response, deserialized_response);
}
TEST(ProtocolMCPTest, MessageWithNotification) {
@@ -186,49 +183,29 @@ TEST(ProtocolMCPTest, MessageWithNotification) {
const Notification &deserialized_notification =
std::get<Notification>(*deserialized_message);
- EXPECT_EQ(notification.method, deserialized_notification.method);
- EXPECT_EQ(notification.params, deserialized_notification.params);
+ EXPECT_EQ(notification, deserialized_notification);
}
-TEST(ProtocolMCPTest, MessageWithError) {
- ErrorInfo error_info;
- error_info.code = -32603;
- error_info.message = "Internal error";
-
+TEST(ProtocolMCPTest, MessageWithErrorResponse) {
Error error;
- error.id = 3;
- error.error = error_info;
+ error.code = -32603;
+ error.message = "Internal error";
+
+ Response error_response;
+ error_response.id = 3;
+ error_response.result = error;
- Message message = error;
+ Message message = error_response;
llvm::Expected<Message> deserialized_message = roundtripJSON(message);
ASSERT_THAT_EXPECTED(deserialized_message, llvm::Succeeded());
- ASSERT_TRUE(std::holds_alternative<Error>(*deserialized_message));
- const Error &deserialized_error = std::get<Error>(*deserialized_message);
-
- EXPECT_EQ(error.id, deserialized_error.id);
- EXPECT_EQ(error.error.code, deserialized_error.error.code);
- EXPECT_EQ(error.error.message, deserialized_error.error.message);
-}
-
-TEST(ProtocolMCPTest, ResponseWithError) {
- ErrorInfo error_info;
- error_info.code = -32700;
- error_info.message = "Parse error";
-
- Response response;
- response.id = 4;
- response.error = error_info;
-
- llvm::Expected<Response> deserialized_response = roundtripJSON(response);
- ASSERT_THAT_EXPECTED(deserialized_response, llvm::Succeeded());
+ ASSERT_TRUE(std::holds_alternative<Response>(*deserialized_message));
+ const Response &deserialized_error =
+ std::get<Response>(*deserialized_message);
- EXPECT_EQ(response.id, deserialized_response->id);
- EXPECT_FALSE(deserialized_response->result.has_value());
- ASSERT_TRUE(deserialized_response->error.has_value());
- EXPECT_EQ(response.error->code, deserialized_response->error->code);
- EXPECT_EQ(response.error->message, deserialized_response->error->message);
+ llvm::errs() << "here: " << toJSON(deserialized_error) << "\n";
+ EXPECT_EQ(error_response, deserialized_error);
}
TEST(ProtocolMCPTest, Resource) {
diff --git a/lldb/unittests/ProtocolServer/ProtocolMCPServerTest.cpp b/lldb/unittests/ProtocolServer/ProtocolMCPServerTest.cpp
index ade299e08698d..2ac40c41dd28e 100644
--- a/lldb/unittests/ProtocolServer/ProtocolMCPServerTest.cpp
+++ b/lldb/unittests/ProtocolServer/ProtocolMCPServerTest.cpp
@@ -200,7 +200,7 @@ class ProtocolServerMCPTest : public ::testing::Test {
} // namespace
-TEST_F(ProtocolServerMCPTest, Intialization) {
+TEST_F(ProtocolServerMCPTest, Initialization) {
llvm::StringLiteral request =
R"json({"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"lldb-unit","version":"0.1.0"}},"jsonrpc":"2.0","id":0})json";
llvm::StringLiteral response =
diff --git a/lldb/unittests/TestingSupport/TestUtilities.h b/lldb/unittests/TestingSupport/TestUtilities.h
index db62881872fef..70db98907eb45 100644
--- a/lldb/unittests/TestingSupport/TestUtilities.h
+++ b/lldb/unittests/TestingSupport/TestUtilities.h
@@ -16,6 +16,7 @@
#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/raw_ostream.h"
#include <string>
#define ASSERT_NO_ERROR(x) \
@@ -61,12 +62,12 @@ class TestFile {
};
template <typename T> static llvm::Expected<T> roundtripJSON(const T &input) {
- llvm::json::Value value = toJSON(input);
+ std::string encoded;
+ llvm::raw_string_ostream OS(encoded);
+ OS << toJSON(input);
+ llvm::errs() << "Here: " << encoded << "\n";
llvm::json::Path::Root root;
- T output;
- if (!fromJSON(value, output, root))
- return root.getError();
- return output;
+ return llvm::json::parse<T>(encoded);
}
} // namespace lldb_private
|
|
This is failing to build on our Windows on Arm bot because apparently Trying to come up with a solution that isn't a magic assert. |
DavidSpickett
added a commit
that referenced
this pull request
Aug 13, 2025
Caused by #153297. This is likely not Windows on Arm specific but the other Windows bots don't have this problem. json::parse tries to construct llvm::Expected<Message> by moving another instance of Message into it. This would normally use this constructor: ``` /// Create an Expected<T> success value from the given OtherT value, which /// must be convertible to T. template <typename OtherT> Expected(OtherT &&Val, std::enable_if_t<std::is_convertible_v<OtherT, T>> * = nullptr) <...> ``` Note that llvm::Expected does not have a T&& constructor. Presumably the authors thought the converting one would be used. Except that in our build, using clang-cl 19.1.7, Visual Studio 2022, MSVC STL 202208, somehow is_convertible_v is false for Message. If you add a static_assert to check that this is the case, it suddenly becomes convertible. As best I can understand, this is because evaluation of the properties of Message are delayed. Delayed so much that by the time the constructor is called, it's still false. So we can "fix" this by asserting that it is convertible some time before it is checked by the constructor. I'm not sure if that's a compiler problem or the way MSVC STL's variant is written. I couldn't reproduce this behaviour in smaller examples or on Linux systems. This is the least invasive fix and only touches the new lldb code, so I'm going with it. Including the whole error here because it's a strange one and maybe later someone who has a clue about this can fix it in a better way. ``` C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build>ninja [5/15] Building CXX object tools\lldb\source\Pro...CP\CMakeFiles\lldbProtocolMCP.dir\Server.cpp.obj FAILED: tools/lldb/source/Protocol/MCP/CMakeFiles/lldbProtocolMCP.dir/Server.cpp.obj C:\Users\tcwg\scoop\shims\ccache.exe C:\Users\tcwg\scoop\apps\llvm-arm64\current\bin\clang-cl.exe /nologo -TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_ENABLE_EXTENDED_ALIGNED_STORAGE -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\source\Protocol\MCP -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\source\Protocol\MCP -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include -IC:\Users\tcwg\scoop\apps\python\current\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\..\clang\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\..\clang\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\source -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\source /DWIN32 /D_WINDOWS /Zc:inline /Zc:__cplusplus /Oi /Brepro /bigobj /permissive- -Werror=unguarded-availability-new /W4 -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported /Gw -Wno-vla-extension /O2 /Ob2 /DNDEBUG -std:c++17 -MD -wd4018 -wd4068 -wd4150 -wd4201 -wd4251 -wd4521 -wd4530 -wd4589 /EHs-c- /GR- /showIncludes /Fotools\lldb\source\Protocol\MCP\CMakeFiles\lldbProtocolMCP.dir\Server.cpp.obj /Fdtools\lldb\source\Protocol\MCP\CMakeFiles\lldbProtocolMCP.dir\lldbProtocolMCP.pdb -c -- C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\source\Protocol\MCP\Server.cpp In file included from C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\source\Protocol\MCP\Server.cpp:9: In file included from C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\include\lldb/Protocol/MCP/Server.h:12: In file included from C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\include\lldb/Protocol/MCP/Protocol.h:17: C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/JSON.h(938,12): error: no viable conversion from returned value of type 'remove_reference_t<variant<Request, Response, Notification> &>' (aka 'std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>') to function return type 'Expected<variant<Request, Response, Notification>>' 938 | return std::move(Result); | ^~~~~~~~~~~~~~~~~ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\source\Protocol\MCP\Server.cpp(57,30): note: in instantiation of function template specialization 'llvm::json::parse<std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>>' requested here 57 | auto message = llvm::json::parse<Message>(/*JSON=*/data); | ^ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/Error.h(485,40): note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'remove_reference_t<variant<Request, Response, Notification> &>' (aka 'std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>') to 'const Expected<variant<Request, Response, Notification>> &' for 1st argument 485 | template <class T> class [[nodiscard]] Expected { | ^~~~~~~~ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/Error.h(507,3): note: candidate constructor not viable: no known conversion from 'remove_reference_t<variant<Request, Response, Notification> &>' (aka 'std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>') to 'Error &&' for 1st argument 507 | Expected(Error &&Err) | ^ ~~~~~~~~~~~ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/Error.h(521,3): note: candidate constructor not viable: no known conversion from 'remove_reference_t<variant<Request, Response, Notification> &>' (aka 'std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>') to 'ErrorSuccess' for 1st argument 521 | Expected(ErrorSuccess) = delete; | ^ ~~~~~~~~~~~~ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/Error.h(539,3): note: candidate constructor not viable: no known conversion from 'remove_reference_t<variant<Request, Response, Notification> &>' (aka 'std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>') to 'Expected<variant<Request, Response, Notification>> &&' for 1st argument 539 | Expected(Expected &&Other) { moveConstruct(std::move(Other)); } | ^ ~~~~~~~~~~~~~~~~ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/Error.h(526,3): note: candidate template ignored: requirement 'std::is_convertible_v<std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>, std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>>' was not satisfied [with OtherT = remove_reference_t<variant<Request, Response, Notification> &>] 526 | Expected(OtherT &&Val, | ^ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/Error.h(544,3): note: candidate template ignored: could not match 'Expected' against 'variant' 544 | Expected(Expected<OtherT> &&Other, | ^ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/Error.h(552,12): note: explicit constructor is not a candidate 552 | explicit Expected( | ^ 1 error generated. [8/15] Building CXX object tools\lldb\source\Plu...nProtocolServerMCP.dir\ProtocolServerMCP.cpp.obj ninja: build stopped: subcommand failed. ```
|
Magic assert it is - dece902. |
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Aug 13, 2025
Caused by llvm/llvm-project#153297. This is likely not Windows on Arm specific but the other Windows bots don't have this problem. json::parse tries to construct llvm::Expected<Message> by moving another instance of Message into it. This would normally use this constructor: ``` /// Create an Expected<T> success value from the given OtherT value, which /// must be convertible to T. template <typename OtherT> Expected(OtherT &&Val, std::enable_if_t<std::is_convertible_v<OtherT, T>> * = nullptr) <...> ``` Note that llvm::Expected does not have a T&& constructor. Presumably the authors thought the converting one would be used. Except that in our build, using clang-cl 19.1.7, Visual Studio 2022, MSVC STL 202208, somehow is_convertible_v is false for Message. If you add a static_assert to check that this is the case, it suddenly becomes convertible. As best I can understand, this is because evaluation of the properties of Message are delayed. Delayed so much that by the time the constructor is called, it's still false. So we can "fix" this by asserting that it is convertible some time before it is checked by the constructor. I'm not sure if that's a compiler problem or the way MSVC STL's variant is written. I couldn't reproduce this behaviour in smaller examples or on Linux systems. This is the least invasive fix and only touches the new lldb code, so I'm going with it. Including the whole error here because it's a strange one and maybe later someone who has a clue about this can fix it in a better way. ``` C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build>ninja [5/15] Building CXX object tools\lldb\source\Pro...CP\CMakeFiles\lldbProtocolMCP.dir\Server.cpp.obj FAILED: tools/lldb/source/Protocol/MCP/CMakeFiles/lldbProtocolMCP.dir/Server.cpp.obj C:\Users\tcwg\scoop\shims\ccache.exe C:\Users\tcwg\scoop\apps\llvm-arm64\current\bin\clang-cl.exe /nologo -TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_ENABLE_EXTENDED_ALIGNED_STORAGE -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\source\Protocol\MCP -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\source\Protocol\MCP -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include -IC:\Users\tcwg\scoop\apps\python\current\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\..\clang\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\..\clang\include -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\source -IC:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\source /DWIN32 /D_WINDOWS /Zc:inline /Zc:__cplusplus /Oi /Brepro /bigobj /permissive- -Werror=unguarded-availability-new /W4 -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported /Gw -Wno-vla-extension /O2 /Ob2 /DNDEBUG -std:c++17 -MD -wd4018 -wd4068 -wd4150 -wd4201 -wd4251 -wd4521 -wd4530 -wd4589 /EHs-c- /GR- /showIncludes /Fotools\lldb\source\Protocol\MCP\CMakeFiles\lldbProtocolMCP.dir\Server.cpp.obj /Fdtools\lldb\source\Protocol\MCP\CMakeFiles\lldbProtocolMCP.dir\lldbProtocolMCP.pdb -c -- C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\source\Protocol\MCP\Server.cpp In file included from C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\source\Protocol\MCP\Server.cpp:9: In file included from C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\include\lldb/Protocol/MCP/Server.h:12: In file included from C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\include\lldb/Protocol/MCP/Protocol.h:17: C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/JSON.h(938,12): error: no viable conversion from returned value of type 'remove_reference_t<variant<Request, Response, Notification> &>' (aka 'std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>') to function return type 'Expected<variant<Request, Response, Notification>>' 938 | return std::move(Result); | ^~~~~~~~~~~~~~~~~ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\source\Protocol\MCP\Server.cpp(57,30): note: in instantiation of function template specialization 'llvm::json::parse<std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>>' requested here 57 | auto message = llvm::json::parse<Message>(/*JSON=*/data); | ^ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/Error.h(485,40): note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'remove_reference_t<variant<Request, Response, Notification> &>' (aka 'std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>') to 'const Expected<variant<Request, Response, Notification>> &' for 1st argument 485 | template <class T> class [[nodiscard]] Expected { | ^~~~~~~~ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/Error.h(507,3): note: candidate constructor not viable: no known conversion from 'remove_reference_t<variant<Request, Response, Notification> &>' (aka 'std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>') to 'Error &&' for 1st argument 507 | Expected(Error &&Err) | ^ ~~~~~~~~~~~ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/Error.h(521,3): note: candidate constructor not viable: no known conversion from 'remove_reference_t<variant<Request, Response, Notification> &>' (aka 'std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>') to 'ErrorSuccess' for 1st argument 521 | Expected(ErrorSuccess) = delete; | ^ ~~~~~~~~~~~~ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/Error.h(539,3): note: candidate constructor not viable: no known conversion from 'remove_reference_t<variant<Request, Response, Notification> &>' (aka 'std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>') to 'Expected<variant<Request, Response, Notification>> &&' for 1st argument 539 | Expected(Expected &&Other) { moveConstruct(std::move(Other)); } | ^ ~~~~~~~~~~~~~~~~ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/Error.h(526,3): note: candidate template ignored: requirement 'std::is_convertible_v<std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>, std::variant<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>>' was not satisfied [with OtherT = remove_reference_t<variant<Request, Response, Notification> &>] 526 | Expected(OtherT &&Val, | ^ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/Error.h(544,3): note: candidate template ignored: could not match 'Expected' against 'variant' 544 | Expected(Expected<OtherT> &&Other, | ^ C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/Support/Error.h(552,12): note: explicit constructor is not a candidate 552 | explicit Expected( | ^ 1 error generated. [8/15] Building CXX object tools\lldb\source\Plu...nProtocolServerMCP.dir\ProtocolServerMCP.cpp.obj ninja: build stopped: subcommand failed. ```
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.
Request/Responsetypes to have anidthat is either a string or a number.Error.datafield to by any JSON value.operator==support to the base protocol types and simplifies the tests.