[PATCH] D89882: [clangd] Migrate to proto2 syntax
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 23 05:10:31 PDT 2020
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/marshalling/Marshalling.cpp:118
Req.IDs = std::move(*IDs);
- Req.Filter = static_cast<RefKind>(Message->filter());
+ // This sets a default for RefKind. Doing it explicitly in proto definition
+ // is error-prone because clangd::RefKind::All can change unexpectedly.
----------------
I don't think this comment is necessary. The code reads clearly and is more obvious/natural than the alternative you're arguing against here.
================
Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:137
+ if (!Message->has_predicate())
+ return error("RelationnsRequest requires RelationKind predicate.");
Req.Predicate = static_cast<RelationKind>(Message->predicate());
----------------
nit: RelationsRequest
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89882/new/
https://reviews.llvm.org/D89882
More information about the cfe-commits
mailing list