sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/index/remote/Index.proto:15
// structures corresponding to their clangd::* counterparts.
+// NOTE: Enum values are offset by one to detect missing values.
service SymbolIndex {
----------------
can we just switch to proto2 instead?
apart from reserving the zero value, proto3 also requires you to explicitly
handle zero in your code (vs being able to set a default in the proto
definition)
Offsetting enums by one isn't a big deal, but it can be in other cases
(consider a number like num_refs where 0 is a meaningful value and offsetting
by 1 is really confusing)
================
Comment at:
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:115
Req.IDs = std::move(*IDs);
- Req.Filter = static_cast<RefKind>(Message->filter());
+ Req.Filter = static_cast<RefKind>(Message->filter() - 1);
if (Message->limit())
----------------
this -1 and +1 everywhere makes me a bit leery.
Seems a bit easy to reverse or forget it.
What about
```
template <typename T> std::underlying_type_t<T> toProtobuf(T Value) {
return static_cast<std::underlying_type_t<T>>(Value) + 1;
}
```
and the inverse?
================
Comment at:
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:277
}
- Result.set_flags(static_cast<uint32_t>(From.Flags));
+ Result.set_flags(static_cast<uint32_t>(From.Flags) + 1);
return Result;
----------------
?!
this is a bitfield
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
