Repository: tokio-rs/prost (language: rust)

Issue:
Question: Why are repeated field relations not added to the message graph?
I see that when building the message graph in the compiler code, repeated fields are ignored. Why?

https://github.com/danburkert/prost/blob/master/prost-build/src/message_graph.rs#L52



Discussion / hints:
That’s because the sole use of the message graph is detecting recursively
nested messages and co-recursively nested messages.   Because prost does
not box message fields, recursively nested messages would not compile in
Rust. To allow it, the message graph is used to detect the situation and
automatically box the recursive field.   Since repeated messages are
already put in a Vec, the extra indirection isn’t necessary.

On Fri, Feb 8, 2019 at 5:44 AM Timothy Hobbs <notifications@github.com>
wrote:

> I see that when building the message graph in the compiler code, repeated
> fields are ignored. Why?
>
>
> https://github.com/danburkert/prost/blob/master/prost-build/src/message_graph.rs#L52
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/danburkert/prost/issues/148>, or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAJx8vV0kvcdO_1IzxYcR_-EnuoKq_MJks5vLX9AgaJpZM4avu3i>
> .
>


Fix patch:
diff --git a/prost-build/src/message_graph.rs b/prost-build/src/message_graph.rs
index fea12d8e1..29b8dc7c0 100644
--- a/prost-build/src/message_graph.rs
+++ b/prost-build/src/message_graph.rs
@@ -43,6 +43,11 @@ impl MessageGraph {
         })
     }
 
+    /// Adds message to graph IFF it contains a non-repeated field containing another message.
+    /// The purpose of the message graph is detecting recursively nested messages and co-recursively nested messages.
+    /// Because prost does not box message fields, recursively nested messages would not compile in Rust.
+    /// To allow recursive messages, the message graph is used to detect recursion and automatically box the recursive field.
+    /// Since repeated messages are already put in a Vec, boxing them isn’t necessary even if the reference is recursive.
     fn add_message(&mut self, package: &str, msg: &DescriptorProto) {
         let msg_name = format!("{}.{}", package, msg.name.as_ref().unwrap());
         let msg_index = self.get_or_insert_index(msg_name.clone());


Test patch: