Skip to content

Commit d861156

Browse files
ashgtiashgti
committed
Adding some comments and adjusting the naming of the helpers.
1 parent 48eb7de commit d861156

File tree

4 files changed

+79
-63
lines changed

4 files changed

+79
-63
lines changed

‎lldb/include/lldb/Host/JSONTransport.h‎

Lines changed: 53 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -76,37 +76,55 @@ using VoidT = std::monostate;
7676

7777
template <typename T> using Callback = llvm::unique_function<T>;
7878

79+
/// A handler for the response to an outgoing request.
7980
template <typename T>
8081
using Reply = typename std::conditional<
81-
std::is_same_v<T, VoidT> == true, llvm::unique_function<void(llvm::Error)>,
82+
std::is_same_v<T, VoidT> || std::is_void_v<T>,
83+
llvm::unique_function<void(llvm::Error)>,
8284
llvm::unique_function<void(llvm::Expected<T>)>>::type;
8385

86+
/// A function to send an outgoing request and receive a response.
8487
template <typename Result, typename Params>
8588
using OutgoingRequest = typename std::conditional<
86-
std::is_same_v<Params, VoidT> == true,
89+
std::is_same_v<Params, VoidT> || std::is_void_v<Params>,
8790
llvm::unique_function<void(Reply<Result>)>,
8891
llvm::unique_function<void(const Params &, Reply<Result>)>>::type;
8992

93+
/// A function to send an outgoing event.
9094
template <typename Params>
9195
using OutgoingEvent = typename std::conditional<
92-
std::is_same_v<Params, VoidT> == true, llvm::unique_function<void()>,
96+
std::is_same_v<Params, VoidT> || std::is_void_v<Params>,
97+
llvm::unique_function<void()>,
9398
llvm::unique_function<void(const Params &)>>::type;
9499

100+
/// Creates a request with the given id, method, and optional params.
95101
template <typename Id, typename Req>
96-
Req make_request(Id id, llvm::StringRef method,
97-
std::optional<llvm::json::Value> params = std::nullopt);
102+
Req MakeRequest(Id, llvm::StringRef, std::optional<llvm::json::Value>);
103+
104+
/// Creates an error response for a given request.
98105
template <typename Req, typename Resp>
99-
Resp make_response(const Req &req, llvm::Error error);
106+
Resp MakeResponse(const Req &, llvm::Error);
107+
108+
/// Creates a success response for a given request.
100109
template <typename Req, typename Resp>
101-
Resp make_response(const Req &req, llvm::json::Value result);
110+
Resp MakeResponse(const Req &, llvm::json::Value);
111+
112+
/// Creates an event.
102113
template <typename Evt>
103-
Evt make_event(llvm::StringRef method,
104-
std::optional<llvm::json::Value> params = std::nullopt);
114+
Evt MakeEvent(llvm::StringRef, std::optional<llvm::json::Value>);
115+
116+
/// Extracts the result value from a response.
105117
template <typename Resp>
106-
llvm::Expected<llvm::json::Value> get_result(const Resp &resp);
107-
template <typename Id, typename T> Id get_id(const T &);
108-
template <typename T> llvm::StringRef get_method(const T &);
109-
template <typename T> llvm::json::Value get_params(const T &);
118+
llvm::Expected<llvm::json::Value> GetResult(const Resp &);
119+
120+
/// Extracts the id from a response.
121+
template <typename Id, typename Resp> Id GetId(const Resp &);
122+
123+
/// Extracts the method from a request or event.
124+
template <typename T> llvm::StringRef GetMethod(const T &);
125+
126+
/// Extracts the parameters from a request or event.
127+
template <typename T> llvm::json::Value GetParams(const T &);
110128

