[PATCH] D83826: [clangd] Don't send invalid messages from remote index

This revision was automatically updated to reflect the committed changes. Closed by commit rGeef162c330b0: [clangd] Don't send invalid messages from remote index (authored by kbobyrev). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83826/new/ https:

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

kbobyrev updated this revision to Diff 279447. kbobyrev marked 5 inline comments as done. kbobyrev added a comment. Address post-LGTM comments, rebase on top of HEAD. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83826/new/ https://reviews.llvm.org

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/index/remote/Client.cpp:68 +DeadlineWaitingTime(DeadlineTime) { +ProtobufMarshaller = std::unique_ptr( +new

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

kbobyrev updated this revision to Diff 278683. kbobyrev added a comment. Remove (now empty) Marshalling.h file comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83826/new/ https://reviews.llvm.org/D83826 Files: clang-tools-extra/clangd/inde

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

kbobyrev updated this revision to Diff 278682. kbobyrev marked 2 inline comments as done. kbobyrev added a comment. Refactor marshalling into a class, log errors only on the high level. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83826/new/ https

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

kbobyrev planned changes to this revision. kbobyrev added a comment. Wrap marshalling into a class, think about filtering vs failing? I think filtering should be done on the clangd-indexer side and marshalling should deal with **valid** symbols only and shouldn't have to filter them. Repositor

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

kbobyrev updated this revision to Diff 278170. kbobyrev added a comment. Use DebugStrings instead of ShortDebugStrings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83826/new/ https://reviews.llvm.org/D83826 Files: clang-tools-extra/clangd/inde

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

kbobyrev updated this revision to Diff 278162. kbobyrev marked an inline comment as done. kbobyrev added a comment. Small update: resolve a couple of comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83826/new/ https://reviews.llvm.org/D83826

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

kbobyrev marked 4 inline comments as done. kbobyrev added a comment. In D83826#2152857 , @sammccall wrote: > > Also add more error messages and logs. > > I'm not sure about the error-handling strategy here: > > - it's very verbose in the marshalling code.

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

sammccall added a comment. In D83826#2151979 , @kbobyrev wrote: > Use the same style for elog messages (1-based indexing, message format). As far as I can tell, 1-based indexing isn't an option that's available. Where do you see that it is? Repository

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

sammccall added a comment. > Also add more error messages and logs. I'm not sure about the error-handling strategy here: - it's very verbose in the marshalling code. Are we sure it pays for itself? For comparison, the marshalling code for LSP itself silently returns `false` on error, which is

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

kbobyrev updated this revision to Diff 278086. kbobyrev added a comment. Don't allow empty paths. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83826/new/ https://reviews.llvm.org/D83826 Files: clang-tools-extra/clangd/index/remote/Client.cpp

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

kbobyrev updated this revision to Diff 278022. kbobyrev added a comment. Use the same style for elog messages (1-based indexing, message format). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83826/new/ https://reviews.llvm.org/D83826 Files: cla

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

kbobyrev updated this revision to Diff 278008. kbobyrev added a comment. Also do not attempt to use callback on unserialized messages. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83826/new/ https://reviews.llvm.org/D83826 Files: clang-tools-ex

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

kbobyrev created this revision. kbobyrev added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Remote server should not send messages that are invalid and will cause problems on the client