Skip to content

Commit

Permalink
fix(prost-build): Prevent repeated fields to be boxed (#1237)
Browse files Browse the repository at this point in the history
Repeated fields are stored in a `Vec`, and therefore they are already heap allocated.

BREAKING CHANGE: A repeated field that is manually marked as boxed was typed as `Vec<Box<T>>`. Those fields are now simply typed as `Vec<T>` to prevent double indirection. The `boxed` configuration is effectively ignored for repeated fields.
  • Loading branch information
caspermeijn authored Feb 18, 2025
1 parent bb81b3c commit 846c452
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 13 deletions.
15 changes: 5 additions & 10 deletions prost-build/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,12 @@ impl<'a> Context<'a> {
oneof: Option<&str>,
field: &FieldDescriptorProto,
) -> bool {
let repeated = field.label() == Label::Repeated;
if field.label() == Label::Repeated {
// Repeated field are stored in Vec, therefore it is already heap allocated
return false;
}
let fd_type = field.r#type();
if !repeated
&& (fd_type == Type::Message || fd_type == Type::Group)
if (fd_type == Type::Message || fd_type == Type::Group)
&& self
.message_graph
.is_nested(field.type_name(), fq_message_name)
Expand All @@ -161,13 +163,6 @@ impl<'a> Context<'a> {
.get_first_field(&config_path, field.name())
.is_some()
{
if repeated {
println!(
"cargo:warning=\
Field X is repeated and manually marked as boxed. \
This is deprecated and support will be removed in a later release"
);
}
return true;
}
false
Expand Down
1 change: 1 addition & 0 deletions tests/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ fn main() {
prost_build::Config::new()
.boxed("Foo.bar")
.boxed("Foo.oneof_field.box_qux")
.boxed("Foo.boxed_bar_list")
.compile_protos(&[src.join("boxed_field.proto")], includes)
.unwrap();

Expand Down
1 change: 1 addition & 0 deletions tests/src/boxed_field.proto
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ message Foo {
string baz = 2;
Bar box_qux = 3;
}
repeated Bar boxed_bar_list = 4;
}

message Bar {
Expand Down
9 changes: 6 additions & 3 deletions tests/src/boxed_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,18 @@ include!(concat!(env!("OUT_DIR"), "/boxed_field.rs"));
use self::foo::OneofField;

#[test]
/// Confirm `Foo::bar` and `OneofField::BoxQux` is boxed by creating an instance
/// - Confirm `Foo::bar` and `OneofField::BoxQux` is boxed by creating an instance
/// - `Foo::boxed_bar_list` should not be boxed as it is a `Vec`, therefore it is already heap allocated
fn test_boxed_field() {
use alloc::boxed::Box;
let _ = Foo {
use alloc::vec::Vec;
let foo = Foo {
bar: Some(Box::new(Bar {})),
oneof_field: Some(OneofField::BoxQux(Box::new(Bar {}))),
boxed_bar_list: Vec::from([Bar {}]),
};
let _ = Foo {
bar: Some(Box::new(Bar {})),
oneof_field: Some(OneofField::Baz("hello".into())),
..foo
};
}

0 comments on commit 846c452

Please sign in to comment.