diff --git a/.changes/unreleased/added-20261004-map-entries-omit-defaults.yaml b/.changes/unreleased/added-20261004-map-entries-omit-defaults.yaml new file mode 100644 index 00000000..4849db9e --- /dev/null +++ b/.changes/unreleased/added-20261004-map-entries-omit-defaults.yaml @@ -0,0 +1,4 @@ +kind: Added +body: |- + **Generated messages can encode map entries as prost does** (#562). The option is `buffa_build::Config::map_entries_omit_defaults`, `map_entries_omit_defaults=true` in `protoc-gen-buffa`, and `CodeGenConfig::map_entries_omit_defaults`; it is off by default. A key or value at its default (zero, `false`, empty, an open enum's 0, a closed enum's first value, or a message value that encodes to nothing) is left out of the entry by the owned, view and lazy view encoders; the entry itself is kept. Decoding is unchanged; as with prost, a `-0.0` float value is left out and so decodes as `0.0`. A message value that equals its default but still encodes a field, such as a `required` field at its default, is written, where prost leaves it out. For maps with several entries to come out in prost's `btree_map` order, also use `map_type(MapRepr::BTreeMap)`. Each crate's types encode as that crate was generated; the well-known types and `DynamicMessage` write both fields, and under `CodecStrategy::Table` messages with a map field keep the unrolled codec, and a `codec_strategy_in` rule that selects the table for such a message by its exact path is an error. +time: 2026-10-04T21:00:00.000000000+00:00 diff --git a/buffa-build/src/lib.rs b/buffa-build/src/lib.rs index 07e0e5da..f268c6fa 100644 --- a/buffa-build/src/lib.rs +++ b/buffa-build/src/lib.rs @@ -1490,6 +1490,61 @@ impl Config { self } + /// Encode map entries as prost does: leave an entry's key or value out + /// when it holds the default (default: false, both are always written, as + /// the C++ and Java runtimes write them). Entries themselves are never + /// dropped. + /// + /// For migrations from prost where stored or hashed map encodings must + /// not change. Decoding is unchanged: buffa, like every protobuf runtime, + /// reads both forms to the same map. One value does not survive the round + /// trip: a `-0.0` float value is left out, as prost leaves it out, and + /// decodes as `0.0`. + /// + /// The default is what a missing field decodes to: zero, `false`, empty, + /// an open enum's 0 or a closed enum's first value. A message value is + /// left out when it encodes to nothing. + /// + /// # Differences from prost + /// + /// - prost leaves out a message value equal to its default. A value that + /// equals its default and still encodes a field, such as one with a + /// `required` field at its default or a float field holding `-0.0`, is + /// written here and not by prost. + /// - A value of an enum made open with + /// [`open_enums_in`](Self::open_enums_in) is left out at 0, which a + /// missing open enum decodes to, where prost leaves out the enum's + /// first value. The two differ only for an enum whose first value is + /// not 0. protoc rejects that as a map value, so the case can occur + /// only with a descriptor set that another tool built + /// ([`use_buf`](Self::use_buf), [`descriptor_set`](Self::descriptor_set)). + /// - The option covers the entries' bytes only. For a map with more than + /// one entry to encode identically, the entries must also come in the + /// same order: use [`map_type`](Self::map_type) with + /// [`MapRepr::BTreeMap`] where prost used `btree_map`. + /// + /// # Scope + /// + /// - The option applies to the owned, view and lazy view encoders of the + /// messages generated in this run. Each crate's types encode as that + /// crate was generated. + /// - `google.protobuf.Struct`, the well-known type with a map field, + /// comes from `buffa-types` and writes both fields, and so does + /// `DynamicMessage`. + /// - A lazy view keeps its singular and repeated message fields undecoded + /// and writes them as they were read, so map entries inside them keep + /// the encoding they arrived with. Every encoder writes entries kept as + /// unknown fields as they were read. + /// - With [`CodecStrategy::Table`], messages with a map field keep the + /// unrolled codec, and a + /// [`codec_strategy_in`](Self::codec_strategy_in) rule that selects the + /// table for such a message by its exact path is an error. + #[must_use] + pub fn map_entries_omit_defaults(mut self, enabled: bool) -> Self { + self.codegen_config.map_entries_omit_defaults = enabled; + self + } + /// Map the matching `map` fields to a custom collection implementing /// `buffa::map_codec::MapStorage`, named by its fully-qualified Rust path /// (e.g. `"::my_crate::OrderedMap"`). The path must **not** include the diff --git a/buffa-codegen/src/impl_message.rs b/buffa-codegen/src/impl_message.rs index 9a8bb4d7..82beeb62 100644 --- a/buffa-codegen/src/impl_message.rs +++ b/buffa-codegen/src/impl_message.rs @@ -3209,6 +3209,26 @@ fn map_element_encode_stmt(ty: Type, tag_num: u32, var: &Ident) -> TokenStream { quote! { #tag #payload } } +/// Whether a view's map entry element bound to `var` is written, that is not +/// its type's default, which `map_entries_omit_defaults` leaves out of the +/// entry, as `MapCodec::is_default` decides for owned messages. +fn map_element_is_written_expr(ty: Type, closed_enum: bool, var: &Ident) -> TokenStream { + match ty { + Type::TYPE_STRING | Type::TYPE_BYTES => quote! { !#var.is_empty() }, + // A closed enum's default is its first value, which need not be 0. + Type::TYPE_ENUM if closed_enum => quote! { + !<::buffa::map_codec::ClosedEnum<_> as ::buffa::map_codec::MapCodec>::is_default(#var) + }, + Type::TYPE_ENUM => quote! { #var.to_i32() != 0 }, + Type::TYPE_BOOL => quote! { *#var }, + Type::TYPE_FLOAT | Type::TYPE_DOUBLE => quote! { *#var != 0.0 }, + Type::TYPE_MESSAGE => { + unreachable!("message map values are handled per-phase by callers") + } + _ => quote! { *#var != 0 }, + } +} + fn map_view_compute_size_stmt( ctx: &CodeGenContext, msg: &DescriptorProto, @@ -3225,14 +3245,40 @@ fn map_view_compute_size_stmt( let (key_fd, val_fd) = find_map_entry_fields(msg, field)?; let key_ty = effective_type_in_map_entry(ctx, key_fd, features); let val_ty = effective_type_in_map_entry(ctx, val_fd, features); + let val_closed_enum = val_ty == Type::TYPE_ENUM + && is_closed_enum(&crate::features::resolve_field(ctx, val_fd, features)); let key_tag_len = tag_encoded_len(1, wire_type_byte(key_ty)); let val_tag_len = tag_encoded_len(2, wire_type_byte(val_ty)); let k = format_ident!("k"); let v = format_ident!("v"); let key_size = map_element_size_expr(key_ty, &k); - let val_size = if val_ty == Type::TYPE_MESSAGE { + let omit_defaults = ctx.config.map_entries_omit_defaults; + let key_part = if omit_defaults { + let written = map_element_is_written_expr(key_ty, false, &k); + quote! { (if #written { #key_tag_len + #key_size } else { 0 }) } + } else { + quote! { #key_tag_len + #key_size } + }; + let val_part = if val_ty == Type::TYPE_MESSAGE && omit_defaults { + // Both passes visit every value, so the slots stay in step when an + // empty one is left out. quote! { { + let __slot = __cache.reserve(); + let inner = #v.compute_size(__cache); + __cache.set(__slot, inner); + if inner == 0 { + 0 + } else { + #val_tag_len + + ::buffa::encoding::varint_len(inner as u64) as u64 + + inner as u64 + } + } + } + } else if val_ty == Type::TYPE_MESSAGE { + quote! { + #val_tag_len + { let __slot = __cache.reserve(); let inner = #v.compute_size(__cache); __cache.set(__slot, inner); @@ -3240,28 +3286,37 @@ fn map_view_compute_size_stmt( } } } else { - map_element_size_expr(val_ty, &v) + let val_size = map_element_size_expr(val_ty, &v); + if omit_defaults { + let written = map_element_is_written_expr(val_ty, val_closed_enum, &v); + quote! { (if #written { #val_tag_len + #val_size } else { 0 }) } + } else { + quote! { #val_tag_len + #val_size } + } }; // Both passes iterate `for (k, v) in &self.#ident`, identical to // `map_view_write_to_stmt`, so SizeCache slot order matches by construction. // When both key and value are fixed-width (no cache slots reserved) the - // entry size is constant and we fold to `len() * const`. - if map_element_size_is_constant(key_ty) && map_element_size_is_constant(val_ty) { + // entry size is constant and we fold to `len() * const`, unless defaults + // are left out, which makes it depend on the values. + let key_constant = map_element_size_is_constant(key_ty) && !omit_defaults; + let val_constant = map_element_size_is_constant(val_ty) && !omit_defaults; + if key_constant && val_constant { return Ok(quote! { { - let entry_size: u64 = #key_tag_len + #key_size + #val_tag_len + #val_size; + let entry_size: u64 = #key_part + #val_part; size += self.#ident.len() as u64 * (#outer_tag_len + ::buffa::encoding::varint_len(entry_size) as u64 + entry_size); } }); } - let k_bind = if map_element_size_is_constant(key_ty) { + let k_bind = if key_constant { format_ident!("_{}", k) } else { k }; - let v_bind = if map_element_size_is_constant(val_ty) { + let v_bind = if val_constant { format_ident!("_{}", v) } else { v @@ -3269,7 +3324,7 @@ fn map_view_compute_size_stmt( Ok(quote! { #[allow(clippy::for_kv_map)] for (#k_bind, #v_bind) in &self.#ident { - let entry_size: u64 = #key_tag_len + #key_size + #val_tag_len + #val_size; + let entry_size: u64 = #key_part + #val_part; size += #outer_tag_len + ::buffa::encoding::varint_len(entry_size) as u64 + entry_size; @@ -3292,6 +3347,8 @@ fn map_view_write_to_stmt( let (key_fd, val_fd) = find_map_entry_fields(msg, field)?; let key_ty = effective_type_in_map_entry(ctx, key_fd, features); let val_ty = effective_type_in_map_entry(ctx, val_fd, features); + let val_closed_enum = val_ty == Type::TYPE_ENUM + && is_closed_enum(&crate::features::resolve_field(ctx, val_fd, features)); let key_tag_len = tag_encoded_len(1, wire_type_byte(key_ty)); let val_tag_len = tag_encoded_len(2, wire_type_byte(val_ty)); let k = format_ident!("k"); @@ -3305,12 +3362,40 @@ fn map_view_write_to_stmt( } else { (quote! {}, map_element_size_expr(val_ty, &v)) }; - let encode_key = map_element_encode_stmt(key_ty, 1, &k); - let encode_val = map_element_encode_stmt(val_ty, 2, &v); + let mut encode_key = map_element_encode_stmt(key_ty, 1, &k); + let mut encode_val = map_element_encode_stmt(val_ty, 2, &v); + let mut key_part = quote! { #key_tag_len + #key_size }; + let mut val_part = quote! { #val_tag_len + #val_size }; + if ctx.config.map_entries_omit_defaults { + let key_written = map_element_is_written_expr(key_ty, false, &k); + key_part = quote! { (if #key_written { #key_part } else { 0 }) }; + encode_key = quote! { if #key_written { #encode_key } }; + if val_ty == Type::TYPE_MESSAGE { + // A value that encodes to nothing loses its tag and length; its + // `write_to` still runs, writing nothing but consuming the slots + // its `compute_size` reserved. + val_part = quote! { (if __v_len == 0 { 0 } else { #val_part }) }; + let val_tag = quote! { + ::buffa::encoding::Tag::new(2, ::buffa::encoding::WireType::LengthDelimited) + .encode(buf); + }; + encode_val = quote! { + if __v_len != 0 { + #val_tag + ::buffa::encoding::encode_varint(__v_len as u64, buf); + } + #v.write_to(__cache, buf); + }; + } else { + let val_written = map_element_is_written_expr(val_ty, val_closed_enum, &v); + val_part = quote! { (if #val_written { #val_part } else { 0 }) }; + encode_val = quote! { if #val_written { #encode_val } }; + } + } Ok(quote! { for (#k, #v) in &self.#ident { #val_len_bind - let entry_size: u64 = #key_tag_len + #key_size + #val_tag_len + #val_size; + let entry_size: u64 = #key_part + #val_part; ::buffa::encoding::Tag::new( #field_number, ::buffa::encoding::WireType::LengthDelimited, @@ -3322,6 +3407,33 @@ fn map_view_write_to_stmt( }) } +/// The `map_codec` functions the owned size and write passes call, picked +/// together so that the two passes cannot disagree on leaving defaults out. +struct MapEncoders { + field_len: TokenStream, + write_field: TokenStream, + message_field_len: TokenStream, + write_message_field: TokenStream, +} + +fn map_encoders(ctx: &CodeGenContext) -> MapEncoders { + if ctx.config.map_entries_omit_defaults { + MapEncoders { + field_len: quote! { field_len_omitting_defaults }, + write_field: quote! { write_field_omitting_defaults }, + message_field_len: quote! { message_field_len_omitting_defaults }, + write_message_field: quote! { write_message_field_omitting_defaults }, + } + } else { + MapEncoders { + field_len: quote! { field_len }, + write_field: quote! { write_field }, + message_field_len: quote! { message_field_len }, + write_message_field: quote! { write_message_field }, + } + } +} + fn map_compute_size_stmt( ctx: &CodeGenContext, msg: &DescriptorProto, @@ -3340,9 +3452,14 @@ fn map_compute_size_stmt( // Message values are two-pass (SizeCache slot per entry, consumed by // `map_write_to_stmt` in identical iteration order — both helpers // iterate the same map, so slot order matches by construction). + let MapEncoders { + field_len, + message_field_len, + .. + } = map_encoders(ctx); if m.val_ty == Type::TYPE_MESSAGE { return Ok(quote! { - size += ::buffa::map_codec::message_field_len::<#key_codec, _, _>( + size += ::buffa::map_codec::#message_field_len::<#key_codec, _, _>( &self.#ident, #outer_tag_len, __cache, @@ -3350,7 +3467,7 @@ fn map_compute_size_stmt( }); } Ok(quote! { - size += ::buffa::map_codec::field_len::<#key_codec, #val_codec, _>( + size += ::buffa::map_codec::#field_len::<#key_codec, #val_codec, _>( &self.#ident, #outer_tag_len, ); @@ -3372,9 +3489,14 @@ fn map_write_to_stmt( val_codec, .. } = &m; + let MapEncoders { + write_field, + write_message_field, + .. + } = map_encoders(ctx); if m.val_ty == Type::TYPE_MESSAGE { return Ok(quote! { - ::buffa::map_codec::write_message_field::<#key_codec, _, _>( + ::buffa::map_codec::#write_message_field::<#key_codec, _, _>( &self.#ident, #field_number, __cache, @@ -3383,7 +3505,7 @@ fn map_write_to_stmt( }); } Ok(quote! { - ::buffa::map_codec::write_field::<#key_codec, #val_codec, _>( + ::buffa::map_codec::#write_field::<#key_codec, #val_codec, _>( &self.#ident, #field_number, buf, diff --git a/buffa-codegen/src/lib.rs b/buffa-codegen/src/lib.rs index b9d6d576..530f72ae 100644 --- a/buffa-codegen/src/lib.rs +++ b/buffa-codegen/src/lib.rs @@ -1099,6 +1099,8 @@ pub enum CodecStrategy { /// - a message that uses the `MessageSet` wire format; /// - a message with extension ranges, when JSON code is generated and /// unknown fields are preserved; + /// - a message with a `map` field, when `map_entries_omit_defaults` is + /// set: the table writes both fields of every entry; /// - a message with a field of a non-default string, bytes, or collection /// type, such as `use_bytes_type`, `string_type`, `bytes_type`, /// `repeated_type`, and a custom `map_type` select. `HashMap` and @@ -1632,6 +1634,27 @@ pub struct CodeGenConfig { /// /// Defaults to `true`. pub generate_with_setters: bool, + /// Encode map entries as prost does: leave an entry's key or value out + /// when it holds what a missing field decodes to, and a message value when + /// it encodes to nothing. + /// + /// Off, every entry carries both fields, as the C++ and Java runtimes + /// write them. Decoding reads either; a `-0.0` float value is left out + /// and so decodes as `0.0`. prost leaves out a message value equal to its + /// default, so a value that still encodes a field, such as one with a + /// `required` field at its default, is written here and not by prost. + /// + /// Applies to the owned, view and lazy view encoders of the messages + /// generated in this run: types generated without it, the well-known + /// types and `DynamicMessage` write both fields. With + /// [`CodecStrategy::Table`], messages with a map field keep the unrolled + /// codec, and a [`codec_strategy_in`](Self::codec_strategy_in) rule that + /// selects the table for such a message by its exact path is a + /// [`CodeGenError`]. `buffa_build::Config::map_entries_omit_defaults` has + /// the details. + /// + /// Defaults to `false`. + pub map_entries_omit_defaults: bool, /// Generate `impl Reflectable` for owned message types (bridge mode). /// /// When enabled, each generated message gets an @@ -2025,6 +2048,7 @@ impl Default for CodeGenConfig { oneof_struct_field_attributes: Vec::new(), gate_impls_on_crate_features: false, generate_with_setters: true, + map_entries_omit_defaults: false, generate_reflection: false, generate_reflection_vtable: false, shared_descriptor_pool: false, diff --git a/buffa-codegen/src/table_plan.rs b/buffa-codegen/src/table_plan.rs index 21963e06..69747014 100644 --- a/buffa-codegen/src/table_plan.rs +++ b/buffa-codegen/src/table_plan.rs @@ -276,6 +276,13 @@ fn map_field<'a>( features: &ResolvedFeatures, ) -> Result, Ineligible> { let name = f.name.as_deref().unwrap_or(""); + // The table writes both fields of every entry. + if ctx.config.map_entries_omit_defaults { + return Err(ineligible( + "has a map field and map_entries_omit_defaults is set", + format!("map_entries_omit_defaults is set and it has map field `{name}`"), + )); + } let field_fqn = format!("{fqn}.{name}"); if matches!(ctx.map_repr(&field_fqn), crate::MapRepr::Custom(_)) { return Err(ineligible( diff --git a/buffa-codegen/src/tests/size_arithmetic.rs b/buffa-codegen/src/tests/size_arithmetic.rs index e8259464..7bee9eac 100644 --- a/buffa-codegen/src/tests/size_arithmetic.rs +++ b/buffa-codegen/src/tests/size_arithmetic.rs @@ -157,6 +157,24 @@ fn corpus_output() -> &'static str { }) } +/// The corpus generated with `map_entries_omit_defaults`. +fn sparse_corpus_output() -> &'static str { + static OUTPUT: std::sync::OnceLock = std::sync::OnceLock::new(); + OUTPUT.get_or_init(|| { + let config = CodeGenConfig { + map_entries_omit_defaults: true, + ..CodeGenConfig::default() + }; + let files = generate( + &[size_corpus_file()], + &["size_corpus.proto".to_string()], + &config, + ) + .expect("corpus should generate"); + joined(&files) + }) +} + /// Extract the bodies of every `fn` whose name starts with `prefix` /// (matching `fn compute_size` and `fn write_to` in owned, view, and lazy /// impls) by brace counting from the signature. @@ -227,25 +245,62 @@ fn no_u32_arithmetic_in_size_paths() { // arithmetic inside these bodies is a wrap bug (see module docs); // u32 uses elsewhere in generated code (decode paths, accessors) are // out of scope by construction. - let content = corpus_output(); - let bodies: Vec<&str> = fn_bodies(content, "compute_size") - .into_iter() - .chain(fn_bodies(content, "write_to")) - .collect(); - assert!( - bodies.len() >= 6, - "corpus should produce compute_size/write_to bodies for owned and \ - view impls, got {}", - bodies.len() - ); - for body in bodies { + for content in [corpus_output(), sparse_corpus_output()] { + let bodies: Vec<&str> = fn_bodies(content, "compute_size") + .into_iter() + .chain(fn_bodies(content, "write_to")) + .collect(); assert!( - !body.contains("as u32"), - "u32 cast reintroduced in a size path: {body}" + bodies.len() >= 6, + "corpus should produce compute_size/write_to bodies for owned and \ + view impls, got {}", + bodies.len() ); + for body in bodies { + assert!( + !body.contains("as u32"), + "u32 cast reintroduced in a size path: {body}" + ); + assert!( + !body.contains("sum::()"), + "u32 sum reintroduced in a size path: {body}" + ); + } + } +} + +/// `map_entries_omit_defaults` switches both map shapes, message- and +/// scalar-valued, and both owned passes to the encoders that leave default +/// entry fields out. +#[test] +fn map_entries_omit_defaults_selects_the_sparse_encoders() { + let dense = [ + "map_codec::field_len::<", + "map_codec::write_field::<", + "map_codec::message_field_len::<", + "map_codec::write_message_field::<", + ]; + let sparse = [ + "map_codec::field_len_omitting_defaults::<", + "map_codec::write_field_omitting_defaults::<", + "map_codec::message_field_len_omitting_defaults::<", + "map_codec::write_message_field_omitting_defaults::<", + ]; + let content = corpus_output(); + for name in dense { assert!( - !body.contains("sum::()"), - "u32 sum reintroduced in a size path: {body}" + content.contains(name), + "{name} without the option: {content}" ); } + for name in sparse { + assert!(!content.contains(name), "{name} without the option"); + } + let content = sparse_corpus_output(); + for name in sparse { + assert!(content.contains(name), "{name} with the option: {content}"); + } + for name in dense { + assert!(!content.contains(name), "{name} with the option"); + } } diff --git a/buffa-codegen/src/tests/table_codec.rs b/buffa-codegen/src/tests/table_codec.rs index 07f7e198..68979cb9 100644 --- a/buffa-codegen/src/tests/table_codec.rs +++ b/buffa-codegen/src/tests/table_codec.rs @@ -1566,6 +1566,38 @@ fn a_map_with_a_custom_collection_or_element_type_falls_back() { } } +#[test] +fn a_message_with_maps_falls_back_when_map_entries_omit_defaults() { + let config = CodeGenConfig { + map_entries_omit_defaults: true, + ..table_config(CodecStrategy::Table) + }; + let (code, warnings) = run_maps(&config); + assert!(!tables(&code).contains(&"Maps".to_string()), "{code}"); + assert!(tables(&code).contains(&"Leaf".to_string()), "{code}"); + let (_, reasons) = summary(&warnings); + assert_eq!( + reasons, + [("has a map field and map_entries_omit_defaults is set", 1)] + ); + assert!( + code.contains("map_codec::field_len_omitting_defaults::<"), + "{code}" + ); + // A rule that names the message asks for the impossible. + let config = CodeGenConfig { + codec_strategy_in: vec![(".t.Maps".to_string(), CodecStrategy::Table)], + ..config + }; + let err = generate_with_diagnostics(&[map_schema()], &["t.proto".to_string()], &config) + .unwrap_err() + .to_string(); + assert!( + err.contains("map_entries_omit_defaults is set and it has map field `counts`"), + "{err}" + ); +} + #[test] fn a_map_of_messages_without_a_table_is_reached_through_the_message_impl() { // `Leaf` is unrolled by a rule, so the map's values have no table here. diff --git a/buffa-codegen/src/tests/view_codegen.rs b/buffa-codegen/src/tests/view_codegen.rs index 89260aef..7a92b796 100644 --- a/buffa-codegen/src/tests/view_codegen.rs +++ b/buffa-codegen/src/tests/view_codegen.rs @@ -649,3 +649,75 @@ fn test_view_oneof_with_message_variant() { "message-type oneof variant must check recursion depth: {content}" ); } + +/// Under `map_entries_omit_defaults`, a view leaves a closed enum map value +/// out at the enum's default, its first value, and an open one at 0, as a +/// missing value decodes to those. +#[test] +fn map_entries_omit_defaults_compares_view_enum_values_with_their_default() { + let entry = |name: &str, value_type: &str| DescriptorProto { + name: Some(name.to_string()), + field: vec![ + make_field("key", 1, Label::LABEL_OPTIONAL, Type::TYPE_INT32), + FieldDescriptorProto { + type_name: Some(value_type.to_string()), + ..make_field("value", 2, Label::LABEL_OPTIONAL, Type::TYPE_ENUM) + }, + ], + options: Some(MessageOptions { + map_entry: Some(true), + ..Default::default() + }) + .into(), + ..Default::default() + }; + let map = |name: &str, number: i32, entry_type: &str| FieldDescriptorProto { + type_name: Some(entry_type.to_string()), + ..make_field(name, number, Label::LABEL_REPEATED, Type::TYPE_MESSAGE) + }; + // `A = 1` first: protoc rejects this as a map value, and a descriptor + // built another way can have it. + let first_is_one = |name: &str| EnumDescriptorProto { + name: Some(name.to_string()), + value: vec![enum_value("A", 1), enum_value("B", 0)], + ..Default::default() + }; + let file = FileDescriptorProto { + name: Some("enum_maps.proto".to_string()), + package: Some("em".to_string()), + syntax: Some("proto2".to_string()), + enum_type: vec![first_is_one("Closed"), first_is_one("Opened")], + message_type: vec![DescriptorProto { + name: Some("Maps".to_string()), + field: vec![ + map("closed", 1, ".em.Maps.ClosedEntry"), + map("opened", 2, ".em.Maps.OpenedEntry"), + ], + nested_type: vec![ + entry("ClosedEntry", ".em.Closed"), + entry("OpenedEntry", ".em.Opened"), + ], + ..Default::default() + }], + ..Default::default() + }; + let config = CodeGenConfig { + map_entries_omit_defaults: true, + feature_overrides: open_enum_overrides(&[".em.Opened"]), + ..CodeGenConfig::default() + }; + let files = + generate(&[file], &["enum_maps.proto".to_string()], &config).expect("should generate"); + // Without whitespace and the trailing commas prettyplease adds. + let view: String = files + .iter() + .filter(|f| f.name.contains("__view")) + .flat_map(|f| f.content.split_whitespace()) + .collect::() + .replace(",>", ">"); + // In the size pass, and twice in the write pass: the length and the field. + let closed = + "!<::buffa::map_codec::ClosedEnum<_>as::buffa::map_codec::MapCodec>::is_default(v)"; + assert_eq!(view.matches(closed).count(), 3, "{view}"); + assert_eq!(view.matches("v.to_i32()!=0").count(), 3, "{view}"); +} diff --git a/buffa-test/build.rs b/buffa-test/build.rs index 06cecd32..47f53e52 100644 --- a/buffa-test/build.rs +++ b/buffa-test/build.rs @@ -529,6 +529,20 @@ fn main() { .compile() .expect("buffa_build failed for map_type.proto"); + // map_omit_defaults: entries encoded as prost writes them, by the owned, + // view and lazy view encoders; BTreeMap so the tests can pin the bytes. + buffa_build::Config::new() + .files(&[ + "protos/map_omit_defaults.proto", + "protos/map_omit_defaults_proto2.proto", + ]) + .includes(&["protos/"]) + .map_type(buffa_build::MapRepr::BTreeMap) + .map_entries_omit_defaults(true) + .lazy_views(true) + .compile() + .expect("buffa_build failed for map_omit_defaults.proto"); + // map_type_custom: a crate-LOCAL `CustomMap` newtype (a `MapStorage` // impl wrapping BTreeMap) used for every `map` field, via the // `map_type_custom` knob. Exercises the `MapRepr::Custom` codegen path and diff --git a/buffa-test/protos/map_omit_defaults.proto b/buffa-test/protos/map_omit_defaults.proto new file mode 100644 index 00000000..9c47b7a2 --- /dev/null +++ b/buffa-test/protos/map_omit_defaults.proto @@ -0,0 +1,29 @@ +syntax = "proto3"; + +package map_omit_defaults; + +// Compiled with `map_entries_omit_defaults(true)`: entry keys and values at +// their default are left out, as prost writes maps. +message Maps { + enum Color { + COLOR_UNSPECIFIED = 0; + COLOR_RED = 1; + } + + message Leaf { + int32 n = 1; + } + + map counts = 1; + map names = 2; + map flags = 3; + map blobs = 4; + map colors = 5; + map leaves = 6; + map floats = 7; +} + +// Holds `Maps` as a message field, which a lazy view keeps undecoded. +message Holder { + Maps maps = 1; +} diff --git a/buffa-test/protos/map_omit_defaults_proto2.proto b/buffa-test/protos/map_omit_defaults_proto2.proto new file mode 100644 index 00000000..d7f5de62 --- /dev/null +++ b/buffa-test/protos/map_omit_defaults_proto2.proto @@ -0,0 +1,19 @@ +syntax = "proto2"; + +package map_omit_defaults_proto2; + +// Compiled as `map_omit_defaults.proto` is. +message Maps { + // Closed. protoc requires a map value's enum to declare 0 first. + enum Size { + SIZE_SMALL = 0; + SIZE_LARGE = 1; + } + + message Required { + required int32 n = 1; + } + + map sizes = 1; + map required = 2; +} diff --git a/buffa-test/src/lib.rs b/buffa-test/src/lib.rs index 27081d72..6938d8ff 100644 --- a/buffa-test/src/lib.rs +++ b/buffa-test/src/lib.rs @@ -385,6 +385,16 @@ pub mod map_type { buffa::include_proto!("map_type"); } +#[allow(clippy::derivable_impls)] +pub mod map_omit_defaults { + buffa::include_proto!("map_omit_defaults"); +} + +#[allow(clippy::derivable_impls)] +pub mod map_omit_defaults_proto2 { + buffa::include_proto!("map_omit_defaults_proto2"); +} + /// `string_map` fixture: a crate-local `MapStr` newtype (a `ProtoString` impl, /// selected with `.string_type_custom(...)`) is used for every `string` map key /// and value. `MapStr` is `Hash + Eq + Ord + serde`, so it satisfies the diff --git a/buffa-test/src/tests/map_omit_defaults.rs b/buffa-test/src/tests/map_omit_defaults.rs new file mode 100644 index 00000000..8d99cf96 --- /dev/null +++ b/buffa-test/src/tests/map_omit_defaults.rs @@ -0,0 +1,164 @@ +//! `map_entries_omit_defaults`: map entries leave a key or value at its +//! default out, as prost writes them, from owned messages, views and lazy +//! views alike. + +use crate::map_omit_defaults::{maps, HolderLazyView, HolderView, Maps, MapsLazyView, MapsView}; +use buffa::view::LazyMessageView; +use buffa::{Message, MessageView, ViewEncode}; + +fn sample() -> Maps { + let mut maps = Maps::default(); + maps.counts.insert(String::new(), 0); + maps.counts.insert("a".into(), 7); + maps.names.insert(0, "x".into()); + maps.flags.insert(true, -0.0); + maps.blobs.insert("k".into(), Vec::new()); + maps.colors.insert(3, maps::Color::COLOR_UNSPECIFIED.into()); + maps.colors.insert(4, maps::Color::COLOR_RED.into()); + maps.leaves.insert(0, maps::Leaf::default()); + maps.leaves.insert( + 1, + maps::Leaf { + n: 5, + ..Default::default() + }, + ); + maps.floats.insert(-1, 0.0); + maps +} + +#[rustfmt::skip] +const SPARSE: &[u8] = &[ + 0x0a, 0x00, // counts: "" => 0, both left out + 0x0a, 0x05, 0x0a, 0x01, b'a', 0x10, 0x07, // counts: "a" => 7 + 0x12, 0x03, 0x12, 0x01, b'x', // names: 0 => "x", the key left out + 0x1a, 0x02, 0x08, 0x01, // flags: true => -0.0, the value left out + 0x22, 0x03, 0x0a, 0x01, b'k', // blobs: "k" => empty + 0x2a, 0x02, 0x08, 0x03, // colors: 3 => COLOR_UNSPECIFIED + 0x2a, 0x04, 0x08, 0x04, 0x10, 0x01, // colors: 4 => COLOR_RED + 0x32, 0x00, // leaves: 0 => empty, both left out + 0x32, 0x06, 0x08, 0x01, 0x12, 0x02, 0x08, 0x05, // leaves: 1 => { n: 5 } + 0x3a, 0x02, 0x08, 0x01, // floats: -1 => 0.0 +]; + +/// `sample()` with both fields of every entry written, as without the option. +#[rustfmt::skip] +const DENSE: &[u8] = &[ + 0x0a, 0x04, 0x0a, 0x00, 0x10, 0x00, // counts: "" => 0 + 0x0a, 0x05, 0x0a, 0x01, b'a', 0x10, 0x07, // counts: "a" => 7 + 0x12, 0x05, 0x08, 0x00, 0x12, 0x01, b'x', // names: 0 => "x" + 0x1a, 0x0b, 0x08, 0x01, // flags: true => -0.0 + 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80, + 0x22, 0x05, 0x0a, 0x01, b'k', 0x12, 0x00, // blobs: "k" => empty + 0x2a, 0x04, 0x08, 0x03, 0x10, 0x00, // colors: 3 => COLOR_UNSPECIFIED + 0x2a, 0x04, 0x08, 0x04, 0x10, 0x01, // colors: 4 => COLOR_RED + 0x32, 0x04, 0x08, 0x00, 0x12, 0x00, // leaves: 0 => empty + 0x32, 0x06, 0x08, 0x01, 0x12, 0x02, 0x08, 0x05, // leaves: 1 => { n: 5 } + 0x3a, 0x07, 0x08, 0x01, 0x15, 0x00, 0x00, 0x00, 0x00, // floats: -1 => 0.0 +]; + +#[test] +fn owned_encoding_leaves_defaults_out() { + let maps = sample(); + assert_eq!(maps.encode_to_vec(), SPARSE); + assert_eq!(maps.encoded_len() as usize, SPARSE.len()); +} + +#[test] +fn view_encoding_matches_the_owned_one() { + let view = MapsView::decode_view(SPARSE).expect("decodes"); + assert_eq!( + view.compute_size(&mut buffa::SizeCache::default()) as usize, + SPARSE.len() + ); + assert_eq!(view.encode_to_vec(), SPARSE); +} + +#[test] +fn lazy_view_encoding_matches_the_owned_one() { + let lazy = MapsLazyView::decode_lazy(SPARSE).expect("decodes"); + assert_eq!(lazy.encoded_len() as usize, SPARSE.len()); + assert_eq!(lazy.encode_to_vec(), SPARSE); +} + +/// Defaults written on the wire are left out by every encoder. +#[test] +fn written_defaults_are_left_out() { + let owned = Maps::decode_from_slice(DENSE).expect("decodes"); + assert_eq!(owned, sample()); + assert_eq!(owned.encode_to_vec(), SPARSE); + let view = MapsView::decode_view(DENSE).expect("decodes"); + assert_eq!(view.encode_to_vec(), SPARSE); + let lazy = MapsLazyView::decode_lazy(DENSE).expect("decodes"); + assert_eq!(lazy.encode_to_vec(), SPARSE); +} + +/// A lazy view writes a message field, which it keeps undecoded, as it was +/// read; a view decodes it and leaves the defaults out. +#[test] +fn lazy_view_writes_a_message_field_as_read() { + let wrap = |maps: &[u8]| { + let mut holder = vec![0x0a, maps.len() as u8]; + holder.extend_from_slice(maps); + holder + }; + let holder = wrap(DENSE); + let lazy = HolderLazyView::decode_lazy(&holder).expect("decodes"); + assert_eq!(lazy.encode_to_vec(), holder); + let view = HolderView::decode_view(&holder).expect("decodes"); + assert_eq!(view.encode_to_vec(), wrap(SPARSE)); +} + +#[test] +fn every_entry_decodes_back() { + let back = Maps::decode_from_slice(SPARSE).expect("decodes"); + assert_eq!(back, sample()); + // `-0.0` is left out like `0.0` and comes back as `0.0`. + assert!(back.flags[&true].is_sign_positive()); +} + +mod proto2 { + use crate::map_omit_defaults_proto2::{maps, Maps, MapsLazyView, MapsView}; + use buffa::view::LazyMessageView; + use buffa::{Message, MessageView, ViewEncode}; + + fn sample() -> Maps { + let mut maps = Maps::default(); + maps.sizes.insert(1, maps::Size::SIZE_SMALL); + maps.sizes.insert(2, maps::Size::SIZE_LARGE); + maps.required.insert( + 1, + maps::Required { + n: 0, + ..Default::default() + }, + ); + maps + } + + #[rustfmt::skip] + const SPARSE: &[u8] = &[ + 0x0a, 0x02, 0x08, 0x01, // sizes: 1 => SIZE_SMALL, the default + 0x0a, 0x04, 0x08, 0x02, 0x10, 0x01, // sizes: 2 => SIZE_LARGE + // required: 1 => { n: 0 }, written: it encodes its required field, + // where prost leaves out a value equal to its default + 0x12, 0x06, 0x08, 0x01, 0x12, 0x02, 0x08, 0x00, + ]; + + #[test] + fn closed_enum_and_required_values() { + let maps = sample(); + assert_eq!(maps.encode_to_vec(), SPARSE); + assert_eq!(maps.encoded_len() as usize, SPARSE.len()); + assert_eq!(Maps::decode_from_slice(SPARSE).expect("decodes"), maps); + let view = MapsView::decode_view(SPARSE).expect("decodes"); + assert_eq!( + view.compute_size(&mut buffa::SizeCache::default()) as usize, + SPARSE.len() + ); + assert_eq!(view.encode_to_vec(), SPARSE); + let lazy = MapsLazyView::decode_lazy(SPARSE).expect("decodes"); + assert_eq!(lazy.encoded_len() as usize, SPARSE.len()); + assert_eq!(lazy.encode_to_vec(), SPARSE); + } +} diff --git a/buffa-test/src/tests/mod.rs b/buffa-test/src/tests/mod.rs index 3bdc58f4..2a99550c 100644 --- a/buffa-test/src/tests/mod.rs +++ b/buffa-test/src/tests/mod.rs @@ -99,6 +99,7 @@ mod json_private_key; mod keyword; mod keyword_collisions; mod lazy_views; +mod map_omit_defaults; mod map_type; mod map_type_custom; mod message_set; diff --git a/buffa/src/map_codec.rs b/buffa/src/map_codec.rs index 4713e8c2..de463776 100644 --- a/buffa/src/map_codec.rs +++ b/buffa/src/map_codec.rs @@ -27,6 +27,10 @@ //! consumed during `write_to`, in identical iteration order). They get //! dedicated [`message_field_len`] / [`write_message_field`] helpers, and //! implement only [`MapValueDecode`] (via [`Msg`]) for the merge path. +//! +//! Every encoder has an `_omitting_defaults` twin, which generated code calls +//! under `map_entries_omit_defaults`: it leaves a key or value at its default +//! out of the entry, as prost does. use crate::bytes::Buf; use crate::encode_sink::EncodeSink; @@ -98,7 +102,7 @@ pub type Map = crate::__private::HashMap; /// The default is the `__private::HashMap` alias above; `buffa_build`'s /// `map_type` knob can instead select the buffa-provided /// [`BTreeMap`](alloc::collections::BTreeMap) (deterministic iteration order, -/// no extra dependency) or a custom map. The five runtime helpers in this +/// no extra dependency) or a custom map. The runtime helpers in this /// module need only three operations from the container — count entries, /// borrow-iterate entries, and insert one entry — so this trait captures that /// minimal surface and lets the helpers stay generic over *which* map a field @@ -349,6 +353,13 @@ pub trait MapCodec: MapValueDecode { /// Write the payload (no tag) to `buf`. fn encode(value: &Self::Value, buf: &mut impl EncodeSink); + + /// Whether `value` is what a missing key or value field decodes to, which + /// the `*_omitting_defaults` encoders leave out of an entry: zero, `false`, + /// empty, an open enum's 0, or a closed enum's default (its first value). + /// Compared with `==`, as prost does, so `-0.0` counts as a float's default + /// and decodes back as `0.0`. + fn is_default(value: &Self::Value) -> bool; } /// Stamp a varint/fixed scalar codec from the existing `types::` functions. @@ -389,6 +400,12 @@ macro_rules! scalar_codec { fn encode(value: &Self::Value, buf: &mut impl EncodeSink) { ($encode)(value, buf) } + + #[inline] + #[allow(clippy::float_cmp)] + fn is_default(value: &Self::Value) -> bool { + *value == <$value as Default>::default() + } } }; } @@ -544,6 +561,11 @@ impl MapCodec for ProtoBytesMap { // sinks by refcount, like top-level bytes fields. types::encode_shared_bytes(value, buf); } + + #[inline] + fn is_default(value: &Self::Value) -> bool { + value.as_ref().is_empty() + } } /// `string` codec for a custom [`ProtoString`](crate::types::ProtoString) @@ -587,6 +609,11 @@ impl MapCodec for ProtoStringMap { fn encode(value: &Self::Value, buf: &mut impl EncodeSink) { types::encode_string(value.as_ref(), buf); } + + #[inline] + fn is_default(value: &Self::Value) -> bool { + value.as_ref().is_empty() + } } /// Open-enum codec: values decode into [`EnumValue`], preserving unknown @@ -620,6 +647,11 @@ impl MapCodec for OpenEnum { fn encode(value: &Self::Value, buf: &mut impl EncodeSink) { types::encode_int32(value.to_i32(), buf); } + + #[inline] + fn is_default(value: &Self::Value) -> bool { + value.to_i32() == 0 + } } /// Closed-enum codec: values decode into the bare enum `E`. @@ -662,6 +694,11 @@ impl MapCodec for ClosedEnum { fn encode(value: &Self::Value, buf: &mut impl EncodeSink) { types::encode_int32(value.to_i32(), buf); } + + #[inline] + fn is_default(value: &Self::Value) -> bool { + *value == E::default() + } } /// Message-value codec (decode side only — encode is two-pass via @@ -785,6 +822,156 @@ pub fn write_message_field( } } +/// Tag length of an entry's key (field 1) or value (field 2), one byte for +/// every wire type. +const ENTRY_FIELD_TAG_LEN: u64 = 1; + +/// Which of an entry's fields the `_omitting_defaults` encoders leave out, +/// decided once for the size and the write of the entry. +struct Omitted { + key: bool, + value: bool, +} + +impl Omitted { + #[inline] + fn of(k: &KC::Value, v: &VC::Value) -> Self { + Self { + key: KC::is_default(k), + value: VC::is_default(v), + } + } + + /// Encoded length of the entry's fields, the length prefix excluded. + #[inline] + fn entry_len(&self, k: &KC::Value, v: &VC::Value) -> u64 { + let value = if self.value { + 0 + } else { + ENTRY_FIELD_TAG_LEN + VC::encoded_len(v) + }; + sparse_key_len::(self.key, k) + value + } +} + +/// Encoded length of an entry's key field, nothing when it is left out. +#[inline] +fn sparse_key_len(omitted: bool, k: &KC::Value) -> u64 { + if omitted { + 0 + } else { + ENTRY_FIELD_TAG_LEN + KC::encoded_len(k) + } +} + +/// Encoded length of an entry's message value field, nothing for a value that +/// encodes to nothing. +#[inline] +fn sparse_message_len(inner: u32) -> u64 { + if inner == 0 { + 0 + } else { + ENTRY_FIELD_TAG_LEN + varint_len(inner as u64) as u64 + inner as u64 + } +} + +/// [`field_len`] for entries that leave a default key or value out, as +/// [`write_field_omitting_defaults`] writes them. +pub fn field_len_omitting_defaults( + map: &C, + outer_tag_len: u64, +) -> u64 +where + C: MapStorage, +{ + let mut size = 0u64; + for (k, v) in map.storage_iter() { + let entry = Omitted::of::(k, v).entry_len::(k, v); + size += outer_tag_len + varint_len(entry) as u64 + entry; + } + size +} + +/// [`write_field`], leaving a key or value at its default out of its entry, as +/// prost does. The entry decodes to the same key and value, except that a +/// `-0.0` float decodes as `0.0`. +pub fn write_field_omitting_defaults( + map: &C, + field_number: u32, + buf: &mut impl EncodeSink, +) where + C: MapStorage, +{ + for (k, v) in map.storage_iter() { + let omitted = Omitted::of::(k, v); + Tag::new(field_number, WireType::LengthDelimited).encode(buf); + encode_varint(omitted.entry_len::(k, v), buf); + if !omitted.key { + Tag::new(1, KC::WIRE_TYPE).encode(buf); + KC::encode(k, buf); + } + if !omitted.value { + Tag::new(2, VC::WIRE_TYPE).encode(buf); + VC::encode(v, buf); + } + } +} + +/// [`message_field_len`] for entries that leave a default key or a message +/// that encodes to nothing out, as [`write_message_field_omitting_defaults`] +/// writes them. +pub fn message_field_len_omitting_defaults( + map: &C, + outer_tag_len: u64, + cache: &mut SizeCache, +) -> u64 +where + C: MapStorage, +{ + let mut size = 0u64; + for (k, v) in map.storage_iter() { + let slot = cache.reserve(); + let inner = v.compute_size(cache); + cache.set(slot, inner); + let entry = sparse_key_len::(KC::is_default(k), k) + sparse_message_len(inner); + size += outer_tag_len + varint_len(entry) as u64 + entry; + } + size +} + +/// [`write_message_field`], leaving a default key or a message that encodes to +/// nothing out of its entry. prost leaves out a message equal to its default +/// instead, which can still encode a field, such as a `required` one. +pub fn write_message_field_omitting_defaults( + map: &C, + field_number: u32, + cache: &mut SizeCache, + buf: &mut impl EncodeSink, +) where + C: MapStorage, +{ + for (k, v) in map.storage_iter() { + let inner = cache.consume_next(); + let key_omitted = KC::is_default(k); + Tag::new(field_number, WireType::LengthDelimited).encode(buf); + encode_varint( + sparse_key_len::(key_omitted, k) + sparse_message_len(inner), + buf, + ); + if !key_omitted { + Tag::new(1, KC::WIRE_TYPE).encode(buf); + KC::encode(k, buf); + } + if inner != 0 { + Tag::new(2, WireType::LengthDelimited).encode(buf); + encode_varint(inner as u64, buf); + } + // Writes nothing for an empty value, but consumes the slots its + // `compute_size` reserved. + v.write_to(cache, buf); + } +} + fn merge_entry_contents( key: &mut KC::Value, val: &mut VC::Value, @@ -997,6 +1184,35 @@ mod tests { map } + /// A closed enum whose default, its first value `A`, is not 0. protoc + /// rejects it as a map value; a descriptor built another way can have it. + #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, Default)] + #[repr(i32)] + enum FirstIsOne { + #[default] + A = 1, + B = 0, + } + + impl Enumeration for FirstIsOne { + fn from_i32(value: i32) -> Option { + match value { + 1 => Some(Self::A), + 0 => Some(Self::B), + _ => None, + } + } + fn to_i32(&self) -> i32 { + *self as i32 + } + fn proto_name(&self) -> &'static str { + match self { + Self::A => "A", + Self::B => "B", + } + } + } + #[test] fn string_int32_round_trip() { let mut map: Map = Map::default(); @@ -1046,6 +1262,138 @@ mod tests { assert_eq!(back, map); } + /// Entries that leave the default key and value out, against the bytes + /// prost writes for them, decoding back to the same map. + #[test] + fn omitting_defaults_leaves_default_fields_out() { + let encode = |key: &str, value: i32| { + let mut map: Map = Map::default(); + map.insert(key.into(), value); + let len = field_len_omitting_defaults::(&map, 1); + let mut buf = Vec::new(); + write_field_omitting_defaults::(&map, 5, &mut buf); + assert_eq!(buf.len() as u64, len, "the size matches the written bytes"); + assert_eq!(decode_field::(&buf), map); + buf + }; + // tag 5, length 0: neither field + assert_eq!(encode("", 0), [0x2a, 0x00]); + // the key only + assert_eq!(encode("a", 0), [0x2a, 0x03, 0x0a, 0x01, b'a']); + // the value only + assert_eq!(encode("", 7), [0x2a, 0x02, 0x10, 0x07]); + assert_eq!( + encode("a", 7), + encode_field::( + &{ + let mut map = Map::default(); + map.insert(String::from("a"), 7); + map + }, + 5, + 1 + ) + ); + // `-0.0` is a float's default to `==`, as to prost + let mut floats: Map = Map::default(); + floats.insert(1, -0.0); + let mut buf = Vec::new(); + write_field_omitting_defaults::(&floats, 2, &mut buf); + assert_eq!(buf, [0x12, 0x02, 0x08, 0x01]); + } + + /// A closed enum value is left out when it is the enum's default, which + /// need not be 0, as prost leaves it out. + #[test] + fn omitting_defaults_compares_a_closed_enum_with_its_default() { + type Codec = ClosedEnum; + let encode = |value: FirstIsOne| { + let mut map: Map = Map::default(); + map.insert(1, value); + let len = field_len_omitting_defaults::(&map, 1); + let mut buf = Vec::new(); + write_field_omitting_defaults::(&map, 2, &mut buf); + assert_eq!(buf.len() as u64, len, "the size matches the written bytes"); + assert_eq!(decode_field::(&buf), map); + buf + }; + // key 1, the default `A` left out + assert_eq!(encode(FirstIsOne::A), [0x12, 0x02, 0x08, 0x01]); + // key 1, `B` written although it is 0 + assert_eq!(encode(FirstIsOne::B), [0x12, 0x04, 0x08, 0x01, 0x10, 0x00]); + } + + /// `is_default` holds for exactly the value a missing entry field decodes + /// to, for every codec. + #[test] + fn is_default_is_the_decoded_default() { + fn check(default: C::Value, others: &[C::Value]) { + // An entry with neither field: key 0, the value's default. + let decoded = decode_field::(&[0x0a, 0x00]); + assert!(C::is_default(&decoded[&0])); + assert!(C::is_default(&default)); + for other in others { + assert!(!C::is_default(other)); + } + } + #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, Default)] + #[repr(i32)] + enum E { + #[default] + A = 0, + B = 1, + } + impl Enumeration for E { + fn from_i32(value: i32) -> Option { + match value { + 0 => Some(E::A), + 1 => Some(E::B), + _ => None, + } + } + fn to_i32(&self) -> i32 { + *self as i32 + } + fn proto_name(&self) -> &'static str { + match self { + E::A => "A", + E::B => "B", + } + } + } + check::(0, &[1, -1]); + check::(0, &[1, i64::MIN]); + check::(0, &[1]); + check::(0, &[u64::MAX]); + check::(0, &[-1]); + check::(0, &[-1]); + check::(0, &[1]); + check::(0, &[1]); + check::(0, &[-1]); + check::(0, &[-1]); + check::(false, &[true]); + check::(0.0, &[1.0, f32::NAN, f32::MIN_POSITIVE]); + check::(-0.0, &[]); + check::(0.0, &[1.0, f64::NAN, f64::MIN_POSITIVE]); + check::(-0.0, &[]); + check::(String::new(), &["a".into()]); + check::(Vec::new(), &[vec![0]]); + check::( + crate::bytes::Bytes::new(), + &[crate::bytes::Bytes::from_static(&[0])], + ); + check::>(String::new(), &[" ".into()]); + check::>>(Vec::new(), &[vec![0]]); + check::>( + EnumValue::from(0), + &[EnumValue::from(1), EnumValue::Unknown(7)], + ); + check::>(E::A, &[E::B]); + // A missing closed enum decodes as its first value, an open one as 0. + check::>(FirstIsOne::A, &[FirstIsOne::B]); + check::>(EnumValue::from(0), &[EnumValue::from(1)]); + } + #[test] fn missing_key_and_value_take_defaults() { // Entry with no fields at all: length prefix 0. @@ -1178,6 +1526,110 @@ mod tests { let back = decode_field::>(&wire); assert_eq!(back, map); + + // Omitting defaults: the empty message and the zero key go, the + // slots stay in step. + let mut cache = SizeCache::default(); + let mut sparse: Map = Map::default(); + sparse.insert(0, FlatMsg { value: 5 }); + sparse.insert(1, FlatMsg { value: 0 }); + sparse.insert(2, FlatMsg { value: -3 }); + let len = message_field_len_omitting_defaults::(&sparse, 1, &mut cache); + let mut wire = Vec::new(); + write_message_field_omitting_defaults::( + &sparse, 4, &mut cache, &mut wire, + ); + assert_eq!(wire.len() as u64, len, "size pass must match write pass"); + assert_eq!(decode_field::>(&wire), sparse); + let entries: Vec> = { + let mut rest = wire.as_slice(); + let mut out = Vec::new(); + while !rest.is_empty() { + Tag::decode(&mut rest).unwrap(); + let len = decode_varint(&mut rest).unwrap() as usize; + out.push(rest[..len].to_vec()); + rest = &rest[len..]; + } + out + }; + // key 0 left out, value 5; key 1, the empty value left out + assert!( + entries.contains(&vec![0x12, 0x02, 0x08, 0x05]), + "{entries:?}" + ); + assert!(entries.contains(&vec![0x08, 0x01]), "{entries:?}"); + + // A value that reserves a slot of its own, after an empty one: the + // slots stay in step. + #[derive(Clone, PartialEq, Eq, Debug, Default)] + struct Outer { + inner: Option, + } + + impl DefaultInstance for Outer { + fn default_instance() -> &'static Self { + static INST: crate::__private::OnceBox = crate::__private::OnceBox::new(); + INST.get_or_init(|| crate::alloc::boxed::Box::new(Outer::default())) + } + } + + impl Message for Outer { + fn compute_size(&self, cache: &mut SizeCache) -> u32 { + let Some(inner) = &self.inner else { + return 0; + }; + let slot = cache.reserve(); + let len = inner.compute_size(cache); + cache.set(slot, len); + 1 + varint_len(len as u64) as u32 + len + } + fn write_to(&self, cache: &mut SizeCache, buf: &mut impl EncodeSink) { + if let Some(inner) = &self.inner { + let len = cache.consume_next(); + Tag::new(1, WireType::LengthDelimited).encode(buf); + encode_varint(len as u64, buf); + inner.write_to(cache, buf); + } + } + fn merge_field( + &mut self, + tag: Tag, + buf: &mut impl Buf, + ctx: DecodeContext<'_>, + ) -> Result<(), DecodeError> { + match tag.field_number() { + 1 => Message::merge_length_delimited( + self.inner.get_or_insert_with(FlatMsg::default), + buf, + ctx, + )?, + _ => skip_field_depth(tag, buf, ctx.depth())?, + } + Ok(()) + } + fn clear(&mut self) { + *self = Self::default(); + } + } + + let mut nested = crate::alloc::collections::BTreeMap::new(); + nested.insert(1, Outer::default()); + nested.insert( + 2, + Outer { + inner: Some(FlatMsg { value: 7 }), + }, + ); + let mut cache = SizeCache::default(); + let len = message_field_len_omitting_defaults::(&nested, 1, &mut cache); + let mut wire = Vec::new(); + write_message_field_omitting_defaults::(&nested, 4, &mut cache, &mut wire); + assert_eq!(wire.len() as u64, len, "size pass must match write pass"); + #[rustfmt::skip] + assert_eq!(wire, [ + 0x22, 0x02, 0x08, 0x01, // key 1, the empty value left out + 0x22, 0x08, 0x08, 0x02, 0x12, 0x04, 0x0a, 0x02, 0x08, 0x07, // key 2 + ]); } #[test] diff --git a/docs/guide.md b/docs/guide.md index 17db0799..dbd7829e 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -208,6 +208,7 @@ The macro pulls in `OUT_DIR/.mod.rs`, which in turn includes the per | `.override_feature_in(path, feature)` | — | Apply a path-scoped editions feature override to the compiled descriptors — for protos you cannot modify; see [Enums](#enumvaluet--type-safe-open-enums) for the `enum_type` override's semantics | | `.open_enums_in(&[...])` | — | Shorthand for `override_feature_in(path, FeatureOverride::EnumType(EnumTypeOverride::Open))` per path: treat matching closed enums (or closed enum fields) as open in generated Rust (`EnumValue`) | | `.generate_with_setters(bool)` | `true` | Emit `with_()` builder-style setters for explicit-presence fields | +| `.map_entries_omit_defaults(bool)` | `false` | Encode map entries as prost does: an entry's key or value at its default is left out, the entry itself is kept. Each crate's types encode as that crate was generated; the well-known types and `DynamicMessage` write both. For multi-entry maps to match prost's bytes, also use `.map_type(MapRepr::BTreeMap)` where prost used `btree_map`. See [Map entries](migration-from-prost.md#map-entries) | | `.generate_arbitrary(bool)` | `false` | Emit `#[derive(arbitrary::Arbitrary)]` gated behind the `arbitrary` feature (for fuzzing) | | `.skip_debug(&[...])` | — | Omit the generated `Debug` impl for matching messages (proto-path prefixes), with their oneof enums, and for enums named exactly, so your crate can write its own. A matched enum needs a hand-written impl to compile. See [`skip_debug` and hand-written `Debug`](#skip_debug-and-hand-written-debug) | | `.gate_impls_on_crate_features(bool)` | `false` | Wrap json/views/text impls in `#[cfg(feature = ...)]` for library crates whose generated code is a public dependency surface | @@ -647,6 +648,7 @@ Passed via `opt:` (works for `remote:` and `local:`): | `gate_impls=true` | Wrap json/views/text impls in `#[cfg(feature = ...)]` for library crates whose generated code is a public dependency surface (default: emitted unconditionally) | | `json_feature=` | Rename the crate feature a gated impl kind is conditioned on (also `views_feature=`, `text_feature=`, `reflect_feature=`); inert without `gate_impls=true` | | `with_setters=false` | Disable `with_()` builder-style setters for explicit-presence fields (default: emitted) | +| `map_entries_omit_defaults=true` | Encode map entries as prost does: an entry's key or value at its default is left out, the entry itself is kept, in the messages generated with it (default: both written). Maps with several entries keep `HashMap` order, so they match prost's `btree_map` bytes only through `buffa-build`'s `map_type` — see [Map entries](migration-from-prost.md#map-entries) | | `lazy_views=true` | Generate the lazy view family alongside the strict views (default: false) — see [Lazy views](#lazy-views--lazy_viewstrue) | | `register_types=false` | Disable the per-package `register_types()` helper that populates a `MessageRegistry` (default: emitted) | | `allow_message_set=true` | Permit `option message_set_wire_format = true;` instead of rejecting it (default: false) | @@ -1253,6 +1255,7 @@ These stay unrolled, whatever the setting: - the message type of a group field; - a message that uses the `MessageSet` wire format; - a message with extension ranges, when JSON code is generated and unknown fields are preserved; +- a message with a `map` field, when `map_entries_omit_defaults` is set: the table writes both fields of every entry; - a message with a field of a non-default string, bytes, or collection type, which `use_bytes_type`, `string_type`, `bytes_type`, `repeated_type`, and a custom map type (`map_type_custom`) select. `HashMap` and `BTreeMap` maps are supported; a `map` whose keys or values are of a non-default string or bytes type is not; - a message that holds, in a singular, repeated, `oneof`, or map value field, a message of the same run that has a `bytes` field of a non-default type, or that holds one. The table decodes from one contiguous slice, so it would copy the `bytes::Bytes` fields that unrolled code decoding from a `Bytes` shares with the input. diff --git a/docs/migration-from-prost.md b/docs/migration-from-prost.md index 72e856ad..7b35f352 100644 --- a/docs/migration-from-prost.md +++ b/docs/migration-from-prost.md @@ -179,6 +179,14 @@ For proto2 closed enums, Buffa follows closed-enum semantics by default and rout +let len = msg.encoded_len() as usize; ``` +### Map entries + +prost leaves a map entry's key or value out when it holds the default; buffa writes both, as the C++ and Java runtimes do. Both decode the same, but the bytes differ. If stored or hashed map encodings must not change, turn on `.map_entries_omit_defaults(true)` to leave a default key or value out as prost does, and keep the entry order with `.map_type(buffa_build::MapRepr::BTreeMap)` where prost used `btree_map`. The option removes this difference, except for the message values described in the next paragraph. Other differences remain, for example: outside a map, a proto3 `float` or `double` field without `optional` that is `-0.0` is written by buffa and not by prost, and buffa writes back the [unknown fields](#unknown-field-preservation) it read. + +The option leaves out what a missing field decodes to: zero, `false`, empty, an open enum's 0, a closed enum's first value, and a message value that encodes to nothing. As with prost, a `-0.0` float value is left out and decodes as `0.0`. prost leaves out a message value equal to its default, so a value that equals its default but still encodes a field, such as a `required` field at its default or a float field holding `-0.0`, is written by buffa and not by prost. + +It applies to the owned, view and lazy view encoders of the messages generated with it. A lazy view keeps its singular and repeated message fields undecoded and writes them as they were read, so map entries inside them keep the encoding they arrived with, and every encoder writes entries kept as unknown fields as they were read. A value of an enum made open with `open_enums_in` is left out at 0, which a missing open enum decodes to, where prost leaves out its first value; the two differ only for an enum whose first value is not 0, which protoc rejects as a map value, so the case can occur only with a descriptor set that another tool built. Each crate's types encode as that crate was generated; `google.protobuf.Struct`, the well-known type with a map field, comes from `buffa-types` and writes both fields, and so does `DynamicMessage`. `protoc-gen-buffa` has no `map_type` option, so through the plugin a map with several entries keeps `HashMap` order. With `CodecStrategy::Table`, messages with a map field keep the unrolled codec, and a `codec_strategy_in` rule that selects the table for such a message by its exact path is an error. + ## 6. Decoding API ```diff @@ -292,6 +300,7 @@ The buffa equivalent of each prost-build feature, or a note that there is none: | prost feature | buffa status | |---------------|-------------| | `btree_map(&[...])` | Supported. `.map_type(buffa_build::MapRepr::BTreeMap)` for all, or `.map_type_in(buffa_build::MapRepr::BTreeMap, &[...])` for specific fields. Paths are fully qualified (`".my_pkg.MyMessage.items"`), and the leading dot is optional. prost's suffix paths (`"items"`, `"MyMessage.items"`) are not supported: `"items"` is read as `".items"`, matches no field, and produces a build warning. | +| map entry encoding (default key or value left out) | Opt-in: `.map_entries_omit_defaults(true)`. See [Map entries](#map-entries). | | `bytes(&[...])` | Supported. `.use_bytes_type()` for all, or `.use_bytes_type_in(&[...])` for specific fields. Paths are matched as for `map_type_in`. | | `extern_path(proto, rust)` | Supported. Same API, both package-level (`.extern_path(".pkg", "::crate")`) and per-type (`.extern_path(".google.protobuf.Timestamp", "::pbjson_types::Timestamp")`) mappings. A per-type mapping to a non-buffa crate requires `.generate_views(false)`, or map to a buffa-generated crate instead — see [External type paths](guide.md#external-type-paths). | | `type_attribute(path, attr)` | Supported. Same API, plus `message_attribute` / `enum_attribute` / `oneof_attribute` for narrower targeting. (For serde, prefer `generate_json(true)`, which emits the proto3-canonical JSON impls.) | diff --git a/protoc-gen-buffa/src/main.rs b/protoc-gen-buffa/src/main.rs index 18c62bc3..f8668a4b 100644 --- a/protoc-gen-buffa/src/main.rs +++ b/protoc-gen-buffa/src/main.rs @@ -289,6 +289,11 @@ fn parse_config(params: &str) -> Result { // methods. Like `register_types`, the default is on, so the // accepted spelling is the negation. "with_setters" => codegen.generate_with_setters = parse_bool("with_setters", value)?, + // `map_entries_omit_defaults=true` writes maps as prost does, + // leaving default entry fields out. + "map_entries_omit_defaults" => { + codegen.map_entries_omit_defaults = parse_bool("map_entries_omit_defaults", value)? + } // `reflection=true` selects the fast vtable mode (same as // `reflect_mode=vtable`); `reflect_mode=bridge` opts into the // smaller round-trip implementation. @@ -762,6 +767,13 @@ mod tests { assert!(!config.codegen.idiomatic_field_names); } + #[test] + fn map_entries_omit_defaults_is_off_unless_asked() { + assert!(!parse_config("").unwrap().codegen.map_entries_omit_defaults); + let config = parse_config("map_entries_omit_defaults=true").unwrap(); + assert!(config.codegen.map_entries_omit_defaults); + } + #[test] fn idiomatic_enum_aliases_can_be_disabled_and_reenabled() { let config = parse_config("").unwrap();