[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