<div dir="ltr"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Wed, Sep 12, 2018 at 6:20 PM Jan Korous via clangd-dev <<a href="mailto:clangd-dev@lists.llvm.org">clangd-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi all,<br>
<br>
We had an internal discussion about XPC use with clangd and the result is that XPC-based transport layer in clangd is a hard requirement. This effectively means that the adapter approach is not feasible and I am looking into the transport layer abstraction Sam proposed earlier (<a href="https://reviews.llvm.org/D49389" rel="noreferrer" target="_blank">https://reviews.llvm.org/D49389</a>). Sorry for the trouble!<br></blockquote><div>No problem! (And sorry if I've been a bit unresponsive!)</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
There is a difficulty with designing an abstraction over transport layer due to XPC API requiring client code to hand control over. It's necessary for XPC server to call xpc_main() with a message handling callback:<br>
xpc_main(connection_handler);<br>
But XPC is running it's own message loop inside xpc_main() which is supposed to never return (in case of failure exit() is called) which makes it difficult to have common interface for JSON RPC and XPC.<br>
Unless I am mistaken that means that the message loop (Transport::loop(), ClangdLSPServer::run()) can't be part of generic code.<br>
<br>
I am curious what are other people's thoughts on this?<br></blockquote><div>That makes sense to me, but I don't think a common interface is impossible.</div><div>I noticed this in your patch, and in D49389 the idea (which I didn't spell out) was that the XPC transport's loop() would call xpc_main and never return.</div><div>Transport::loop() is specific to a transport, it doesn't need to be part of generic code. ClangdLSPServer::run() ends up calling Transport::loop(), so it could be common code.</div><div><br></div><div>It'd look something like:</div><div><br></div><div>static Transport::MessageHandler *IncomingXPCMessages; // XPC -> clangd</div><div>static xpc_connection_t *XPCConnection; // clangd -> XPC</div><div><br></div><div>XPCTransport::loop(MessageHandler &M) {<br></div><div>  IncomingXPCMessages = &M;</div><div>  xpc_main(&XPCEntrypoint);</div><div>}</div><div><br></div><div>static void XPCEntrypoint(xpc_connection_t Conn) {</div><div>  // I don't remember how an XPC server looks</div><div>  // i guess you're reading editor->clangd messages in a loop?</div><div>  for (;;) {</div><div>    // lets say you parsed a notification</div><div>    std::string name = ...;<br></div><div>    json::Value args = xpcToJSON(...);</div><div>    IncomingXPCMessages->notify(name, args); // send the notification to clangd</div><div>  }</div><div>}</div><div><br></div><div>XPCTransport::notify(StringRef name, json::Value args) {<br></div><div>  xpc_object_t XPCNotification = encodeNotificationForXPCSomehow(name, jsonToXPC(args))</div><div>  xpc_connection_send(*XPCConnection, XPCNotification);</div><div>}</div><div><br></div><div>WDYT?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
I tried to come up with some reasonable abstraction but I am probably already anchored by my previous work (<a href="https://reviews.llvm.org/D48559" rel="noreferrer" target="_blank">https://reviews.llvm.org/D48559</a>) - all my design ideas tend to go in the direction of abstraction over JsonOutput and custom dispatchers (StdIoDispatcher/XPCDispatcher).<br>
<br>
Cheers,<br>
<br>
Jan<br>
_______________________________________________<br>
clangd-dev mailing list<br>
<a href="mailto:clangd-dev@lists.llvm.org" target="_blank">clangd-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/clangd-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/clangd-dev</a><br>
</blockquote></div></div></div>