[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-19 Thread Kirill Bobyrev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc6b2b784299b: [clangd-remote] Replace YAML serialization with proper Protobuf messages (authored by kbobyrev). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-19 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added a comment. In D79862#2044073 , @sammccall wrote: > Thanks! Sorry about the hassle with the tests. No worries, I think it looks cleaner and simpler which is important. Thanks for the review! Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks! Sorry about the hassle with the tests. Comment at: clang-tools-extra/clangd/unittests/remote/CMakeLists.txt:2 +set(LLVM_LINK_COMPONENTS + Support + )

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-19 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/unittests/remote/CMakeLists.txt:2 +set(LLVM_LINK_COMPONENTS + Support + ) kbobyrev wrote: > sammccall wrote: > > Can we conditionally include these tests into the main ClangdTests, instead >

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-19 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 264874. kbobyrev marked 3 inline comments as done. kbobyrev added a comment. Move remote tests into all Clangd tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79862/new/ https://reviews.llvm.org/D79862 Fi

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-19 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 264877. kbobyrev added a comment. Move a comment to the line it corresponds to. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79862/new/ https://reviews.llvm.org/D79862 Files: clang-tools-extra/clangd/index

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-19 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/unittests/remote/CMakeLists.txt:2 +set(LLVM_LINK_COMPONENTS + Support + ) sammccall wrote: > Can we conditionally include these tests into the main ClangdTests, instead > of setting up anothe

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-19 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 264873. kbobyrev marked 6 inline comments as done. kbobyrev added a comment. Address some comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79862/new/ https://reviews.llvm.org/D79862 Files: clang-tool

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Just more stuff about simplifying the tests :-( This is really just about converting between a struct and a proto, if we have to add indexing-related fixtures to shared libraries something seems to have gone a bit off the rails. In the worst case we could just create

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-18 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 264633. kbobyrev added a comment. Rebase on top of master. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79862/new/ https://reviews.llvm.org/D79862 Files: clang-tools-extra/clangd/index/remote/Client.cpp

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-18 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 264554. kbobyrev added a comment. Inline HeaderCode and MainCode. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79862/new/ https://reviews.llvm.org/D79862 Files: clang-tools-extra/clangd/index/remote/Client

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-18 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 264553. kbobyrev added a comment. Don't complicate SymbolCollectorTest, use TestTU. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79862/new/ https://reviews.llvm.org/D79862 Files: clang-tools-extra/clangd/i

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-15 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/clangd/unittests/TestTU.h:1 //===--- TestTU.h - Scratch source files for testing -*- C++-*-===// // This header provides a widely used abstraction and has a clear scope, please don't d

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-15 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 264205. kbobyrev added a comment. Simplify test setup and rebase on top of master. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79862/new/ https://reviews.llvm.org/D79862 Files: clang-tools-extra/clangd/in

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-15 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 264190. kbobyrev marked an inline comment as done. kbobyrev added a comment. Herald added a subscriber: mgorny. Add tests for Protobuf (de)serialization. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79862/new/

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. It's tedious, but we really should have tests for this now. Comment at: clang-tools-extra/clangd/index/remote/Index.proto:68 -// FIXME(kirillbobyrev): Properly serialize symbols and refs instead of passing -// YAML. -message Ref { string yaml_serial

[PATCH] D79862: [clangd-remote] Replace YAML serialization with proper Protobuf messages

2020-05-13 Thread Kirill Bobyrev via Phabricator via cfe-commits
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. YAML serialization was used in the Proof of Concept for simplicity. This patch replaces imple