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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
