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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to