Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default to packed for repeated primitives in proto3. #707

Merged
merged 4 commits into from
Jun 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 37 additions & 5 deletions protobuf-codegen/src/gen/field/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@

// Representation of map entry: key type and value type
#[derive(Clone, Debug)]
pub struct EntryKeyValue<'a>(FieldElem<'a>, FieldElem<'a>);

Check warning on line 122 in protobuf-codegen/src/gen/field/mod.rs

View workflow job for this annotation

GitHub Actions / linux beta (default features)

fields `0` and `1` are never read

#[derive(Clone)]
pub(crate) struct FieldGen<'a> {
Expand Down Expand Up @@ -185,11 +185,43 @@
}
RuntimeFieldType::Repeated(..) => {
let elem = field_elem(&field, root_scope, &customize);

FieldKind::Repeated(RepeatedField {
elem,
packed: field.field.proto().options.get_or_default().packed(),
})
let primitive = match field.field.proto().type_() {
Type::TYPE_DOUBLE
| Type::TYPE_FLOAT
| Type::TYPE_INT64
| Type::TYPE_UINT64
| Type::TYPE_INT32
| Type::TYPE_FIXED64
| Type::TYPE_FIXED32
| Type::TYPE_BOOL
| Type::TYPE_UINT32
| Type::TYPE_SFIXED32
| Type::TYPE_SFIXED64
| Type::TYPE_SINT32
| Type::TYPE_SINT64
| Type::TYPE_ENUM => true,
Type::TYPE_STRING
| Type::TYPE_GROUP
| Type::TYPE_MESSAGE
| Type::TYPE_BYTES => false,
};
let packed = field
.field
.proto()
.options
.get_or_default()
.packed
.unwrap_or(match field.message.scope.file_scope.syntax() {
Syntax::Proto2 => false,
// in proto3, repeated primitive types are packed by default
Syntax::Proto3 => primitive,
});
if packed && !primitive {
anyhow::bail!(
"[packed = true] can only be specified for repeated primitive fields"
);
}
FieldKind::Repeated(RepeatedField { elem, packed })
}
RuntimeFieldType::Singular(..) => {
let elem = field_elem(&field, root_scope, &customize);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use protobuf::reflect::Syntax;
use protobuf::MessageFull;
use protobuf_test_common::*;

use super::test_repeated_packed_pb::*;
Expand Down Expand Up @@ -83,3 +85,16 @@ fn test_issue_281() {
test.values = (0..100).collect();
test_serialize_deserialize_no_hex(&test);
}

#[test]
fn test_write_packed_default() {
let mut test = TestPackedDefault::new();
test.varints = vec![0, 1, 2, 3, 4, 5];

// Proto3 packs primitives by default, proto2 does not.
let expected_hex = match TestPackedDefault::descriptor().file_descriptor().syntax() {
Syntax::Proto2 => "08 00 08 01 08 02 08 03 08 04 08 05",
Syntax::Proto3 => "0a 06 00 01 02 03 04 05",
};
test_serialize_deserialize(expected_hex, &test);
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@ message TestUnpacked {
message TestIssue281 {
repeated fixed32 values = 1 [packed=true];
}

message TestPackedDefault {
repeated uint32 varints = 1;
}
Loading