[PATCH] D48559: [clangd] refactoring for XPC transport layer [NFCI]

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 20 10:13:08 PDT 2018


sammccall added a comment.

Sorry for the delay here, and I'm sorry I haven't been into the patches in detail again yet - crazy week. After experiments, I am convinced the Transport abstraction patch can turn into something nice **if** we want XPC vs JSON to be a pure transport-level difference with the same semantics. But you need to decide the approach here based on your requirements.

In https://reviews.llvm.org/D48559#1168204, @jkorous wrote:

> Sam, just out of curiosity - would it be possible for you to share any relevant experience gained by using porting clangd to protobuf-based transport layer?


Sure! (and sorry for the delay here).

This is done as part of an internal-only editor that's a hosted web application with tight integration to our internal source control, build system etc.
That's not my immediate team, but we talk a lot and collaborate on the integration, which is different than a typical LSP server/editor integration in a few ways.
(I expect some of these aspects will apply to XCode+clangd, and others won't). This is kind of a brain dump...

- like a regular LSP client, the editor wants to run clangd out-of-process for isolation reasons, and because of cross-language issues. Because of our hosted datacenter environment, a network RPC server was the most obvious choice, and protobufs are our lingua franca for such servers. This sounds analagous to the choice of XPC to me.
- the editor is multi-language, so the idea of using the existing LSP is appealing (reuse servers, we can't rewrite the world).
- adapting to protobuf is some work (write a wrapper for each LSP server). In principle a generic one is possible, but not for us due to VFS needs etc.
- you can wrap at the LSP level (CompletionList -> CompletionListProto) or at the transport level (json::Value --> JsonValueProto). Editor team team went for the former.
- consuming clangd as a C++ API rather than as an external process or opaque JSON stream, gives lots more flexibility, and we can add extension points relatively easily. Robust VFS support, performance tracing, logging, different threading model are examples of this outside the scope of LSP. The internal code uses unmodified upstream clangd code, but provides its own main().
- Inside the scope of LSP, changes are tempting too due to LSP limitations. Since the editor team controls the protocol and the frontend, they can add features. This is a source of tension: we (clangd folks) don't want to support two divergent APIs. So in **limited** cases we added to clangd a richer/more semantic C++ API that provides extra features over more-presentational LSP. Best examples are diagnostics (C++ API includes fixits and 'note' stack, before LSP supported these), and code completion (C++ API includes semantic things like "name of include to insert"). We've tried to keep this limited to what's both valuable and sensible.
- adding LSP extensions to the *protocol* causes skew across languages. So the divergences at that level tend to be more presentational (e.g. "completion item documentation subtitle"). The clangd-wrapper decides how to map the semantic Clangd C++ API fields onto the presentational mutant-LSP fields, in the same way that regular clangd decides how to map them onto regular LSP.
- it's annoying that the issues you run into with this setup need to be carefully dissected (editor vs wrapper vs clangd) to work out where the bug belongs. Compare to "plain" clangd where you can just check with a different editor and then know the bug is in clangd.

In summary:

- speaking a different wire protocol that's semantically identical to LSP is an easy win
- owning a separate `main()` so you can plug in cross-cutting facilities (logging, tracing, index) is very manageable and worthwhile
- maintaining a separate protocol is a bunch more work and decisions all the time, even if divergences from LSP are small. It *is* more flexible though.

One halfway possibility if you're on the fence: we could support some limited extensions to the protocol behind an option (e.g. extra fields serialized with the option, whether XPC or JSON is used). It's less flexibility than full ownership of the protocol semantics on Apple's side though.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48559





More information about the cfe-commits mailing list