arphaman added a comment. In https://reviews.llvm.org/D48559#1169975, @sammccall wrote:
> 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. A custom protocol does sound quite appealing. I previously was quite skeptical just because ClangdLSPServer seemed to have had a lot of important logic, but it seems that that's not the case right now. I will lead an internal discussion on this to figure out if that's something we'd like to do. I'll respond here sometime next week. 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