111129
/// A transport is responsible for maintaining the connection to a client
112130
/// application, and reading/writing structured messages to it.
@@ -193,7 +211,7 @@ class JSONTransport {
193211
~ReplyOnce() {
194212
if (transport && handler && !replied) {
195213
assert(false && "must reply to all calls!");
196-
(*this)(make_response<Req, Resp>(
214+
(*this)(MakeResponse<Req, Resp>(
197215
req, llvm::createStringError("failed to reply")));
198216
}
199217
}
@@ -257,9 +275,9 @@ class JSONTransport {
257275
return std::move(result);
258276
}
259277

260-
/// Bind a handler for a request.
261-
/// e.g. `bind("peek", &ThisModule::peek, this, std::placeholders::_1);`.
262-
/// Handler should be e.g. `Expected<PeekResult> peek(const PeekParams&);`
278+
/// Bind a handler for an incoming request.
279+
/// e.g. `bind("peek", &ThisModule::peek, this);`.
280+
/// Handler should be e.g. `Expected<PeekResult> peek(const PeekParams&);`
263281
/// PeekParams must be JSON parsable and PeekResult must be serializable.
264282
template <typename Result, typename Params, typename Fn, typename... Args>
265283
void bind(llvm::StringLiteral method, Fn &&fn, Args &&...args) {
@@ -273,30 +291,30 @@ class JSONTransport {
273291
llvm::Expected<Result> result = std::invoke(
274292
std::forward<Fn>(fn), std::forward<Args>(args)...);
275293
if (!result)
276-
return reply(make_response<Req, Resp>(req, result.takeError()));
277-
reply(make_response<Req, Resp>(req, toJSON(*result)));
294+
return reply(MakeResponse<Req, Resp>(req, result.takeError()));
295+
reply(MakeResponse<Req, Resp>(req, toJSON(*result)));
278296
};
279297
} else {
280298
m_request_handlers[method] =
281299
[method, fn,
282300
args...](const Req &req,
283301
llvm::unique_function<void(const Resp &)> reply) mutable {
284302
llvm::Expected<Params> params =
285-
parse<Params>(get_params<Req>(req), method);
303+
parse<Params>(GetParams<Req>(req), method);
286304
if (!params)
287-
return reply(make_response<Req, Resp>(req, params.takeError()));
305+
return reply(MakeResponse<Req, Resp>(req, params.takeError()));
288306

289307
llvm::Expected<Result> result = std::invoke(
290308
std::forward<Fn>(fn), std::forward<Args>(args)..., *params);
291309
if (!result)
292-
return reply(make_response<Req, Resp>(req, result.takeError()));
310+
return reply(MakeResponse<Req, Resp>(req, result.takeError()));
293311

294-
reply(make_response<Req, Resp>(req, toJSON(*result)));
312+
reply(MakeResponse<Req, Resp>(req, toJSON(*result)));
295313
};
296314
}
297315
}
298316

299-
/// Bind a handler for a event.
317+
/// Bind a handler for an incoming event.
300318
/// e.g. `bind("peek", &ThisModule::peek, this);`
301319
/// Handler should be e.g. `void peek(const PeekParams&);`
302320
/// PeekParams must be JSON parsable.
@@ -312,7 +330,7 @@ class JSONTransport {
312330
m_event_handlers[method] = [this, method, fn,
313331
args...](const Evt &evt) mutable {
314332
llvm::Expected<Params> params =
315-
parse<Params>(get_params<Evt>(evt), method);
333+
parse<Params>(GetParams<Evt>(evt), method);
316334
if (!params)
317335
return OnError(params.takeError());
318336
std::invoke(std::forward<Fn>(fn), std::forward<Args>(args)...,
@@ -330,10 +348,10 @@ class JSONTransport {
330348
return [this, method](Reply<Result> fn) {
331349
std::scoped_lock<std::recursive_mutex> guard(m_mutex);
332350
Id id = ++m_seq;
333-
Req req = make_request<Req, Resp>(id, method, std::nullopt);
351+
Req req = MakeRequest<Req, Resp>(id, method, std::nullopt);
334352
m_pending_responses[id] = [fn = std::move(fn),
335353
method](const Resp &resp) mutable {
336-
llvm::Expected<llvm::json::Value> result = get_result<Resp>(resp);
354+
llvm::Expected<llvm::json::Value> result = GetResult<Resp>(resp);
337355
if (!result)
338356
return fn(result.takeError());
339357
fn(parse<Result>(*result, method));
@@ -345,11 +363,10 @@ class JSONTransport {
345363
return [this, method](const Params &params, Reply<Result> fn) {
346364
std::scoped_lock<std::recursive_mutex> guard(m_mutex);
347365
Id id = ++m_seq;
348-
Req req =
349-
make_request<Id, Req>(id, method, llvm::json::Value(params));
366+
Req req = MakeRequest<Id, Req>(id, method, llvm::json::Value(params));
350367
m_pending_responses[id] = [fn = std::move(fn),
351368
method](const Resp &resp) mutable {
352-
llvm::Expected<llvm::json::Value> result = get_result<Resp>(resp);
369+
llvm::Expected<llvm::json::Value> result = GetResult<Resp>(resp);
353370
if (llvm::Error err = result.takeError())
354371
return fn(std::move(err));
355372
fn(parse<Result>(*result, method));
@@ -368,21 +385,21 @@ class JSONTransport {
368385
if constexpr (std::is_void_v<Params> || std::is_same_v<VoidT, Params>) {
369386
return [this, method]() {
370387
if (llvm::Error error =
371-
m_transport.Send(make_event<Evt>(method, std::nullopt)))
388+
m_transport.Send(MakeEvent<Evt>(method, std::nullopt)))
372389
OnError(std::move(error));
373390
};
374391
} else {
375392
return [this, method](const Params &params) {
376393
if (llvm::Error error =
377-
m_transport.Send(make_event<Evt>(method, toJSON(params))))
394+
m_transport.Send(MakeEvent<Evt>(method, toJSON(params))))
378395
OnError(std::move(error));
379396
};
380397
}
381398
}
382399

383400
void Received(const Evt &evt) override {
384401
std::scoped_lock<std::recursive_mutex> guard(m_mutex);
385-
auto it = m_event_handlers.find(get_method<Evt>(evt));
402+
auto it = m_event_handlers.find(GetMethod<Evt>(evt));
386403
if (it == m_event_handlers.end()) {
387404
OnError(llvm::createStringError(
388405
llvm::formatv("no handled for event {0}", toJSON(evt))));
@@ -395,9 +412,9 @@ class JSONTransport {
395412
ReplyOnce reply(req, &m_transport, this);
396413

397414
std::scoped_lock<std::recursive_mutex> guard(m_mutex);
398-
auto it = m_request_handlers.find(get_method<Req>(req));
415+
auto it = m_request_handlers.find(GetMethod<Req>(req));
399416
if (it == m_request_handlers.end()) {
400-
reply(make_response<Req, Resp>(
417+
reply(MakeResponse<Req, Resp>(
401418
req, llvm::createStringError("method not found")));
402419
return;
403420
}
@@ -407,7 +424,7 @@ class JSONTransport {
407424

408425
void Received(const Resp &resp) override {
409426
std::scoped_lock<std::recursive_mutex> guard(m_mutex);
410-
auto it = m_pending_responses.find(get_id<Id, Resp>(resp));
427+
auto it = m_pending_responses.find(GetId<Id, Resp>(resp));
411428
if (it == m_pending_responses.end()) {
412429
OnError(llvm::createStringError(
413430
llvm::formatv("no pending request for {0}", toJSON(resp))));

‎lldb/include/lldb/Protocol/MCP/Transport.h‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@ namespace lldb_private {
2222
/// @{
2323
template <>
2424
inline lldb_protocol::mcp::Request
25-
make_request(int64_t id, llvm::StringRef method,
26-
std::optional<llvm::json::Value> params) {
25+
MakeRequest(int64_t id, llvm::StringRef method,
26+
std::optional<llvm::json::Value> params) {
2727
return lldb_protocol::mcp::Request{id, method.str(), params};
2828
}
2929
template <>
3030
inline lldb_protocol::mcp::Response
31-
make_response(const lldb_protocol::mcp::Request &req, llvm::Error error) {
31+
MakeResponse(const lldb_protocol::mcp::Request &req, llvm::Error error) {
3232
lldb_protocol::mcp::Error protocol_error;
3333
llvm::handleAllErrors(
3434
std::move(error),
@@ -44,42 +44,41 @@ make_response(const lldb_protocol::mcp::Request &req, llvm::Error error) {
4444
}
4545
template <>
4646
inline lldb_protocol::mcp::Response
47-
make_response(const lldb_protocol::mcp::Request &req,
48-
llvm::json::Value result) {
47+
MakeResponse(const lldb_protocol::mcp::Request &req, llvm::json::Value result) {
4948
return lldb_protocol::mcp::Response{req.id, std::move(result)};
5049
}
5150
template <>
5251
inline lldb_protocol::mcp::Notification
53-
make_event(llvm::StringRef method, std::optional<llvm::json::Value> params) {
52+
MakeEvent(llvm::StringRef method, std::optional<llvm::json::Value> params) {
5453
return lldb_protocol::mcp::Notification{method.str(), params};
5554
}
5655
template <>
5756
inline llvm::Expected<llvm::json::Value>
58-
get_result(const lldb_protocol::mcp::Response &resp) {
57+
GetResult(const lldb_protocol::mcp::Response &resp) {
5958
if (const lldb_protocol::mcp::Error *error =
6059
std::get_if<lldb_protocol::mcp::Error>(&resp.result))
6160
return llvm::make_error<lldb_protocol::mcp::MCPError>(error->message,
6261
error->code);
6362
return std::get<llvm::json::Value>(resp.result);
6463
}
65-
template <> inline int64_t get_id(const lldb_protocol::mcp::Response &resp) {
64+
template <> inline int64_t GetId(const lldb_protocol::mcp::Response &resp) {
6665
return std::get<int64_t>(resp.id);
6766
}
6867
template <>
69-
inline llvm::StringRef get_method(const lldb_protocol::mcp::Request &req) {
68+
inline llvm::StringRef GetMethod(const lldb_protocol::mcp::Request &req) {
7069
return req.method;
7170
}
7271
template <>
73-
inline llvm::StringRef get_method(const lldb_protocol::mcp::Notification &evt) {
72+
inline llvm::StringRef GetMethod(const lldb_protocol::mcp::Notification &evt) {
7473
return evt.method;
7574
}
7675
template <>
77-
inline llvm::json::Value get_params(const lldb_protocol::mcp::Request &req) {
76+
inline llvm::json::Value GetParams(const lldb_protocol::mcp::Request &req) {
7877
return req.params;
7978
}
8079
template <>
8180
inline llvm::json::Value
82-
get_params(const lldb_protocol::mcp::Notification &evt) {
81+
GetParams(const lldb_protocol::mcp::Notification &evt) {
8382
return evt.params;
8483
}
8584
/// @}

‎lldb/unittests/Host/JSONTransportTest.cpp‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -322,38 +322,38 @@ class TestTransportBinder : public testing::Test {
322322
} // namespace
323323

324324
namespace lldb_private {
325+
325326
using namespace test_protocol;
327+
326328
template <>
327-
inline test_protocol::Req make_request(int id, llvm::StringRef method,
328-
std::optional<json::Value> params) {
329+
inline test_protocol::Req MakeRequest(int id, llvm::StringRef method,
330+
std::optional<json::Value> params) {
329331
return test_protocol::Req{id, method.str(), params};
330332
}
331-
template <> inline Resp make_response(const Req &req, llvm::Error error) {
333+
template <> inline Resp MakeResponse(const Req &req, llvm::Error error) {
332334
llvm::consumeError(std::move(error));
333335
return Resp{req.id, std::nullopt};
334336
}
335-
template <> inline Resp make_response(const Req &req, json::Value result) {
337+
template <> inline Resp MakeResponse(const Req &req, json::Value result) {
336338
return Resp{req.id, std::move(result)};
337339
}
338340
template <>
339-
inline Evt make_event(llvm::StringRef method,
340-
std::optional<json::Value> params) {
341+
inline Evt MakeEvent(llvm::StringRef method,
342+
std::optional<json::Value> params) {
341343
return Evt{method.str(), params};
342344
}
343-
344-
template <> inline llvm::Expected<json::Value> get_result(const Resp &resp) {
345+
template <> inline llvm::Expected<json::Value> GetResult(const Resp &resp) {
345346
return resp.result;
346347
}
347-
348-
template <> inline int get_id(const Resp &resp) { return resp.id; }
349-
template <> inline llvm::StringRef get_method(const Req &req) {
348+
template <> inline int GetId(const Resp &resp) { return resp.id; }
349+
template <> inline llvm::StringRef GetMethod(const Req &req) {
350350
return req.name;
351351
}
352-
template <> inline llvm::StringRef get_method(const Evt &evt) {
352+
template <> inline llvm::StringRef GetMethod(const Evt &evt) {
353353
return evt.name;
354354
}
355-
template <> inline json::Value get_params(const Req &req) { return req.params; }
356-
template <> inline json::Value get_params(const Evt &evt) { return evt.params; }
355+
template <> inline json::Value GetParams(const Req &req) { return req.params; }
356+
template <> inline json::Value GetParams(const Evt &evt) { return evt.params; }
357357

358358
} // namespace lldb_private
359359

‎lldb/unittests/Protocol/ProtocolMCPServerTest.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ class ProtocolServerMCPTest : public testing::Test {
173173
template <typename Result, typename Params>
174174
Expected<json::Value> Call(StringRef method, const Params &params) {
175175
std::promise<Response> promised_result;
176-
Request req = make_request<int64_t, lldb_protocol::mcp::Request>(
176+
Request req = MakeRequest<int64_t, lldb_protocol::mcp::Request>(
177177
/*id=*/1, method, toJSON(params));
178178
EXPECT_THAT_ERROR(to_server->Send(req), Succeeded());
179179
EXPECT_CALL(client, Received(testing::An<const Response &>()))

0 commit comments

Comments
 (0)