Page MenuHomePhabricator

[clangd] Implement Relations request for remote index

Authored by kbobyrev on Jul 29 2020, 3:30 PM.



This is the last missing bit in the core remote index implementation. The only
remaining bits are some API refactorings (replacing Optional with Expected and
being better at reporting errors).

Diff Detail

Event Timeline

kbobyrev created this revision.Jul 29 2020, 3:30 PM
kbobyrev requested review of this revision.Jul 29 2020, 3:30 PM
kbobyrev updated this revision to Diff 281754.Jul 29 2020, 3:34 PM

Fix comment in proto file and rebase on top of master.

thanks mostly nits, only annoying bit is usage of optionals and multi-logging but letting them be for now :D.

also looks like there are some irrelevant changes, e.g. auto's in lambdas or formatting in protos, feel free to land an NFC change(without review) for those before landing this.


nit: why not just do:

RelationsRequest Serialized;
// check for failure during deserialization

nit: move it near Sym.ID assignemnt.


nit: I would move all of the following population logic into a helper and share between here and symbol serialization tests.


nit: swap parameters ID is expected, Deserialized->first is actual.


i think this is already tested in symbol deserialization test above, feel free to leave it out.


again this should be part of symbol serialization tests.

kbobyrev updated this revision to Diff 281863.Jul 30 2020, 3:31 AM
kbobyrev marked 6 inline comments as done.

Address all comments.

kadircet accepted this revision.Jul 30 2020, 3:43 AM

thanks, lgtm!


i think you should still check for Deserialized->first == ID and Deserialized->second->ID == Sym.Id

This revision is now accepted and ready to land.Jul 30 2020, 3:43 AM
kbobyrev updated this revision to Diff 281869.Jul 30 2020, 3:56 AM
kbobyrev marked an inline comment as done.

Address post-LGTM comment.

This revision was landed with ongoing or failed builds.Jul 30 2020, 3:58 AM
This revision was automatically updated to reflect the committed changes.