[clang-tools-extra] c7ef0ac - [clangd] Drop required attributes from ContainedRef protos

Kadir Cetinkaya via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 5 01:19:43 PST 2024


Author: Kadir Cetinkaya
Date: 2024-12-05T10:18:19+01:00
New Revision: c7ef0ac9fd28cb55b8c7c91a890b365cc688f9a9

URL: https://github.com/llvm/llvm-project/commit/c7ef0ac9fd28cb55b8c7c91a890b365cc688f9a9
DIFF: https://github.com/llvm/llvm-project/commit/c7ef0ac9fd28cb55b8c7c91a890b365cc688f9a9.diff

LOG: [clangd] Drop required attributes from ContainedRef protos

Per https://protobuf.dev/programming-guides/dos-donts/#add-required this
is discouraged and we already handle errors when marshalling protos.
This also ensures new message types are consistent with the rest in the
file.

Added: 
    

Modified: 
    clang-tools-extra/clangd/index/remote/Index.proto
    clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/index/remote/Index.proto b/clang-tools-extra/clangd/index/remote/Index.proto
index 689ef9d44ee409..3e39724e320860 100644
--- a/clang-tools-extra/clangd/index/remote/Index.proto
+++ b/clang-tools-extra/clangd/index/remote/Index.proto
@@ -133,7 +133,7 @@ message Relation {
 }
 
 message ContainedRefsRequest {
-  required string id = 1;
+  optional string id = 1;
   optional uint32 limit = 2;
 }
 
@@ -145,7 +145,7 @@ message ContainedRefsReply {
 }
 
 message ContainedRef {
-  required SymbolLocation location = 1;
-  required uint32 kind = 2;
-  required string symbol = 3;
+  optional SymbolLocation location = 1;
+  optional uint32 kind = 2;
+  optional string symbol = 3;
 }

diff  --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
index a80d12347d48d2..d8d3b64a5ac18c 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -129,6 +129,8 @@ Marshaller::fromProtobuf(const RefsRequest *Message) {
 llvm::Expected<clangd::ContainedRefsRequest>
 Marshaller::fromProtobuf(const ContainedRefsRequest *Message) {
   clangd::ContainedRefsRequest Req;
+  if (!Message->has_id())
+    return error("ContainedRefsRequest requires an id.");
   auto ID = SymbolID::fromStr(Message->id());
   if (!ID)
     return ID.takeError();
@@ -207,6 +209,12 @@ llvm::Expected<clangd::Ref> Marshaller::fromProtobuf(const Ref &Message) {
 llvm::Expected<clangd::ContainedRefsResult>
 Marshaller::fromProtobuf(const ContainedRef &Message) {
   clangd::ContainedRefsResult Result;
+  if (!Message.has_location())
+    return error("ContainedRef must have a location.");
+  if (!Message.has_kind())
+    return error("ContainedRef must have a kind.");
+  if (!Message.has_symbol())
+    return error("ContainedRef must have a symbol.");
   auto Location = fromProtobuf(Message.location());
   if (!Location)
     return Location.takeError();


        


More information about the cfe-commits mailing list