[clangd] Separate final_result into a different message

[kbobyrev]

Summary

This is a breaking change in remote index protocol.

[kbobyrev]

Fix proto formatting

[kbobyrev]

Rebase on top of master, use proto2 syntax.

[sammccall]

clang-tools-extra/clangd/index/remote/Index.proto

18

this deserves a comment: "Common final result for streaming requests."

18

this isn't specific to Lookup, so shouldn't go between LookupRequest and LookupReply, rather either at the very top or below all the request/reply messages.

23

I think stream_result/result suggests that result is "primary" which isn't the case.
I think the old names stream_result/final_result were clearer.

[kbobyrev]

Address post-LGTM comments.

clang-tools-extra/clangd/index/remote/Client.cpp

std::chrono::system_clock::time_point Deadline =
         std::chrono::system_clock::now() + DeadlineWaitingTime;
     Context.set_deadline(Deadline);
     auto Reader = (Stub.get()->*RPCCall)(&Context, RPCRequest);
     ReplyT Reply;
     unsigned Successful = 0;
     unsigned FailedToParse = 0;
     while (Reader->Read(&Reply)) {
       if (!Reply.has_stream_result()) {
-        FinalResult = Reply.final_result();
+        FinalResult = Reply.final_result().has_more();
         continue;
       }
       auto Response = ProtobufMarshaller->fromProtobuf(Reply.stream_result());
       if (!Response) {
         elog("Received invalid {0}: {1}. Reason: {2}",
              ReplyT::descriptor()->name(), Reply.stream_result().DebugString(),
              Response.takeError());
         ++FailedToParse;

clang-tools-extra/clangd/index/remote/Index.proto

 //===--- Index.proto - Remote index Protocol Buffers definition -----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 syntax = "proto2";
 
 package clang.clangd.remote;
 
+// Common final result for streaming requests.
+message FinalResult { optional bool has_more = 1; }
+
 message LookupRequest { repeated string ids = 1; }
 
 // The response is a stream of symbol messages and the terminating message
 // indicating the end of stream.
 message LookupReply {
   oneof kind {
     Symbol stream_result = 1;
-    bool final_result = 2;
+    FinalResult final_result = 2;
   }
 }
 
 message FuzzyFindRequest {
   optional string query = 1;
   repeated string scopes = 2;
   optional bool any_scope = 3;
   optional uint32 limit = 4;
   optional bool restricted_for_code_completion = 5;
   repeated string proximity_paths = 6;
   repeated string preferred_types = 7;
 }
 
 // The response is a stream of symbol messages, and one terminating has_more
 // message.
 message FuzzyFindReply {
   oneof kind {
     Symbol stream_result = 1;
-    bool final_result = 2; // HasMore
+    FinalResult final_result = 2;
   }
 }
 
 message RefsRequest {
   repeated string ids = 1;
   optional uint32 filter = 2;
   optional uint32 limit = 3;
 }
 
 // The response is a stream of reference messages, and one terminating has_more
 // message.
 message RefsReply {
   oneof kind {
     Ref stream_result = 1;
-    bool final_result = 2; // HasMore
+    FinalResult final_result = 2;
   }
 }
 
 message Symbol {
   optional string id = 1;
   optional SymbolInfo info = 2;
   optional string name = 3;
   optional SymbolLocation definition = 4;
message RelationsRequest {
   optional uint32 limit = 3;
 }
 
 // The response is a stream of reference messages, and one terminating has_more
 // message.
 message RelationsReply {
   oneof kind {
     Relation stream_result = 1;
-    bool final_result = 2; // HasMore
+    FinalResult final_result = 2;
   }
 }
 
 // This struct does not mirror clangd::Relation but rather the arguments of
 // SymbolIndex::relations callback.
 message Relation {
   optional string subject_id = 1;
   optional Symbol object = 2;
 }

clang-tools-extra/clangd/index/remote/server/Server.cpp

Index.lookup(*Req, [&](const clangd::Symbol &Item) {
         return;
       }
       LookupReply NextMessage;
       *NextMessage.mutable_stream_result() = *SerializedItem;
       Reply->Write(NextMessage);
       ++Sent;
     });
     LookupReply LastMessage;
-    LastMessage.set_final_result(true);
+    LastMessage.mutable_final_result()->set_has_more(true);
     Reply->Write(LastMessage);
     SPAN_ATTACH(Tracer, "Sent", Sent);
     SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
     return grpc::Status::OK;
   }
 
   grpc::Status FuzzyFind(grpc::ServerContext *Context,
                          const FuzzyFindRequest *Request,
bool HasMore = Index.fuzzyFind(*Req, [&](const clangd::Symbol &Item) {
         return;
       }
       FuzzyFindReply NextMessage;
       *NextMessage.mutable_stream_result() = *SerializedItem;
       Reply->Write(NextMessage);
       ++Sent;
     });
     FuzzyFindReply LastMessage;
-    LastMessage.set_final_result(HasMore);
+    LastMessage.mutable_final_result()->set_has_more(HasMore);
     Reply->Write(LastMessage);
     SPAN_ATTACH(Tracer, "Sent", Sent);
     SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
     return grpc::Status::OK;
   }
 
   grpc::Status Refs(grpc::ServerContext *Context, const RefsRequest *Request,
                     grpc::ServerWriter<RefsReply> *Reply) override {
bool HasMore = Index.refs(*Req, [&](const clangd::Ref &Item) {
         return;
       }
       RefsReply NextMessage;
       *NextMessage.mutable_stream_result() = *SerializedItem;
       Reply->Write(NextMessage);
       ++Sent;
     });
     RefsReply LastMessage;
-    LastMessage.set_final_result(HasMore);
+    LastMessage.mutable_final_result()->set_has_more(HasMore);
     Reply->Write(LastMessage);
     SPAN_ATTACH(Tracer, "Sent", Sent);
     SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
     return grpc::Status::OK;
   }
 
   grpc::Status Relations(grpc::ServerContext *Context,
                          const RelationsRequest *Request,
Index.relations(
             return;
           }
           RelationsReply NextMessage;
           *NextMessage.mutable_stream_result() = *SerializedItem;
           Reply->Write(NextMessage);
           ++Sent;
         });
     RelationsReply LastMessage;
-    LastMessage.set_final_result(true);
+    LastMessage.mutable_final_result()->set_has_more(true);
     Reply->Write(LastMessage);
     SPAN_ATTACH(Tracer, "Sent", Sent);
     SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
     return grpc::Status::OK;
   }
 
   std::unique_ptr<Marshaller> ProtobufMarshaller;
   clangd::SymbolIndex &Index;