[PATCH] D89862: [clangd] Give the server information about client's remote index protocol version

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

What's the goal here?

There are at least 3 mechanisms we can use to mitigate client/server skew:

1. extending payloads in compatible way, so mismatched versions result in correct behavior (switching from proto3 to proto2 gives us more options here if it's possible)
2. signalling the version in-band as in this patch
3. defining a new incompatible protocol with a new name, as in versioned services

IME 1 is the most maintainable way to handle minor changes, while 3 is the most maintainable way to handle dramatic changes. Maybe there's an intermediate spot where 2 is the best option, but I'd like to understand concretely what scenario we might be solving.

OTOH, including/logging client version to make it easier to *debug* errors, without changing server logic, makes a lot of sense.
In that case we should use the VCS version (the version included in clangd --version) so we don't have another string we need to bump.



================
Comment at: clang-tools-extra/clangd/index/remote/Index.proto:63
   uint32 limit = 3;
+  string client_version = 4;
 }
----------------
rather than add this to every message, can we use ClientContext::AddMetadata to do this in a cross-cutting way?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89862/new/

https://reviews.llvm.org/D89862