Skip to content

Commit 5d70656

Browse files
committed
Fix generated code for mutually recursive messages with oneof fields
``` message A { oneof a { int32 i = 1; B b = 2; } } message B { oneof b { int32 i = 1; A a = 2; } } ```
1 parent c50ba4b commit 5d70656

File tree

6 files changed

+66
-14
lines changed

6 files changed

+66
-14
lines changed

protobuf-codegen/src/field.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,7 @@ impl<'a> FieldGen<'a> {
647647
}),
648648
}
649649
} else if let Some(oneof) = field.oneof() {
650-
FieldKind::Oneof(OneofField::parse(&oneof, &field, elem))
650+
FieldKind::Oneof(OneofField::parse(&oneof, &field, elem, root_scope))
651651
} else {
652652
let flag = if field.message.scope.file_scope.syntax() == Syntax::PROTO3
653653
&& field.field.get_field_type() != field_descriptor_proto::Type::TYPE_MESSAGE

protobuf-codegen/src/message.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use crate::serde;
2626
/// Message info for codegen
2727
pub(crate) struct MessageGen<'a> {
2828
pub message: &'a MessageWithScope<'a>,
29-
root_scope: &'a RootScope<'a>,
29+
pub root_scope: &'a RootScope<'a>,
3030
type_name: RustIdentWithPath,
3131
pub fields: Vec<FieldGen<'a>>,
3232
pub lite_runtime: bool,

protobuf-codegen/src/oneof.rs

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@ use crate::message::MessageGen;
1010
use crate::rust_name::{RustIdent, RustIdentWithPath, RustPath};
1111
use crate::rust_types_values::make_path;
1212
use crate::rust_types_values::RustType;
13-
use crate::scope::OneofWithContext;
1413
use crate::scope::WithScope;
1514
use crate::scope::{FieldWithContext, OneofVariantWithContext};
16-
use crate::serde;
15+
use crate::scope::{OneofWithContext, RootScope};
16+
use crate::{serde, ProtobufAbsolutePath};
1717
use protobuf::descriptor::field_descriptor_proto;
18+
use std::collections::HashSet;
1819

1920
// oneof one { ... }
2021
#[derive(Clone)]
@@ -27,17 +28,39 @@ pub(crate) struct OneofField<'a> {
2728
}
2829

2930
impl<'a> OneofField<'a> {
31+
// Detecting recursion: if oneof fields contains a self-reference
32+
// or another message which has a reference to self,
33+
// put oneof variant into a box.
34+
fn need_boxed(
35+
field: &FieldWithContext,
36+
root_scope: &RootScope,
37+
owner_name: &ProtobufAbsolutePath,
38+
) -> bool {
39+
let mut visited_messages = HashSet::new();
40+
let mut fields = vec![field.clone()];
41+
while let Some(field) = fields.pop() {
42+
if field.field.get_field_type() == field_descriptor_proto::Type::TYPE_MESSAGE {
43+
let message_name = ProtobufAbsolutePath::from(field.field.get_type_name());
44+
if !visited_messages.insert(message_name.clone()) {
45+
continue;
46+
}
47+
if message_name == *owner_name {
48+
return true;
49+
}
50+
let message = root_scope.find_message(&message_name);
51+
fields.extend(message.fields().into_iter().filter(|f| f.is_oneof()));
52+
}
53+
}
54+
false
55+
}
56+
3057
pub fn parse(
3158
oneof: &OneofWithContext<'a>,
3259
field: &FieldWithContext<'a>,
3360
elem: FieldElem<'a>,
61+
root_scope: &RootScope,
3462
) -> OneofField<'a> {
35-
// detecting recursion
36-
let boxed = if let &FieldElem::Message(ref m) = &elem {
37-
m.message.name_absolute() == oneof.message.name_absolute()
38-
} else {
39-
false
40-
};
63+
let boxed = OneofField::need_boxed(field, root_scope, &oneof.message.name_absolute());
4164

4265
OneofField {
4366
elem,
@@ -83,6 +106,7 @@ impl<'a> OneofVariantGen<'a> {
83106
oneof: &'a OneofGen<'a>,
84107
variant: OneofVariantWithContext<'a>,
85108
field: &'a FieldGen,
109+
root_scope: &RootScope,
86110
) -> OneofVariantGen<'a> {
87111
OneofVariantGen {
88112
oneof,
@@ -101,7 +125,12 @@ impl<'a> OneofVariantGen<'a> {
101125
),
102126
field.rust_name
103127
),
104-
oneof_field: OneofField::parse(variant.oneof, &field.proto_field, field.elem().clone()),
128+
oneof_field: OneofField::parse(
129+
variant.oneof,
130+
&field.proto_field,
131+
field.elem().clone(),
132+
oneof.message.root_scope,
133+
),
105134
}
106135
}
107136

@@ -160,7 +189,12 @@ impl<'a> OneofGen<'a> {
160189
.expect(&format!("field not found by name: {}", v.field.get_name()));
161190
match field.proto_type {
162191
field_descriptor_proto::Type::TYPE_GROUP => None,
163-
_ => Some(OneofVariantGen::parse(self, v, field)),
192+
_ => Some(OneofVariantGen::parse(
193+
self,
194+
v,
195+
field,
196+
self.message.root_scope,
197+
)),
164198
}
165199
})
166200
.collect()

protobuf-codegen/src/protobuf_name.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ mod relative_path_test {
231231
}
232232
}
233233

234-
#[derive(Clone, Eq, PartialEq, Debug)]
234+
#[derive(Clone, Eq, PartialEq, Debug, Hash)]
235235
pub struct ProtobufAbsolutePath {
236236
pub path: String,
237237
}

protobuf-codegen/src/scope.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ impl<'a> RootScope<'a> {
4343
}
4444

4545
// find message by fully qualified name
46-
pub fn _find_message(&'a self, fqn: &ProtobufAbsolutePath) -> MessageWithScope<'a> {
46+
pub fn find_message(&'a self, fqn: &ProtobufAbsolutePath) -> MessageWithScope<'a> {
4747
match self.find_message_or_enum(fqn) {
4848
MessageOrEnumWithScope::Message(m) => m,
4949
_ => panic!("not a message: {}", fqn),

protobuf-test/src/common/v2/test_oneof_recursive_pb.proto

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,21 @@ message LinkedList {
88
LinkedList node = 2;
99
}
1010
}
11+
12+
message RecursiveA {
13+
oneof x {
14+
RecursiveB b = 1;
15+
}
16+
}
17+
18+
message RecursiveB {
19+
oneof x {
20+
RecursiveC c = 1;
21+
}
22+
}
23+
24+
message RecursiveC {
25+
oneof x {
26+
RecursiveA a = 1;
27+
}
28+
}

0 commit comments

Comments
 (0)