diff --git a/.changes/unreleased/added-20260923-codec-strategy-table.yaml b/.changes/unreleased/added-20260923-codec-strategy-table.yaml index 757b250f..acae42eb 100644 --- a/.changes/unreleased/added-20260923-codec-strategy-table.yaml +++ b/.changes/unreleased/added-20260923-codec-strategy-table.yaml @@ -1,4 +1,4 @@ kind: Added body: |- - **Table-driven message codec** (#469, refs #463). `buffa_build::Config::codec_strategy(CodecStrategy::Table)` (plugin option `codec_strategy=table`, `CodeGenConfig::codec_strategy`) generates each message's binary `Message` implementation from a static table and interpreters that every message shares, instead of code specialised to the message's fields. The compiled code is substantially smaller, and messages made of many small fields are slower. `CodecStrategy::Unrolled` stays the default. `codec_strategy_in(strategy, &[paths])` (plugin option `codec_strategy_in==`, repeatable) chooses the strategy for matching messages and the messages nested in them, and the last matching rule wins. The wire format does not change. Messages the table cannot handle stay unrolled, and `CodeGenWarning::TableCodecFallbackSummary` counts them. A rule that selects the table, by exact path, for a message that cannot use it is an error. The generated code needs Rust 1.77 or later, which `buffa-build` checks, and compiles in a crate with `#![forbid(unsafe_code)]`. The guide's "Smaller generated code" section has the measurements, the messages that stay unrolled and the behavioural differences. + **Table-driven message codec** (#469, refs #463). `buffa_build::Config::codec_strategy(CodecStrategy::Table)` (plugin option `codec_strategy=table`, `CodeGenConfig::codec_strategy`) generates each message's binary `Message` implementation from a static table and interpreters that every message shares, instead of code specialised to the message's fields. The compiled code is substantially smaller, and messages made of many small fields are slower. `CodecStrategy::Unrolled` stays the default. `codec_strategy_in(strategy, &[paths])` (plugin option `codec_strategy_in==`, repeatable) chooses the strategy for matching messages and the messages nested in them, and the last matching rule wins. The wire format does not change, and a message with a `oneof` uses the table, including when a member is a message that has no table. Messages the table cannot handle stay unrolled, and `CodeGenWarning::TableCodecFallbackSummary` counts them. A rule that selects the table, by exact path, for a message that cannot use it is an error. The generated code needs Rust 1.77 or later, which `buffa-build` checks, and compiles in a crate with `#![forbid(unsafe_code)]`. The guide's "Smaller generated code" section has the measurements, the messages that stay unrolled and the behavioural differences. time: 2026-09-23T03:10:00+00:00 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 37a9ae54..4afdc2aa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -305,7 +305,10 @@ jobs: # provenance, alignment and initialisation on the kinds and field # shapes the module's tests build, including message fields stored # inline, boxed and in a `Vec`, whose child pointers point into the - # parent or the heap. + # parent or the heap, and oneofs, through hand-written enums with + # inline and boxed message members. The `OneofEnum` code that codegen + # emits runs natively only, in the differential tests of `buffa-test`, + # not under Miri. - name: Miri (table interpreter soundness) run: cargo +${{ env.MIRI_TOOLCHAIN }} miri test -p buffa --lib -- 'table::' diff --git a/DESIGN.md b/DESIGN.md index bf30d42b..2e1aa65e 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -560,12 +560,13 @@ Within that bound, the shortcuts do not pay off. `merge` does not help: it consu The size, write, and merge code of an unrolled message is specialised to its fields. `CodecStrategy::Table` (a per-message option, `Unrolled` by default) replaces it with a static `buffa::table::Table` and a `Message` impl that forwards to interpreters in `buffa::table`. The decision to keep `Unrolled` as the default is in [#463](https://github.com/anthropics/buffa/issues/463), and the guide's "Smaller generated code" section has the measurements and the list of messages that stay unrolled. -A table holds a sorted array of 12-byte entries `{tag, offset, kind, tag_len, aux}`, a dense array that maps field numbers below 64 to entries, and the offset of the unknown-fields slot. `kind` is the field type crossed with its cardinality, so the interpreter dispatches once per field. Message, repeated-message, and enum fields carry a small descriptor (`Aux`) with the accessors that their storage needs, because a `MessageField`, a `Vec`, and an `EnumValue` cannot be read through an offset alone. Offsets come from `core::mem::offset_of!`, so the table needs Rust 1.77 and the generated code refers to it through `buffa::__table!`, which is a compile error on an older compiler. +A table holds a sorted array of 12-byte entries `{tag, offset, kind, tag_len, aux}`, a dense array that maps field numbers below 64 to entries, and the offset of the unknown-fields slot. `kind` is the field type crossed with its cardinality, so the interpreter dispatches once per field. Message, repeated-message, and enum fields carry a small descriptor (`Aux`) with the accessors that their storage needs, because a `MessageField`, a `Vec`, and an `EnumValue` cannot be read through an offset alone; a oneof has a `Group` descriptor and each of its members a `Member` one. Offsets come from `core::mem::offset_of!`, so the table needs Rust 1.77 and the generated code refers to it through `buffa::__table!`, which is a compile error on an older compiler. -Three decisions shape the runtime: +Four decisions shape the runtime: - **The `unsafe` lives in `buffa`.** `__table!` and `__table_entry!` contain the `unsafe` blocks and witness each field's type against its kind, so a table that names the wrong kind for a field does not compile, and generated code compiles under `#![forbid(unsafe_code)]`. `Table::new` also checks the layout constants at compile time. The interpreters run under Miri in CI. - **The interpreters are not generic over the sink or the input where that is avoidable.** `Message::encode` and its siblings write any `BufMut` through one shared, non-generic cursor (`buffa/src/encode_sink.rs`), and decoding runs over a contiguous `&[u8]`, so the interpreters are compiled once in `buffa`, at its `opt-level`, and not once per caller. +- **A oneof is one table entry per member,** all at the offset of the `Option`, because that enum has no specified layout. Generated code implements the safe trait `OneofEnum` for it, so the interpreters can find the member that is set, and the `unsafe` stays in `buffa`. The bytes and the decoding are the same as unrolled code's; the documentation of `OneofEnum` in `buffa/src/table/oneof.rs` describes the mechanism and what an implementation must guarantee. - **A child reaches the interpreters through its table or its `Message` impl.** A message field's `Aux` descriptor holds either the child's `MessageTable` or a `DynVt` of function pointers instantiated for the child type (`table/bridge.rs`), so a table message can hold any message it is not required to leave unrolled. The write pointer takes the `PreSized` cursor because a function pointer cannot be generic over the sink. A pointer per child type and sink type would avoid the copy that other sinks cost, at the price of code in every crate that uses the table. The planner in `buffa-codegen` (`table_plan.rs`) selects the messages that asked for the table and drops those the interpreters cannot handle. Among them it drops, transitively, the holders of a message of the run with a non-default bytes type, because the table decodes from one contiguous slice and would copy the `bytes::Bytes` fields that unrolled code shares with a `Bytes` input. A child from another crate is not inspected, so a holder of `google.protobuf.Any`, whose `value` is `bytes::Bytes`, keeps its table and copies the payload. ### Owned decode: intentional throughput trade-offs diff --git a/buffa-codegen/src/lib.rs b/buffa-codegen/src/lib.rs index cb1aae03..7fd55583 100644 --- a/buffa-codegen/src/lib.rs +++ b/buffa-codegen/src/lib.rs @@ -1094,7 +1094,7 @@ pub enum CodecStrategy { /// /// Not every message can use it. These stay [`Unrolled`](Self::Unrolled): /// - /// - a message with a `oneof`, a `map` field, or a group field; + /// - a message with a `map` field or a group field; /// - 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 @@ -1108,9 +1108,10 @@ pub enum CodecStrategy { /// contiguous slice, so it would copy `bytes::Bytes` fields that /// unrolled code decoding from a `Bytes` shares with the input. /// - /// A table message may hold any other message. It reaches a child that is a - /// table message through the child's table, and any other child, whether - /// it is [`Unrolled`](Self::Unrolled), generated by another crate, or a + /// A table message may hold any other message, including as a member of a + /// `oneof`. It reaches a child that is a table message through the + /// child's table, and any other child, whether it is + /// [`Unrolled`](Self::Unrolled), generated by another crate, or a /// well-known type, through the child's `Message` impl, which costs a /// function call per child. Selecting a message does not select the /// messages it holds, and keeping a child [`Unrolled`](Self::Unrolled) does @@ -1156,7 +1157,7 @@ pub enum CodecStrategy { #[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] pub struct TableCodecFallbackReason { - /// The reason as a predicate, such as `has a oneof`. The wording is for + /// The reason as a predicate, such as `has a map field`. The wording is for /// people and may change between releases. pub reason: String, /// The proto paths of all of them, with a leading dot, in declaration diff --git a/buffa-codegen/src/table_codec.rs b/buffa-codegen/src/table_codec.rs index bbe02243..d66d17f3 100644 --- a/buffa-codegen/src/table_codec.rs +++ b/buffa-codegen/src/table_codec.rs @@ -4,15 +4,17 @@ //! Which messages get one is decided by [`crate::table_plan`]; this module //! emits the code for a message the plan selected. +use std::collections::{BTreeMap, HashMap, HashSet}; + use proc_macro2::TokenStream; use quote::{format_ident, quote}; -use crate::context::MessageScope; +use crate::context::{ancillary_prefix, AncillaryKind, MessageScope}; use crate::generated::descriptor::field_descriptor_proto::Type; -use crate::generated::descriptor::DescriptorProto; +use crate::generated::descriptor::{DescriptorProto, FieldDescriptorProto}; use crate::idents::rust_path_to_tokens; use crate::message::classify_field; -use crate::table_plan::{table_fields, Card, TableField}; +use crate::table_plan::{table_fields, Card, OneofMembership, TableField}; use crate::CodeGenError; /// The name of the static table of the message struct `rust_name`. @@ -73,11 +75,17 @@ pub(crate) fn generate_table_impl( let mut entries: Vec = Vec::with_capacity(fields.len()); let mut aux: Vec = Vec::new(); + let mut oneofs = Oneofs::new(scope, msg, &fields)?; for f in &fields { + if let Some(member) = &f.oneof { + entries.push(oneofs.member_entry(scope, &name, f, member, &mut aux)?); + continue; + } let (entry, aux_item) = field_entry(scope, msg, &name, f, aux.len(), resolver)?; entries.push(entry); aux.extend(aux_item); } + let oneof_impls = oneofs.into_impls(); let dense = dense_lookup(&fields); let abi = proc_macro2::Literal::u32_unsuffixed(TABLE_ABI); @@ -140,6 +148,8 @@ pub(crate) fn generate_table_impl( *self = ::core::default::Default::default(); } } + + #(#oneof_impls)* }) } @@ -177,52 +187,17 @@ fn field_entry( aux_index: usize, resolver: &crate::imports::ImportResolver, ) -> Result<(TokenStream, Option), CodeGenError> { - let MessageScope { - ctx, - current_package, - nesting, - .. - } = scope; + let MessageScope { ctx, .. } = scope; let field = f.field; let field_name = field.name.as_deref().unwrap_or(""); let ident = ctx.field_ident(field_name, field.number.unwrap_or(0)); let kind = format_ident!("{}", f.kind); let number = f.number; - let aux_u16 = || { - u16::try_from(aux_index).map_err(|_| { - CodeGenError::Other(format!( - "table codec: {}.{field_name}: a message has more than 65535 fields \ - that need a descriptor", - scope.proto_fqn - )) - }) - }; - let type_name = || { - field - .type_name - .as_deref() - .ok_or(CodeGenError::MissingField("field.type_name")) - }; - let type_path = |what: &str| -> Result { - let type_name = type_name()?; - ctx.rust_type_relative(type_name, current_package, nesting) - .ok_or_else(|| CodeGenError::Other(format!("{what} type '{type_name}' not found"))) - }; - - // The table is not among the imports that `idiomatic_imports` shortens - // paths with, so its path is built from the unshortened one. - let unshortened_path = || -> Result { - let type_name = type_name()?; - let split = ctx - .rust_type_relative_split(type_name, current_package, nesting) - .ok_or_else(|| CodeGenError::Other(format!("message type '{type_name}' not found")))?; - Ok(if split.to_package.is_empty() { - split.within_package - } else { - format!("{}::{}", split.to_package, split.within_package) - }) - }; + let aux_u16 = || u16::try_from(aux_index).map_err(|_| too_many_descriptors(scope)); + let type_path = |what: &str| type_path(scope, field, what); + let unshortened_path = || unshortened_path(scope, field); + let type_name = || field_type_name(field); match f.ty { Type::TYPE_MESSAGE => { @@ -292,3 +267,319 @@ fn field_entry( )), } } + +/// The proto path of the message or enum type of `field`. +fn field_type_name(field: &FieldDescriptorProto) -> Result<&str, CodeGenError> { + field + .type_name + .as_deref() + .ok_or(CodeGenError::MissingField("field.type_name")) +} + +/// The error for a message whose table needs more descriptors than an aux +/// index can name. +fn too_many_descriptors(scope: MessageScope<'_>) -> CodeGenError { + CodeGenError::Other(format!( + "table codec: {}: a message has more than 65535 fields that need a descriptor", + scope.proto_fqn + )) +} + +/// The Rust path of the message or enum type of `field`, as seen from the +/// message's scope. `what` names the kind of type in the error. +fn type_path( + scope: MessageScope<'_>, + field: &FieldDescriptorProto, + what: &str, +) -> Result { + let type_name = field_type_name(field)?; + scope + .ctx + .rust_type_relative(type_name, scope.current_package, scope.nesting) + .ok_or_else(|| CodeGenError::Other(format!("{what} type '{type_name}' not found"))) +} + +/// The path of the message type of `field` before `idiomatic_imports` shortens +/// it. The table is not among the names that `idiomatic_imports` shortens, so +/// its path is built from this one. +fn unshortened_path( + scope: MessageScope<'_>, + field: &FieldDescriptorProto, +) -> Result { + let type_name = field_type_name(field)?; + let split = scope + .ctx + .rust_type_relative_split(type_name, scope.current_package, scope.nesting) + .ok_or_else(|| CodeGenError::Other(format!("message type '{type_name}' not found")))?; + Ok(if split.to_package.is_empty() { + split.within_package + } else { + format!("{}::{}", split.to_package, split.within_package) + }) +} + +/// The match arms of the `OneofEnum` implementation of one oneof enum. +#[derive(Default)] +struct OneofArms { + number: Vec, + payload: Vec, + payload_mut: Vec, + with_default: Vec, +} + +/// The oneofs of the message being generated: the table entries and aux items +/// of their members, and the `OneofEnum` implementations that the interpreters +/// reach the members through. +struct Oneofs { + /// The path of the oneof enums' module, from the message's scope. + prefix: TokenStream, + /// The Rust names of the oneof enums, by the oneof's index in the message. + enum_idents: HashMap, + /// The lowest member number of each oneof. + first: HashMap, + /// The oneofs that have a member of message type. + with_messages: HashSet, + /// The aux index of each oneof's descriptor, once it has one. + group_aux: HashMap, + /// The aux index of each payload descriptor, by its tokens, so that + /// members with the same kind of value share one. + payload_aux: HashMap, + arms: BTreeMap, +} + +impl Oneofs { + fn new( + scope: MessageScope<'_>, + msg: &DescriptorProto, + fields: &[TableField<'_>], + ) -> Result { + let mut first: HashMap = HashMap::new(); + let mut with_messages = HashSet::new(); + for (oneof, f) in fields + .iter() + .filter_map(|f| f.oneof.as_ref().map(|oneof| (oneof, f))) + { + let lowest = first.entry(oneof.index).or_insert(f.number); + *lowest = (*lowest).min(f.number); + if f.ty == Type::TYPE_MESSAGE { + with_messages.insert(oneof.index); + } + } + Ok(Self { + prefix: ancillary_prefix( + AncillaryKind::Oneof, + scope.current_package, + scope.proto_fqn, + scope.nesting, + ), + enum_idents: crate::oneof::resolve_oneof_idents(msg, scope.proto_fqn)?, + first, + with_messages, + group_aux: HashMap::new(), + payload_aux: HashMap::new(), + arms: BTreeMap::new(), + }) + } + + /// Add `item` to `aux`, or find where the same item already is. + fn shared_aux(&mut self, aux: &mut Vec, item: TokenStream) -> usize { + *self.payload_aux.entry(item.to_string()).or_insert_with(|| { + aux.push(item); + aux.len() - 1 + }) + } + + /// The `__table_entry!` of one oneof member, and the aux items it needs + /// (its oneof's descriptor if this is the first member met, the + /// descriptor of its value if the kind of value has one, and the member + /// item itself), which are pushed to `aux`. Records the member's arms of + /// the enum's implementation. + fn member_entry( + &mut self, + scope: MessageScope<'_>, + message: &proc_macro2::Ident, + f: &TableField<'_>, + member: &OneofMembership<'_>, + aux: &mut Vec, + ) -> Result { + let ctx = scope.ctx; + let field = f.field; + let field_name = field.name.as_deref().unwrap_or(""); + let too_many = || too_many_descriptors(scope); + let enum_ident = self.enum_idents.get(&member.index).ok_or_else(|| { + CodeGenError::Other(format!( + "table codec: {}.{field_name}: the oneof `{}` has no enum", + scope.proto_fqn, member.name + )) + })?; + let prefix = &self.prefix; + let enum_path = quote! { #prefix #enum_ident }; + let oneof_field = ctx.oneof_ident(member.name); + let variant = crate::oneof::oneof_variant_ident(field_name); + let number = f.number; + let payload_kind = format_ident!("{}", f.kind); + let first = self.first[&member.index]; + + // The oneof's descriptor, made when its first member is met. + let group = match self.group_aux.get(&member.index) { + Some(&group) => group, + None => { + let ctor = if self.with_messages.contains(&member.index) { + format_ident!("with_messages") + } else { + format_ident!("new") + }; + aux.push(quote! { + ::buffa::table::Aux::Group(&::buffa::table::OneofVt::#ctor::<#enum_path>( + ::buffa::table::offset_of!(#message, #oneof_field), + #first, + )) + }); + self.group_aux.insert(member.index, aux.len() - 1); + aux.len() - 1 + } + }; + + // What the member's value is: the type the accessors give a pointer + // to, the descriptor of the value, and the value a member has when it + // is first set. + let variant_fqn = format!(".{}.{}.{field_name}", scope.proto_fqn, member.name); + let default = quote! { ::core::default::Default::default() }; + let (slot, value_aux, new) = match f.ty { + Type::TYPE_MESSAGE => { + let child = rust_path_to_tokens(&type_path(scope, field, "message")?); + // A child without a table here is reached through its + // `Message` impl. + let msg_vt = if ctx.uses_table_codec(field_type_name(field)?) { + let child_table = table_path(&unshortened_path(scope, field)?)?; + quote! { ::buffa::table::MsgVt::direct::<#child>(&#child_table) } + } else { + quote! { ::buffa::table::MsgVt::direct_via_message::<#child>() } + }; + let new = if crate::oneof::variant_boxed(ctx, f.ty, &variant_fqn) { + match ctx.pointer_repr(&variant_fqn) { + crate::PointerRepr::Box => quote! { ::buffa::alloc::boxed::Box::default() }, + repr => repr.pointer_new(&child, &default)?, + } + } else { + default + }; + ( + child, + Some(quote! { ::buffa::table::Aux::Msg(&#msg_vt) }), + new, + ) + } + Type::TYPE_ENUM => { + let enum_ty = rust_path_to_tokens(&type_path(scope, field, "enum")?); + let shape = if f.closed_enum { + quote! { ::buffa::table::ImplicitClosed<#enum_ty> } + } else { + quote! { ::buffa::table::ImplicitOpen<#enum_ty> } + }; + ( + quote! { <#shape as ::buffa::table::EnumShape>::Slot }, + Some( + quote! { ::buffa::table::Aux::Enum(&::buffa::table::EnumVt::new::<#shape>()) }, + ), + default, + ) + } + _ => ( + quote! { <::buffa::table::kinds::#payload_kind as ::buffa::table::KindSlot>::Slot }, + None, + default, + ), + }; + let value_aux = value_aux.map_or(Ok(0), |item| { + u16::try_from(self.shared_aux(aux, item)).map_err(|_| too_many()) + })?; + + let arms = self.arms.entry(member.index).or_default(); + arms.number.push(quote! { Self::#variant(_) => #number, }); + // The pointers come from references of the type `slot`, so a variant of + // another type does not compile (a boxed message coerces to its + // message). They are coerced and not cast, which `trivial_casts` + // would flag, and `ptr::from_ref` needs a Rust newer than the MSRV of + // some crates that build this code. + arms.payload.push(quote! { + Self::#variant(v) => { + let value: &#slot = v; + let ptr: *const #slot = value; + ptr.cast::() + } + }); + arms.payload_mut.push(quote! { + Self::#variant(v) => { + let value: &mut #slot = v; + let ptr: *mut #slot = value; + ptr.cast::() + } + }); + arms.with_default + .push(quote! { #number => ::core::option::Option::Some(Self::#variant(#new)), }); + + let leader = number == first; + let group = u16::try_from(group).map_err(|_| too_many())?; + aux.push(quote! { + ::buffa::table::Aux::Member(::buffa::table::Member::new( + #group, + ::buffa::table::Kind::#payload_kind, + #value_aux, + )) + }); + let member_aux = u16::try_from(aux.len() - 1).map_err(|_| too_many())?; + Ok(quote! { + ::buffa::__table_entry!( + #message, #oneof_field, oneof(#payload_kind, #leader), #number, + aux = #member_aux, + slot = ::core::option::Option<#enum_path> + ) + }) + } + + /// The `OneofEnum` implementation of every oneof enum that had a member. + fn into_impls(self) -> Vec { + let prefix = self.prefix; + self.arms + .into_iter() + .map(|(index, arms)| { + let enum_ident = &self.enum_idents[&index]; + let OneofArms { + number, + payload, + payload_mut, + with_default, + } = arms; + quote! { + impl ::buffa::table::OneofEnum for #prefix #enum_ident { + fn number(&self) -> u32 { + match self { + #(#number)* + } + } + + fn payload(&self) -> *const u8 { + match self { + #(#payload)* + } + } + + fn payload_mut(&mut self) -> *mut u8 { + match self { + #(#payload_mut)* + } + } + + fn with_default(number: u32) -> ::core::option::Option { + match number { + #(#with_default)* + _ => ::core::option::Option::None, + } + } + } + } + }) + .collect() + } +} diff --git a/buffa-codegen/src/table_plan.rs b/buffa-codegen/src/table_plan.rs index 3bfe446e..af87fd16 100644 --- a/buffa-codegen/src/table_plan.rs +++ b/buffa-codegen/src/table_plan.rs @@ -50,10 +50,21 @@ pub(crate) struct TableField<'a> { pub(crate) number: u32, pub(crate) ty: Type, pub(crate) card: Card, - /// The name of the `buffa::table::Kind` variant of this field. + /// The name of the `buffa::table::Kind` variant of this field's value. For + /// a oneof member, the emitter wraps it in the member kind that `oneof` + /// says it needs. pub(crate) kind: String, /// For an enum field: whether the enum is closed. pub(crate) closed_enum: bool, + /// For a oneof member, the oneof it belongs to. + pub(crate) oneof: Option>, +} + +/// The oneof that a [`TableField`] is a member of. +pub(crate) struct OneofMembership<'a> { + /// The index of the oneof in the message's `oneof_decl`. + pub(crate) index: usize, + pub(crate) name: &'a str, } /// The `Kind` variant name of a field type, or `None` for a group, which has @@ -135,12 +146,19 @@ pub(crate) fn table_fields<'a>( let mut fields = Vec::with_capacity(msg.field.len()); for f in &msg.field { let name = f.name.as_deref().unwrap_or(""); - if is_real_oneof_member(f) { - return Err(ineligible( - "has a oneof", - format!("field `{name}` is in a oneof"), - )); - } + let oneof = if is_real_oneof_member(f) { + let index = f.oneof_index.and_then(|i| usize::try_from(i).ok()); + let decl = index.and_then(|i| msg.oneof_decl.get(i)); + let Some((index, decl)) = index.zip(decl) else { + return Err(ineligible( + "has a field in a oneof that does not exist", + format!("field `{name}` names a oneof that the message does not declare"), + )); + }; + Some((index, decl.name.as_deref().unwrap_or(""))) + } else { + None + }; if find_map_entry(msg, f).is_some() { return Err(ineligible( "has a map field", @@ -163,7 +181,10 @@ pub(crate) fn table_fields<'a>( } let number = crate::impl_message::validated_field_number(f) .map_err(|e| ineligible("has an invalid field number", e.to_string()))?; - let card = if repeated { + // A oneof member's value is written whenever the member is set. + let card = if oneof.is_some() { + Card::Required + } else if repeated { if is_field_packed(f, features) { Card::Packed } else { @@ -196,6 +217,7 @@ pub(crate) fn table_fields<'a>( card, kind, closed_enum, + oneof: oneof.map(|(index, name)| OneofMembership { index, name }), }); } fields.sort_by_key(|f| f.number); diff --git a/buffa-codegen/src/tests/table_codec.rs b/buffa-codegen/src/tests/table_codec.rs index f48633a6..3635a5ef 100644 --- a/buffa-codegen/src/tests/table_codec.rs +++ b/buffa-codegen/src/tests/table_codec.rs @@ -26,21 +26,12 @@ fn scalar(name: &str, number: i32, ty: Type) -> FieldDescriptorProto { /// Package `t` with: /// /// - `Plain`, `Leaf`, and `HasLeaf` (holds a `Leaf`), which can use the table; -/// - `Oneofy` (has a oneof), which cannot, and `HasOneofy` (holds an `Oneofy`), -/// which can, because a table message may hold a message that has no table; +/// - `CustomStr` (has a string with a custom type, which the table does not +/// support), which cannot, and `HoldsCustomStr` (holds a `CustomStr`), which +/// can, because a table message may hold a message that has no table; /// - `Outer` with a nested `Inner`, both plain. fn schema() -> FileDescriptorProto { - let mut oneofy = message( - "Oneofy", - vec![FieldDescriptorProto { - oneof_index: Some(0), - ..scalar("a", 1, Type::TYPE_INT32) - }], - ); - oneofy.oneof_decl = vec![OneofDescriptorProto { - name: Some("choice".to_string()), - ..Default::default() - }]; + let custom_str = message("CustomStr", vec![scalar("a", 1, Type::TYPE_STRING)]); let mut outer = message("Outer", vec![scalar("x", 1, Type::TYPE_INT32)]); outer.nested_type = vec![message("Inner", vec![scalar("y", 1, Type::TYPE_STRING)])]; FileDescriptorProto { @@ -55,20 +46,40 @@ fn schema() -> FileDescriptorProto { ), message("Leaf", vec![scalar("x", 1, Type::TYPE_INT32)]), message("HasLeaf", vec![message_field("leaf", 1, ".t.Leaf")]), - oneofy, - message("HasOneofy", vec![message_field("o", 1, ".t.Oneofy")]), + custom_str, + message( + "HoldsCustomStr", + vec![message_field("o", 1, ".t.CustomStr")], + ), outer, ], ..proto3_file("t.proto") } } +/// `.t.CustomStr.a` has a custom string type, which the table cannot use. +/// [`run`] applies it to every config it is given. +fn with_custom_string(config: &CodeGenConfig) -> CodeGenConfig { + let mut config = config.clone(); + config.string_fields.push(( + ".t.CustomStr.a".to_string(), + StringRepr::Custom("crate::Str".to_string()), + )); + config +} + fn run(config: &CodeGenConfig) -> Result<(String, Vec), CodeGenError> { - let (files, warnings) = - generate_with_diagnostics(&[schema()], &["t.proto".to_string()], config)?; + let (files, warnings) = generate_with_diagnostics( + &[schema()], + &["t.proto".to_string()], + &with_custom_string(config), + )?; Ok((joined(&files), warnings)) } +/// The reason `CustomStr` cannot use the table. +const CUSTOM: &str = "has a field with a custom string, bytes or collection type"; + fn table_config(strategy: CodecStrategy) -> CodeGenConfig { CodeGenConfig { codec_strategy: strategy, @@ -145,15 +156,25 @@ fn the_global_setting_gives_a_table_to_every_message_that_can_use_one() { let (code, warnings) = run(&table_config(CodecStrategy::Table)).unwrap(); assert_eq!( tables(&code), - ["Plain", "Leaf", "HasLeaf", "HasOneofy", "Outer", "Inner"] + [ + "Plain", + "Leaf", + "HasLeaf", + "HoldsCustomStr", + "Outer", + "Inner" + ] ); - // The one with a oneof falls back, and one warning covers the run. + // The one with a custom string falls back, and one warning covers the run. let (counts, reasons) = summary(&warnings); assert_eq!(counts, (1, 7)); - assert_eq!(reasons, [("has a oneof", 1)]); + assert_eq!(reasons, [(CUSTOM, 1)]); let text = table_warnings(&warnings)[0].to_string(); assert!(text.starts_with("1 of 7 messages selected for the table codec")); - assert!(text.contains("has a oneof (1: .t.Oneofy)"), "{text}"); + assert!( + text.contains(&format!("{CUSTOM} (1: .t.CustomStr)")), + "{text}" + ); assert!(text.contains("codec_strategy_in==unrolled"), "{text}"); } @@ -188,17 +209,20 @@ fn a_child_message_is_referenced_through_its_own_table() { fn a_child_without_a_table_is_reached_through_its_message_impl() { let (code, _) = run(&table_config(CodecStrategy::Table)).unwrap(); let code = squashed(&code); - // `HasOneofy` is a table message that holds `Oneofy`, which is unrolled. - let holder = code.split("static__BUFFA_TABLE_HasOneofy").nth(1).unwrap(); + // `HoldsCustomStr` is a table message that holds `CustomStr`, which is unrolled. + let holder = code + .split("static__BUFFA_TABLE_HoldsCustomStr") + .nth(1) + .unwrap(); let holder = holder - .split("impl::buffa::MessageforHasOneofy") + .split("impl::buffa::MessageforHoldsCustomStr") .next() .unwrap(); assert!( - holder.contains("Aux::Msg(&::buffa::table::MsgVt::new_via_message::<::buffa::MessageField>>())"), + holder.contains("Aux::Msg(&::buffa::table::MsgVt::new_via_message::<::buffa::MessageField>>())"), "{holder}" ); - assert!(!holder.contains("__BUFFA_TABLE_Oneofy"), "{holder}"); + assert!(!holder.contains("__BUFFA_TABLE_CustomStr"), "{holder}"); } #[test] @@ -208,20 +232,23 @@ fn a_repeated_child_without_a_table_is_reached_through_its_message_impl() { let (files, _) = generate_with_diagnostics( &[file], &["t.proto".to_string()], - &table_config(CodecStrategy::Table), + &with_custom_string(&table_config(CodecStrategy::Table)), ) .unwrap(); let code = squashed(&joined(&files)); - let holder = code.split("static__BUFFA_TABLE_HasOneofy").nth(1).unwrap(); + let holder = code + .split("static__BUFFA_TABLE_HoldsCustomStr") + .nth(1) + .unwrap(); let holder = holder - .split("impl::buffa::MessageforHasOneofy") + .split("impl::buffa::MessageforHoldsCustomStr") .next() .unwrap(); assert!( - holder.contains("Aux::Rep(&::buffa::table::RepVt::new_via_message::())"), + holder.contains("Aux::Rep(&::buffa::table::RepVt::new_via_message::())"), "{holder}" ); - assert!(holder.contains("Vec"), "{holder}"); + assert!(holder.contains("Vec"), "{holder}"); } #[test] @@ -296,7 +323,10 @@ fn the_last_matching_rule_wins() { }; let (code, _) = run(&config).unwrap(); // `Outer` is unrolled, though its nested message is a table. - assert_eq!(tables(&code), ["Leaf", "HasLeaf", "HasOneofy", "Inner"]); + assert_eq!( + tables(&code), + ["Leaf", "HasLeaf", "HoldsCustomStr", "Inner"] + ); } #[test] @@ -350,24 +380,29 @@ fn a_rule_for_a_message_does_not_select_the_messages_it_holds() { fn every_exact_path_rule_that_cannot_be_honoured_is_reported_at_once() { let mut file = schema(); let mut second = file.message_type[3].clone(); - second.name = Some("Oneofy2".to_string()); + second.name = Some("CustomStr2".to_string()); file.message_type.push(second); let config = CodeGenConfig { codec_strategy_in: vec![ - (".t.Oneofy".to_string(), CodecStrategy::Table), - (".t.Oneofy2".to_string(), CodecStrategy::Table), - (".t.HasOneofy".to_string(), CodecStrategy::Table), + (".t.CustomStr".to_string(), CodecStrategy::Table), + (".t.CustomStr2".to_string(), CodecStrategy::Table), + (".t.HoldsCustomStr".to_string(), CodecStrategy::Table), ], ..Default::default() }; + let mut config = with_custom_string(&config); + config.string_fields.push(( + ".t.CustomStr2.a".to_string(), + StringRepr::Custom("crate::Str".to_string()), + )); let err = generate_with_diagnostics(&[file], &["t.proto".to_string()], &config) .unwrap_err() .to_string(); - // `HasOneofy` can use the table, so it is not among them. + // `HoldsCustomStr` can use the table, so it is not among them. assert!( - err.contains("rule '.t.Oneofy'") - && err.contains("rule '.t.Oneofy2'") - && !err.contains("rule '.t.HasOneofy'"), + err.contains("rule '.t.CustomStr'") + && err.contains("rule '.t.CustomStr2'") + && !err.contains("rule '.t.HoldsCustomStr'"), "{err}" ); } @@ -375,12 +410,15 @@ fn every_exact_path_rule_that_cannot_be_honoured_is_reported_at_once() { #[test] fn an_exact_path_rule_for_a_message_that_cannot_use_the_table_is_an_error() { let config = CodeGenConfig { - codec_strategy_in: vec![(".t.Oneofy".to_string(), CodecStrategy::Table)], + codec_strategy_in: vec![(".t.CustomStr".to_string(), CodecStrategy::Table)], ..Default::default() }; let err = run(&config).unwrap_err().to_string(); assert!(err.contains("cannot use it"), "{err}"); - assert!(err.contains("field `a` is in a oneof"), "{err}"); + assert!( + err.contains("field `a` has a custom string, bytes or collection type"), + "{err}" + ); } #[test] @@ -392,7 +430,14 @@ fn a_broad_rule_that_covers_such_a_message_only_warns() { let (code, warnings) = run(&config).unwrap(); assert_eq!( tables(&code), - ["Plain", "Leaf", "HasLeaf", "HasOneofy", "Outer", "Inner"] + [ + "Plain", + "Leaf", + "HasLeaf", + "HoldsCustomStr", + "Outer", + "Inner" + ] ); // Messages a rule selects are counted like the ones the global setting does. assert_eq!(summary(&warnings).0, (1, 7)); @@ -409,7 +454,7 @@ fn setting_a_message_to_unrolled_does_not_affect_the_messages_that_hold_it() { // anyway. The warning counts the messages selected, which leaves out `Leaf`. assert_eq!( tables(&code), - ["Plain", "HasLeaf", "HasOneofy", "Outer", "Inner"] + ["Plain", "HasLeaf", "HoldsCustomStr", "Outer", "Inner"] ); assert_eq!(summary(&warnings).0, (1, 6)); @@ -418,7 +463,7 @@ fn setting_a_message_to_unrolled_does_not_affect_the_messages_that_hold_it() { let config = CodeGenConfig { codec_strategy_in: vec![ (".t.Leaf".to_string(), CodecStrategy::Unrolled), - (".t.Oneofy".to_string(), CodecStrategy::Unrolled), + (".t.CustomStr".to_string(), CodecStrategy::Unrolled), ], ..table_config(CodecStrategy::Table) }; @@ -463,8 +508,12 @@ fn a_message_type_from_another_crate_is_reached_through_its_message_impl() { extern_paths: vec![(".other".to_string(), "::other_crate".to_string())], ..table_config(CodecStrategy::Table) }; - let (files, warnings) = - generate_with_diagnostics(&[file, other], &["t.proto".to_string()], &config).unwrap(); + let (files, warnings) = generate_with_diagnostics( + &[file, other], + &["t.proto".to_string()], + &with_custom_string(&config), + ) + .unwrap(); let code = joined(&files); assert!(tables(&code).contains(&"HasLeaf".to_string()), "{code}"); assert!(tables(&code).contains(&"Leaf".to_string())); @@ -580,7 +629,7 @@ fn the_warning_texts_say_what_to_do() { message names" ); let reason = TableCodecFallbackReason { - reason: "has a oneof".to_string(), + reason: "has a map field".to_string(), messages: [".t.A", ".t.B", ".t.C", ".t.D", ".t.E"] .map(String::from) .to_vec(), @@ -593,7 +642,7 @@ fn the_warning_texts_say_what_to_do() { let text = summary.to_string(); // Three messages are named, and the rest are counted. assert!( - text.contains("has a oneof (5: .t.A, .t.B, .t.C, and 2 more)"), + text.contains("has a map field (5: .t.A, .t.B, .t.C, and 2 more)"), "{text}" ); } @@ -761,24 +810,17 @@ fn blob_config() -> CodeGenConfig { fn a_message_that_holds_a_message_with_a_bytes_type_stays_unrolled() { let (code, warnings) = run_bytes(&blob_config()).unwrap(); // The holders of `Blob` fall back, directly, in a list, transitively, and - // through a oneof member and a map value, though the last two also hold - // messages that cannot use the table for other reasons. + // through a oneof member and a map value. assert_eq!( tables(&code), ["PlainBytes", "HoldsPlain", "Leaf", "HoldsLeaf"] ); let (counts, reasons) = summary(&warnings); assert_eq!(counts, (8, 12)); - // `OneofBlob` and `MapBlob` fall back for their oneof and map, so they are - // not counted as holders. + // `MapBlob` falls back for its map, so it is not counted as a holder. assert_eq!( reasons, - [ - (HOLDS_BYTES, 5), - (CUSTOM_FIELD, 1), - ("has a oneof", 1), - ("has a map field", 1), - ] + [(HOLDS_BYTES, 6), (CUSTOM_FIELD, 1), ("has a map field", 1)] ); let text = table_warnings(&warnings)[0].to_string(); assert!(text.contains(HOLDS_BYTES), "{text}"); @@ -799,7 +841,7 @@ fn a_message_with_a_plain_bytes_field_is_unaffected() { } #[test] -fn a_child_set_to_unrolled_without_bytes_still_lets_its_holder_use_the_table() { +fn a_holder_of_a_child_set_to_unrolled_without_bytes_is_a_table() { let config = CodeGenConfig { codec_strategy_in: vec![(".b.Leaf".to_string(), CodecStrategy::Unrolled)], ..blob_config() @@ -819,7 +861,7 @@ fn a_child_set_to_unrolled_that_has_a_bytes_type_keeps_its_holder_unrolled() { }; let (code, warnings) = run_bytes(&config).unwrap(); assert!(!tables(&code).contains(&"HasBlob".to_string()), "{code}"); - assert!(summary(&warnings).1.contains(&(HOLDS_BYTES, 5))); + assert!(summary(&warnings).1.contains(&(HOLDS_BYTES, 6))); } #[test] @@ -1019,3 +1061,286 @@ fn a_map_with_a_bytes_key_keeps_vec_values_so_its_holder_uses_the_table() { "{tables:?}" ); } + +/// Package `o` with `WithOneof { oneof choice { int32 a = 1; string b = 2; Leaf leaf = 5; }; int32 c = 3; }`. +fn oneof_schema() -> FileDescriptorProto { + let member = |name, number, ty| FieldDescriptorProto { + oneof_index: Some(0), + ..scalar(name, number, ty) + }; + let mut with_oneof = message( + "WithOneof", + vec![ + member("a", 1, Type::TYPE_INT32), + member("b", 2, Type::TYPE_STRING), + FieldDescriptorProto { + oneof_index: Some(0), + ..message_field("leaf", 5, ".o.Leaf") + }, + scalar("c", 3, Type::TYPE_INT32), + ], + ); + with_oneof.oneof_decl = vec![OneofDescriptorProto { + name: Some("choice".to_string()), + ..Default::default() + }]; + FileDescriptorProto { + package: Some("o".to_string()), + message_type: vec![ + message("Leaf", vec![scalar("x", 1, Type::TYPE_INT32)]), + with_oneof, + ], + ..proto3_file("o.proto") + } +} + +fn run_oneof(config: &CodeGenConfig) -> (String, Vec) { + let (files, warnings) = + generate_with_diagnostics(&[oneof_schema()], &["o.proto".to_string()], config).unwrap(); + (joined(&files), warnings) +} + +#[test] +fn a_message_with_a_oneof_gets_a_table_with_one_entry_per_member() { + let (code, warnings) = run_oneof(&table_config(CodecStrategy::Table)); + assert_eq!(tables(&code), ["Leaf", "WithOneof"]); + assert!(table_warnings(&warnings).is_empty(), "{warnings:?}"); + let code = squashed(&code); + let table = code.split("static__BUFFA_TABLE_WithOneof").nth(1).unwrap(); + let table = table.split("impl::buffa::Message").next().unwrap(); + // The members carry their payload kinds, in field-number order with `c` + // (3) between them, and all name the field that holds the oneof. Only + // the lowest leads. + for entry in [ + "(WithOneof,choice,oneof(Int32Required,true),1u32,", + "(WithOneof,choice,oneof(StrRequired,false),2u32,", + "(WithOneof,c,Int32Implicit,3u32)", + "(WithOneof,choice,oneof(MsgSingular,false),5u32,", + ] { + assert!(table.contains(entry), "{entry} in {table}"); + } + // One descriptor for the oneof, at the lowest member number, and a member + // item for each of the three. + assert_eq!(table.matches("Aux::Group(").count(), 1, "{table}"); + assert!(table.contains("OneofVt::with_messages::<"), "{table}"); + assert!( + table.contains("offset_of!(WithOneof,choice),1u32"), + "{table}" + ); + assert_eq!(table.matches("Member::new(0u16,").count(), 3, "{table}"); + // The message member's child is reached through its own table. + assert!( + table.contains("MsgVt::direct::(&__BUFFA_TABLE_Leaf)"), + "{table}" + ); +} + +#[test] +fn oneof_members_of_one_type_share_a_descriptor() { + // `x` and `y` are `Leaf`, `p` and `q` are `Color`, and `z` is a `Leaf` in + // a second oneof, which shares the descriptor of the first's. + let mut file = oneof_schema(); + file.enum_type.push(EnumDescriptorProto { + name: Some("Color".to_string()), + value: vec![EnumValueDescriptorProto { + name: Some("RED".to_string()), + number: Some(0), + ..Default::default() + }], + ..Default::default() + }); + let with_oneof = &mut file.message_type[1]; + let member = |field: FieldDescriptorProto, oneof| FieldDescriptorProto { + oneof_index: Some(oneof), + ..field + }; + with_oneof.field.extend([ + member(message_field("x", 6, ".o.Leaf"), 0), + member(message_field("y", 7, ".o.Leaf"), 0), + member( + FieldDescriptorProto { + type_name: Some(".o.Color".to_string()), + ..scalar("p", 8, Type::TYPE_ENUM) + }, + 0, + ), + member( + FieldDescriptorProto { + type_name: Some(".o.Color".to_string()), + ..scalar("q", 9, Type::TYPE_ENUM) + }, + 0, + ), + member(message_field("z", 10, ".o.Leaf"), 1), + ]); + with_oneof.oneof_decl.push(OneofDescriptorProto { + name: Some("other".to_string()), + ..Default::default() + }); + let (files, _) = generate_with_diagnostics( + &[file], + &["o.proto".to_string()], + &table_config(CodecStrategy::Table), + ) + .unwrap(); + let code = squashed(&joined(&files)); + let table = code.split("static__BUFFA_TABLE_WithOneof").nth(1).unwrap(); + let table = table.split("impl::buffa::Message").next().unwrap(); + // Four members are a `Leaf` and two a `Color`, with one descriptor each. + assert_eq!(table.matches("Aux::Msg(").count(), 1, "{table}"); + assert_eq!(table.matches("Aux::Enum(").count(), 1, "{table}"); + assert_eq!(table.matches("Aux::Member(").count(), 8, "{table}"); + assert_eq!(table.matches("Aux::Group(").count(), 2, "{table}"); + assert_eq!( + table.matches("OneofVt::with_messages::<").count(), + 2, + "{table}" + ); +} + +#[test] +fn only_a_oneof_with_a_message_member_is_built_to_decode_messages_in_place() { + // `choice` has a message member; `plain` has only an int32 and a string. + let mut file = oneof_schema(); + let with_oneof = &mut file.message_type[1]; + with_oneof.field.extend([ + FieldDescriptorProto { + oneof_index: Some(1), + ..scalar("p", 6, Type::TYPE_INT32) + }, + FieldDescriptorProto { + oneof_index: Some(1), + ..scalar("q", 7, Type::TYPE_STRING) + }, + ]); + with_oneof.oneof_decl.push(OneofDescriptorProto { + name: Some("plain".to_string()), + ..Default::default() + }); + let (files, _) = generate_with_diagnostics( + &[file], + &["o.proto".to_string()], + &table_config(CodecStrategy::Table), + ) + .unwrap(); + let code = squashed(&joined(&files)); + let table = code.split("static__BUFFA_TABLE_WithOneof").nth(1).unwrap(); + let table = table.split("impl::buffa::Message").next().unwrap(); + assert_eq!( + table.matches("OneofVt::with_messages::<").count(), + 1, + "{table}" + ); + assert_eq!(table.matches("OneofVt::new::<").count(), 1, "{table}"); + assert!( + table.contains("OneofVt::new::<__buffa::oneof::with_oneof::Plain>"), + "{table}" + ); +} + +#[test] +fn a_oneof_member_whose_child_has_no_table_is_reached_through_its_message_impl() { + // `Leaf` is set to unrolled, so `WithOneof` reaches it through its + // `Message` impl, and stays a table message. + let config = CodeGenConfig { + codec_strategy_in: vec![(".o.Leaf".to_string(), CodecStrategy::Unrolled)], + ..table_config(CodecStrategy::Table) + }; + let (code, warnings) = run_oneof(&config); + assert_eq!(tables(&code), ["WithOneof"]); + assert!(table_warnings(&warnings).is_empty(), "{warnings:?}"); + let code = squashed(&code); + let table = code.split("static__BUFFA_TABLE_WithOneof").nth(1).unwrap(); + let table = table.split("impl::buffa::Message").next().unwrap(); + assert!( + table.contains("MsgVt::direct_via_message::()"), + "{table}" + ); + assert!(!table.contains("MsgVt::direct::<"), "{table}"); +} + +#[test] +fn a_oneof_member_from_another_crate_is_reached_through_its_message_impl() { + let mut file = oneof_schema(); + file.message_type[1].field[2].type_name = Some(".other.Foreign".to_string()); + let other = FileDescriptorProto { + package: Some("other".to_string()), + message_type: vec![message("Foreign", vec![scalar("x", 1, Type::TYPE_INT32)])], + ..proto3_file("other.proto") + }; + let config = CodeGenConfig { + extern_paths: vec![(".other".to_string(), "::other_crate".to_string())], + ..table_config(CodecStrategy::Table) + }; + let (files, warnings) = + generate_with_diagnostics(&[file, other], &["o.proto".to_string()], &config).unwrap(); + let code = joined(&files); + assert!(tables(&code).contains(&"WithOneof".to_string()), "{code}"); + assert!(table_warnings(&warnings).is_empty(), "{warnings:?}"); + assert!( + squashed(&code).contains("MsgVt::direct_via_message::<::other_crate::Foreign>()"), + "{code}" + ); +} + +#[test] +fn the_oneof_enum_implements_the_accessors_the_table_reads_it_through() { + let (code, _) = run_oneof(&table_config(CodecStrategy::Table)); + let code = squashed(&code); + let imp = code + .split("impl::buffa::table::OneofEnumfor__buffa::oneof::with_oneof::Choice{") + .nth(1) + .and_then(|rest| rest.split("impl::buffa::ExtensionSet").next()) + .unwrap(); + assert!(imp.contains("Self::A(_)=>1u32"), "{imp}"); + assert!(imp.contains("Self::B(_)=>2u32"), "{imp}"); + assert!(imp.contains("Self::Leaf(_)=>5u32"), "{imp}"); + // A boxed message is reached through its box (by deref coercion), and a + // new one starts empty. + assert!( + imp.contains("Self::Leaf(v)=>{letvalue:&Leaf=v;letptr:*constLeaf=value;ptr.cast::()}"), + "{imp}" + ); + assert!( + imp.contains("Self::Leaf(v)=>{letvalue:&mutLeaf=v;letptr:*mutLeaf=value;ptr.cast::()}"), + "{imp}" + ); + // Neither is a cast of the reference, which `trivial_casts` flags. + assert!(!imp.contains("valueas"), "{imp}"); + assert!( + imp.contains( + "5u32=>{::core::option::Option::Some(Self::Leaf(::buffa::alloc::boxed::Box::default()" + ), + "{imp}" + ); + assert!(imp.contains("_=>::core::option::Option::None"), "{imp}"); + // None of it is unsafe: the generated crate may forbid unsafe code. + assert!(!imp.contains("unsafe"), "{imp}"); +} + +#[test] +fn an_unrolled_message_with_a_oneof_has_no_oneof_enum_impl() { + let (code, warnings) = run_oneof(&CodeGenConfig::default()); + assert!(tables(&code).is_empty()); + assert!(!code.contains("OneofEnum")); + assert!(table_warnings(&warnings).is_empty()); +} + +#[test] +fn a_oneof_member_with_a_custom_type_keeps_the_message_unrolled() { + let config = CodeGenConfig { + string_fields: vec![( + ".o.WithOneof.b".to_string(), + StringRepr::Custom("crate::Str".to_string()), + )], + ..table_config(CodecStrategy::Table) + }; + let (code, warnings) = run_oneof(&config); + assert_eq!(tables(&code), ["Leaf"]); + assert!( + table_warnings(&warnings)[0] + .to_string() + .contains(".o.WithOneof"), + "{warnings:?}" + ); +} diff --git a/buffa-test/build.rs b/buffa-test/build.rs index 5c689484..89281d5c 100644 --- a/buffa-test/build.rs +++ b/buffa-test/build.rs @@ -4,6 +4,18 @@ /// below the package, such as `Hot`), which stay unrolled. A test compares the /// two codecs on the same schema. `file` names the schema in messages. fn compile_both_codecs(file: &str, source: &str, base: &str, unrolled: &[&str]) { + compile_both_codecs_with(file, source, base, unrolled, |config| config); +} + +/// [`compile_both_codecs`] with `configure` applied to both builds, for the +/// options that change what the two codecs generate the same way. +fn compile_both_codecs_with( + file: &str, + source: &str, + base: &str, + unrolled: &[&str], + configure: fn(buffa_build::Config) -> buffa_build::Config, +) { let out = std::path::PathBuf::from(std::env::var("OUT_DIR").expect("OUT_DIR")); let package = format!("package {base};"); assert!(source.contains(&package), "{file} must declare `{package}`"); @@ -21,15 +33,17 @@ fn compile_both_codecs(file: &str, source: &str, base: &str, unrolled: &[&str]) .iter() .map(|path| format!(".{base}{suffix}.{path}")) .collect(); - buffa_build::Config::new() - .files(&[renamed]) - .includes(&[&out]) - .generate_json(true) - .generate_text(true) - .codec_strategy(strategy) - .codec_strategy_in(buffa_build::CodecStrategy::Unrolled, &rules) - .compile() - .unwrap_or_else(|e| panic!("buffa_build failed for {file} ({suffix}): {e}")); + configure( + buffa_build::Config::new() + .files(&[renamed]) + .includes(&[&out]) + .generate_json(true) + .generate_text(true) + .codec_strategy(strategy) + .codec_strategy_in(buffa_build::CodecStrategy::Unrolled, &rules), + ) + .compile() + .unwrap_or_else(|e| panic!("buffa_build failed for {file} ({suffix}): {e}")); } } @@ -61,7 +75,8 @@ fn compile_extern_children() { &holder, format!( "syntax = \"proto3\";\npackage xf{suffix};\nimport \"xe.proto\";\n\ - message Holder {{ xe.Leaf leaf = 1; repeated xe.Leaf leaves = 2; int32 tail = 3; }}\n" + message Holder {{ xe.Leaf leaf = 1; repeated xe.Leaf leaves = 2; int32 tail = 3;\n\ + oneof pick {{ int32 n = 4; xe.Leaf pl = 5; }} }}\n" ), ) .expect("write proto"); @@ -79,9 +94,9 @@ fn compile_extern_children() { /// ways: unrolled (`xau`, `xbu`), table (`xat`, `xbt`), and table with /// `file_per_package` and `idiomatic_imports` (`xati`, `xbti`), which shortens /// the paths of types in other packages and so changes what a table path may -/// be. `Cold`, which `Holder` holds singly and in a list, stays unrolled in -/// all three, so the table holders reach it through its `Message` impl with -/// the shortened path. +/// be. `Cold`, which `Holder` holds singly, in a list and in a oneof member, +/// stays unrolled in all three, so the table holders reach it through its +/// `Message` impl with the shortened path. fn compile_cross_package() { let out = std::path::PathBuf::from(std::env::var("OUT_DIR").expect("OUT_DIR")); let sources = |suffix: &str| { @@ -101,6 +116,7 @@ fn compile_cross_package() { xa{suffix}.Cold cold = 5;\n\ repeated xa{suffix}.Cold colds = 6;\n\ message Sub {{ xa{suffix}.Leaf l = 1; }}\n\ + oneof pick {{ int32 n = 7; xa{suffix}.Leaf pl = 8; xa{suffix}.Wrap pw = 9; Sub ps = 10; xa{suffix}.Cold pc = 11; }}\n\ }}\n" ); (dep, user) @@ -169,14 +185,25 @@ fn wide_proto() -> String { } proto.push_str("}\n"); } + // A oneof in a message with more entries than a dense array can index, so + // the table searches for its members. + proto.push_str("enum WideColor {\n WIDE_UNSPECIFIED = 0;\n WIDE_RED = 1;\n}\n"); + proto.push_str("message WideOneof {\n"); + for n in 1..=254 { + proto.push_str(&format!(" int32 f{n} = {n};\n")); + } + proto.push_str( + " oneof pick {\n int32 a = 255;\n string b = 256;\n Leaf c = 257;\n WideColor d = 258;\n }\n}\n", + ); proto } /// Compile `protos/` as package `x` with the table codec and the /// options that change the names and fields the table refers to: a type name -/// prefix (`RpcScalars`), no unknown-field slot, boxed message fields, and the -/// lazy view and reflection code that read the same fields. -fn compile_table_with_options(file: &str, base: &str) { +/// prefix (`RpcScalars`), no unknown-field slot, boxed message fields, message +/// members of the oneofs under `unboxed` stored inline, and the lazy view and +/// reflection code that read the same fields. +fn compile_table_with_options(file: &str, base: &str, unboxed: &[&str]) { let out = std::path::PathBuf::from(std::env::var("OUT_DIR").expect("OUT_DIR")); let source = read_proto(file); let renamed = out.join(format!("{base}x.proto")); @@ -192,6 +219,7 @@ fn compile_table_with_options(file: &str, base: &str) { .type_name_prefix("Rpc") .preserve_unknown_fields(false) .box_type(buffa_build::PointerRepr::Box) + .unbox_oneof_in(unboxed) .lazy_views(true) .reflect_mode(buffa_build::ReflectMode::VTable) .compile() @@ -250,6 +278,37 @@ fn main() { "br", &["Hot"], ); + compile_both_codecs( + "table_codec4.proto", + &read_proto("table_codec4.proto"), + "tc4", + &[], + ); + // A custom pointer for the message members of oneofs, and names that + // `idiomatic_field_names` changes. Without the text format, whose + // decoder of a oneof message member wraps it in a `Box` whatever the + // pointer is, for both codecs. + compile_both_codecs_with( + "table_codec5.proto", + &read_proto("table_codec5.proto"), + "tc5", + &[], + |config| { + config + .box_type_custom("crate::box_type::CustomBox<*>") + .idiomatic_field_names(true) + .generate_text(false) + }, + ); + // A closed enum in a oneof, with nowhere to keep a number it lacks. + compile_both_codecs_with( + "table_codec6.proto", + &read_proto("table_codec6.proto"), + "tc6", + &[], + |config| config.preserve_unknown_fields(false), + ); + compile_cross_package(); compile_extern_children(); // `bytes` fields as `bytes::Bytes`, which the messages that hold one @@ -261,7 +320,19 @@ fn main() { .use_bytes_type() .compile() .expect("buffa_build failed for table_bytes.proto"); - compile_table_with_options("table_codec.proto", "tc"); + compile_table_with_options("table_codec.proto", "tc", &[]); + // Not `Tree` and `Pair`, which recurse through their oneof. + compile_table_with_options( + "table_codec4.proto", + "tc4", + &[ + ".tc4x.Kinds", + ".tc4x.Interleaved", + ".tc4x.Sparse", + ".tc4x.Outer", + ".tc4x.Twins", + ], + ); } // Basic proto — the original test file. Also the codegen target for diff --git a/buffa-test/protos/table_bridge.proto b/buffa-test/protos/table_bridge.proto index 3bfda171..176c8f3d 100644 --- a/buffa-test/protos/table_bridge.proto +++ b/buffa-test/protos/table_bridge.proto @@ -55,3 +55,20 @@ message Wkt { repeated google.protobuf.Value values = 12; Cold cold = 13; } + +// A oneof whose members are table messages, a message that `brt` leaves +// unrolled, and well-known types, which a table message reaches through its +// `Message` impl. +message Pick { + oneof choice { + int32 n = 1; + Leaf leaf = 2; + Hot hot = 3; + Cold cold = 4; + google.protobuf.Timestamp ts = 5; + google.protobuf.Any any = 6; + google.protobuf.Value val = 7; + string s = 8; + } + Leaf tail = 9; +} diff --git a/buffa-test/protos/table_bytes.proto b/buffa-test/protos/table_bytes.proto index 96e0347f..97f3e368 100644 --- a/buffa-test/protos/table_bytes.proto +++ b/buffa-test/protos/table_bytes.proto @@ -4,8 +4,9 @@ syntax = "proto3"; // and `bytes` fields stored as `bytes::Bytes`. A table decodes from a // contiguous slice, where `Bytes` fields could not alias the input, so `Blob`, // which has such fields, is unrolled, and so are `HoldsBlob`, which holds -// `Blob`, and `HoldsBlobs`, which holds `HoldsBlob`. `Plain` and `HoldsPlain` -// have no `bytes` field and are tables. +// `Blob`, `HoldsBlobs`, which holds `HoldsBlob`, `HoldsBlobInOneof`, which +// holds `Blob` in a oneof member, and `HoldsPick`, which holds that. `Plain` +// and `HoldsPlain` have no `bytes` field and are tables. package tbz; message Blob { @@ -23,6 +24,17 @@ message HoldsBlobs { HoldsBlob inner = 1; } +message HoldsBlobInOneof { + oneof pick { + Blob blob = 1; + int32 n = 2; + } +} + +message HoldsPick { + HoldsBlobInOneof inner = 1; +} + message Plain { int32 x = 1; } diff --git a/buffa-test/protos/table_codec.proto b/buffa-test/protos/table_codec.proto index d0981c1b..6fb3898a 100644 --- a/buffa-test/protos/table_codec.proto +++ b/buffa-test/protos/table_codec.proto @@ -102,9 +102,7 @@ message Sparse { message Empty {} -// A oneof and a map, which the table cannot handle, so `WithOneof` and -// `WithMap` stay unrolled when the table is requested. `HoldsOneof` holds both -// and is a table message, which reaches them through their `Message` impl. +// A oneof, which the table handles. message WithOneof { oneof choice { int32 a = 1; @@ -114,22 +112,24 @@ message WithOneof { int32 c = 3; } +// A map, which the table cannot handle, so a message that has one stays +// unrolled when the table is requested. A message that holds one is a table +// message that reaches it through its `Message` impl. message WithMap { map m = 1; map inners = 2; } -message HoldsOneof { +message HoldsMap { WithOneof o = 1; int32 x = 2; WithMap m = 3; } -// A table message that holds table messages (`Inner`, `Nested`) and -// `HoldsOneof`, whose own children are unrolled. +// A table message that holds table messages (`Inner`, `Nested`) and `HoldsMap`. message Mixed { Inner inner = 1; - HoldsOneof holds = 2; + HoldsMap holds = 2; Nested nested = 3; repeated Nested many = 4; } diff --git a/buffa-test/protos/table_codec2.proto b/buffa-test/protos/table_codec2.proto index 63f08e98..02a3ff89 100644 --- a/buffa-test/protos/table_codec2.proto +++ b/buffa-test/protos/table_codec2.proto @@ -30,6 +30,18 @@ message Req { optional bool flag = 13 [default = true]; } +// A oneof with a closed enum, which keeps the member it holds when it reads a +// number the enum does not have. +message ClosedOneof { + oneof pick { + Color shade = 1; + int32 n = 2; + string s = 3; + Inner i = 4; + } + optional int32 tail = 5; +} + // A group field, which the table cannot handle, and the group's own type. message Grouped { optional int32 a = 1; diff --git a/buffa-test/protos/table_codec4.proto b/buffa-test/protos/table_codec4.proto new file mode 100644 index 00000000..54754a18 --- /dev/null +++ b/buffa-test/protos/table_codec4.proto @@ -0,0 +1,114 @@ +syntax = "proto3"; + +// See table_codec.proto: compiled as `tc4u` (unrolled), `tc4t` (table) and +// `tc4x` (table, with options). Oneofs. +package tc4; + +enum Color { + COLOR_UNSPECIFIED = 0; + RED = 1; + GREEN = 2; +} + +message Leaf { + int32 id = 1; + string label = 2; +} + +// A member of every type. +message Kinds { + oneof v { + int32 i32 = 1; + int64 i64 = 2; + uint32 u32 = 3; + uint64 u64 = 4; + sint32 s32 = 5; + sint64 s64 = 6; + fixed32 f32 = 7; + fixed64 f64 = 8; + sfixed32 sf32 = 9; + sfixed64 sf64 = 10; + float fl = 11; + double db = 12; + bool b = 13; + string s = 14; + bytes by = 15; + Color e = 16; + Leaf m = 17; + } +} + +// Two oneofs whose members interleave with each other and with ordinary +// fields, a proto3 `optional` (which is a oneof to the descriptor and not to +// the table), and a repeated field. +message Interleaved { + int32 a = 1; + oneof first { + int32 f1 = 2; + string f2 = 5; + } + int32 b = 3; + oneof second { + Leaf s1 = 4; + bytes s2 = 7; + } + string tail = 6; + repeated int32 r = 8; + optional int32 opt = 9; +} + +// Recursion through a oneof, directly and through another message. +message Tree { + oneof node { + int32 leaf = 1; + Tree left = 2; + Pair pair = 3; + string name = 4; + } +} + +message Pair { + Tree a = 1; + Tree b = 2; +} + +// Field numbers that miss the dense lookup array, and one at the limit. +message Sparse { + oneof o { + int32 lo = 1; + int32 far = 100000; + string end = 536870911; + } + int32 mid = 70; +} + +// A oneof in a nested message and in the message that holds it, and members +// named like Rust keywords. +message Outer { + message Sub { + oneof pick { + int32 type = 1; + string match = 2; + int32 self = 3; + } + } + Sub sub = 1; + oneof o { + Sub x = 2; + Outer nested = 3; + Leaf leaf = 4; + } + repeated Sub subs = 5; +} + +// Members that share a message type, and members that share an enum type, +// whose descriptors the table shares. +message Twins { + oneof pick { + Leaf a = 1; + Leaf b = 2; + Color c = 3; + Color d = 4; + int32 n = 5; + } +} diff --git a/buffa-test/protos/table_codec5.proto b/buffa-test/protos/table_codec5.proto new file mode 100644 index 00000000..2cdba5ed --- /dev/null +++ b/buffa-test/protos/table_codec5.proto @@ -0,0 +1,42 @@ +syntax = "proto3"; + +// See table_codec.proto: compiled as `tc5u` (unrolled) and `tc5t` (table), +// both with a custom pointer type for message fields and members +// (`crate::box_type::CustomBox`) and with `idiomatic_field_names`. Oneofs. +package tc5; + +message Leaf { + int32 id = 1; + string label = 2; +} + +// A message member reached through the custom pointer, and one that recurses +// through it. +message Held { + oneof pick { + int32 n = 1; + Leaf leaf = 2; + Held again = 3; + string s = 4; + } + Leaf single = 5; +} + +// Oneofs named like Rust keywords, members declared out of number order, and +// camelCase names that `idiomatic_field_names` renames. +message Names { + oneof type { + int32 third = 3; + string first = 1; + Leaf second = 2; + } + oneof match { + int32 matchNum = 5; + string matchText = 4; + } + oneof myChoice { + int32 fooBar = 8; + string bazQux = 7; + Leaf theLeaf = 6; + } +} diff --git a/buffa-test/protos/table_codec6.proto b/buffa-test/protos/table_codec6.proto new file mode 100644 index 00000000..416095c4 --- /dev/null +++ b/buffa-test/protos/table_codec6.proto @@ -0,0 +1,26 @@ +syntax = "proto2"; + +// See table_codec.proto: compiled as `tc6u` (unrolled) and `tc6t` (table), +// both with `preserve_unknown_fields(false)`, so a number that a closed enum +// does not have is dropped, not kept. A oneof with a closed enum member. +package tc6; + +enum Color { + RED = 1; + GREEN = 2; + BLUE = 3; +} + +message Inner { + optional int32 id = 1; +} + +message Closed { + oneof pick { + Color shade = 1; + int32 n = 2; + string s = 3; + Inner i = 4; + } + optional int32 tail = 5; +} diff --git a/buffa-test/src/lib.rs b/buffa-test/src/lib.rs index cb163c61..a75c3055 100644 --- a/buffa-test/src/lib.rs +++ b/buffa-test/src/lib.rs @@ -1073,10 +1073,13 @@ pub mod string_copy; pub mod string_copy_counted; // The table codec is tested by compiling a schema twice under renamed packages -// and comparing the results (see `tests::table_codec`). `tcu`, `tc2u`, `tc3u` -// and `wideu` use the default unrolled codec; `tct`, `tc2t`, `tc3t` and `widet` -// are the same schemas with `codec_strategy = Table`, and `tcx` is `tct` again -// with options that change the names and fields a table refers to. They exist +// and comparing the results (see `tests::table_codec`). `tcu`, `tc2u`, `tc3u`, +// `tc4u`, `tc5u`, `tc6u` and `wideu` use the default unrolled codec; `tct`, +// `tc2t`, `tc3t`, `tc4t`, `tc5t`, `tc6t` and `widet` are the same schemas with +// `codec_strategy = Table` (`tc5` and `tc6` are built with options, the same +// for both codecs), and +// `tcx` and `tc4x` are `tct` and `tc4t` again with options that change the +// names and fields a table refers to. They exist // only on Rust 1.77 or later (see build.rs). The table modules forbid unsafe // code, which checks that the `unsafe` a table needs stays inside `buffa`'s // macros. @@ -1115,6 +1118,45 @@ pub mod tc3t { } #[allow(clippy::derivable_impls, clippy::match_single_binding)] #[cfg(has_table_codec)] +pub mod tc4u { + buffa::include_proto!("tc4u"); +} +#[forbid(unsafe_code)] +#[allow(clippy::derivable_impls, clippy::match_single_binding)] +#[cfg(has_table_codec)] +pub mod tc4t { + buffa::include_proto!("tc4t"); +} +#[forbid(unsafe_code)] +#[allow(clippy::derivable_impls, clippy::match_single_binding)] +#[cfg(has_table_codec)] +pub mod tc4x { + buffa::include_proto!("tc4x"); +} +#[allow(clippy::derivable_impls, clippy::match_single_binding)] +#[cfg(has_table_codec)] +pub mod tc5u { + buffa::include_proto!("tc5u"); +} +#[forbid(unsafe_code)] +#[allow(clippy::derivable_impls, clippy::match_single_binding)] +#[cfg(has_table_codec)] +pub mod tc5t { + buffa::include_proto!("tc5t"); +} +#[allow(clippy::derivable_impls, clippy::match_single_binding)] +#[cfg(has_table_codec)] +pub mod tc6u { + buffa::include_proto!("tc6u"); +} +#[forbid(unsafe_code)] +#[allow(clippy::derivable_impls, clippy::match_single_binding)] +#[cfg(has_table_codec)] +pub mod tc6t { + buffa::include_proto!("tc6t"); +} +#[allow(clippy::derivable_impls, clippy::match_single_binding)] +#[cfg(has_table_codec)] pub mod wideu { buffa::include_proto!("wideu"); } diff --git a/buffa-test/src/tests/table_codec.rs b/buffa-test/src/tests/table_codec.rs index 2b39cfd1..c38cdcca 100644 --- a/buffa-test/src/tests/table_codec.rs +++ b/buffa-test/src/tests/table_codec.rs @@ -385,9 +385,10 @@ fn invalid_utf8_is_rejected() { } #[test] -fn messages_the_table_cannot_handle_still_work() { - // A oneof and a map are unrolled, and the messages that hold them are - // tables that reach them through their `Message` impl. +fn a_message_with_a_oneof_and_a_holder_of_a_message_with_a_map_round_trip() { + // A message with a oneof is a table. A message with a map is unrolled, and + // the messages that hold it are tables that reach it through their + // `Message` impl. let with_oneof = crate::tct::WithOneof { choice: Some(crate::tct::with_oneof::Choice::B("x".into())), c: 4, @@ -399,7 +400,7 @@ fn messages_the_table_cannot_handle_still_work() { let mixed = crate::tct::Mixed { inner: buffa::MessageField::some(t::inner(1, "a", &[1])), - holds: buffa::MessageField::some(crate::tct::HoldsOneof { + holds: buffa::MessageField::some(crate::tct::HoldsMap { o: buffa::MessageField::some(with_oneof), x: 2, ..Default::default() @@ -411,6 +412,17 @@ fn messages_the_table_cannot_handle_still_work() { assert_eq!(decoded, mixed); } +#[test] +fn messages_with_a_map_stay_unrolled_and_work() { + let with_map = crate::tct::WithMap { + m: [("k".to_string(), 3)].into_iter().collect(), + ..Default::default() + }; + let decoded = + ::decode_from_slice(&with_map.encode_to_vec()).unwrap(); + assert_eq!(decoded, with_map); +} + // --------------------------------------------------------------------------- // proto2: required fields, defaults, closed enums // --------------------------------------------------------------------------- @@ -615,7 +627,7 @@ macro_rules! shape_samples { }; Mixed { inner: MessageField::some(inner()), - holds: MessageField::some(crate::$m::HoldsOneof { + holds: MessageField::some(crate::$m::HoldsMap { o: MessageField::some(WithOneof { choice: Some(crate::$m::with_oneof::Choice::I(Box::new(inner()))), c: 4, @@ -762,9 +774,9 @@ fn messages_named_like_what_generated_code_uses_agree() { #[test] fn unrolled_messages_inside_a_table_tree_agree() { - // `Mixed.inner` is a table in `tct`, and so are `Mixed` and `HoldsOneof`, - // which hold `WithOneof` and `WithMap`, which are not, and `WithOneof` - // holds a table message in a oneof variant. + // `Mixed.inner` is a table in `tct`, and so are `Mixed` and `HoldsMap`, + // which hold `WithMap`, which is not, and `WithOneof` holds a table + // message in a oneof variant. let wire = assert_same_codec(&shapes_u::mixed(), &shapes_t::mixed()); assert_same_chained::(&wire); let wire = assert_same_codec( @@ -924,18 +936,36 @@ fn the_messages_the_table_can_handle_use_it() { crate::tct::__BUFFA_TABLE_Entry, crate::tct::__BUFFA_TABLE_Aux, crate::tct::__BUFFA_TABLE_Keywords, - crate::tct::__BUFFA_TABLE_HoldsOneof, + crate::tct::__BUFFA_TABLE_HoldsMap, crate::tct::__BUFFA_TABLE_Mixed, + crate::tct::__BUFFA_TABLE_WithOneof, crate::tc2t::__BUFFA_TABLE_Req, crate::tc2t::__BUFFA_TABLE_AllRequired, crate::tc2t::__BUFFA_TABLE_AllRepeated, + crate::tc2t::__BUFFA_TABLE_ClosedOneof, crate::tc3t::__BUFFA_TABLE_E, crate::tc3t::__BUFFA_TABLE_Child, crate::widet::__BUFFA_TABLE_Wide, crate::widet::__BUFFA_TABLE_W254, crate::widet::__BUFFA_TABLE_W255, crate::widet::__BUFFA_TABLE_W256, + crate::widet::__BUFFA_TABLE_WideOneof, crate::tcx::__BUFFA_TABLE_RpcNested, + crate::tc4t::__BUFFA_TABLE_Kinds, + crate::tc4t::__BUFFA_TABLE_Interleaved, + crate::tc4t::__BUFFA_TABLE_Tree, + crate::tc4t::__BUFFA_TABLE_Pair, + crate::tc4t::__BUFFA_TABLE_Twins, + crate::tc4t::__BUFFA_TABLE_Sparse, + crate::tc4t::__BUFFA_TABLE_Outer, + crate::tc4t::outer::__BUFFA_TABLE_Sub, + crate::tc4x::__BUFFA_TABLE_RpcKinds, + crate::tc4x::__BUFFA_TABLE_RpcTwins, + crate::tc5t::__BUFFA_TABLE_Leaf, + crate::tc5t::__BUFFA_TABLE_Held, + crate::tc5t::__BUFFA_TABLE_Names, + crate::tc6t::__BUFFA_TABLE_Inner, + crate::tc6t::__BUFFA_TABLE_Closed, crate::xat::__BUFFA_TABLE_Leaf, crate::xat::__BUFFA_TABLE_Wrap, crate::xbt::__BUFFA_TABLE_Holder, @@ -947,6 +977,7 @@ fn the_messages_the_table_can_handle_use_it() { crate::brt::__BUFFA_TABLE_Leaf, crate::brt::__BUFFA_TABLE_Cold, crate::brt::__BUFFA_TABLE_Wkt, + crate::brt::__BUFFA_TABLE_Pick, crate::xe::__BUFFA_TABLE_Leaf, crate::xft::__BUFFA_TABLE_Holder, ); @@ -955,57 +986,68 @@ fn the_messages_the_table_can_handle_use_it() { #[test] fn messages_held_across_packages_agree_in_every_layout() { use buffa::MessageField; - macro_rules! sample { + // The same values in each layout: one holder for each member of its oneof + // (whose members are messages of the other package, one of them unrolled + // in the table layouts), and one with none. + macro_rules! samples { ($xa:ident, $xb:ident) => {{ + use $xb::holder::Pick; let leaf = |x| $xa::Leaf { x, s: "s".into(), ..Default::default() }; - $xb::Holder { + let wrap = || $xa::Wrap { + leaf: MessageField::some(leaf(4)), + leaves: vec![leaf(5)], + ..Default::default() + }; + let sub = || $xb::holder::Sub { + l: MessageField::some(leaf(6)), + ..Default::default() + }; + // Unrolled in the table layouts. + let cold = |c| $xa::Cold { + c, + s: "cold".into(), + ..Default::default() + }; + [ + None, + Some(Pick::N(7)), + Some(Pick::Pl(Box::new(leaf(8)))), + Some(Pick::Pw(Box::new(wrap()))), + Some(Pick::Ps(Box::new(sub()))), + Some(Pick::Pc(Box::new(cold(9)))), + ] + .into_iter() + .map(|pick| $xb::Holder { leaf: MessageField::some(leaf(1)), leaves: vec![leaf(2), leaf(3)], - wrap: MessageField::some($xa::Wrap { - leaf: MessageField::some(leaf(4)), - leaves: vec![leaf(5)], - ..Default::default() - }), - sub: MessageField::some($xb::holder::Sub { - l: MessageField::some(leaf(6)), - ..Default::default() - }), + wrap: MessageField::some(wrap()), + sub: MessageField::some(sub()), // Unrolled in the table layouts. - cold: MessageField::some($xa::Cold { - c: 7, - s: "cold".into(), - ..Default::default() - }), - colds: vec![ - $xa::Cold::default(), - $xa::Cold { - c: 8, - ..Default::default() - }, - ], + cold: MessageField::some(cold(7)), + colds: vec![$xa::Cold::default(), cold(8)], + pick, ..Default::default() - } + }) + .collect::>() }}; } use crate::xti::{xati, xbti}; use crate::{xat, xau, xbt, xbu}; - let unrolled = sample!(xau, xbu); - let table = sample!(xat, xbt); - let idiomatic = sample!(xati, xbti); - let wire = assert_same_codec(&unrolled, &table); - assert_eq!(idiomatic.encode_to_vec(), wire); - assert_eq!( - xbti::Holder::decode_from_slice(&wire) - .unwrap() - .encode_to_vec(), - wire - ); - assert_same_chained::(&wire); - assert_same_on_corrupt_input::(&wire, true); + let unrolled = samples!(xau, xbu); + let table = samples!(xat, xbt); + let idiomatic = samples!(xati, xbti); + for ((u, t), i) in unrolled.iter().zip(&table).zip(&idiomatic) { + let wire = assert_same_codec(u, t); + assert_eq!(i.encode_to_vec(), wire); + let decoded = xbti::Holder::decode_from_slice(&wire).unwrap(); + assert_eq!(&decoded, i); + assert_same_chained::(&wire); + assert_same_on_corrupt_input::(&wire, true); + } } // --------------------------------------------------------------------------- @@ -1016,7 +1058,8 @@ fn messages_held_across_packages_agree_in_every_layout() { macro_rules! bridge_samples { ($name:ident, $m:ident) => { mod $name { - use crate::$m::{Cold, Hot, Leaf, Wkt}; + use crate::$m::pick::Choice; + use crate::$m::{Cold, Hot, Leaf, Pick, Wkt}; use buffa::MessageField; use buffa_types::google::protobuf::{ Any, Duration, Empty, FieldMask, Int32Value, StringValue, Struct, Timestamp, Value, @@ -1062,6 +1105,33 @@ macro_rules! bridge_samples { } } + /// One `Pick` for each member of its oneof, and one with none. + pub fn picks() -> Vec { + let mut st = Struct::new(); + st.insert("k", 1.5); + [ + None, + Some(Choice::N(7)), + Some(Choice::Leaf(Box::new(leaf(1, "l", &[2])))), + Some(Choice::Hot(Box::new(hot(2, true)))), + Some(Choice::Cold(Box::new(cold()))), + Some(Choice::Ts(Box::new(Timestamp::from_unix(1_700_000_000, 5)))), + Some(Choice::Any(Box::new(Any::pack( + &Timestamp::from_unix(1, 2), + "type.googleapis.com/google.protobuf.Timestamp", + )))), + Some(Choice::Val(Box::new(Value::from("v")))), + Some(Choice::S("s".into())), + ] + .into_iter() + .map(|choice| Pick { + choice, + tail: MessageField::some(leaf(9, "tail", &[])), + ..Default::default() + }) + .collect() + } + pub fn wkt() -> Wkt { let mut st = Struct::new(); st.insert("k", 1.5); @@ -1201,6 +1271,62 @@ fn a_table_message_copies_the_payload_of_an_any_it_holds() { assert_eq!(table.anys, unrolled.anys); } +#[test] +fn oneof_members_without_a_table_agree() { + // `Hot` is unrolled in `brt`, and the well-known types have no table, so + // those members are reached through their `Message` impl; `Leaf` and + // `Cold` have tables. + let (unrolled, table) = (bru_s::picks(), brt_s::picks()); + assert_eq!(unrolled.len(), 9); + for (u, t) in unrolled.iter().zip(&table) { + let wire = assert_same_codec(u, t); + assert_same_chained::(&wire); + assert_same_on_corrupt_input::(&wire, true); + } +} + +#[test] +fn oneof_members_without_a_table_merge_and_fail_alike() { + let opts = buffa::DecodeOptions::new(); + let wires: [&[u8]; 8] = [ + // `hot` twice, which merges: {a = 1}, then {leaf = {x = 5}}. + &[0x1a, 0x02, 0x08, 0x01, 0x1a, 0x04, 0x12, 0x02, 0x08, 0x05], + // `ts` twice: {seconds = 1}, then {nanos = 2}. + &[0x2a, 0x02, 0x08, 0x01, 0x2a, 0x02, 0x10, 0x02], + // Another member replaces the one that is set, in both orders. + &[0x12, 0x02, 0x08, 0x03, 0x1a, 0x02, 0x08, 0x01], + &[0x1a, 0x02, 0x08, 0x01, 0x12, 0x02, 0x08, 0x03], + // A member that fails to decode after a complete one. + &[0x1a, 0x02, 0x08, 0x01, 0x1a, 0x05, 0x08], + &[0x2a, 0x02, 0x08, 0x01, 0x2a, 0x03, 0x08], + // Bad UTF-8 in a message member. + &[0x1a, 0x03, 0x12, 0x01, 0xff], + &[0x3a, 0x02, 0xc3, 0x28], + ]; + macro_rules! starts { + ($m:ident, $s:ident) => { + [ + crate::$m::Pick::default(), + $s::picks()[3].clone(), + $s::picks()[5].clone(), + $s::picks()[6].clone(), + ] + }; + } + let (start_u, start_t) = (starts!(bru, bru_s), starts!(brt, brt_s)); + for wire in wires { + assert_same_decode::(wire, false); + for (u, t) in start_u.iter().zip(&start_t) { + assert_same_merge(&opts, u, t, wire); + } + } + let merged = ::decode_from_slice(wires[0]).unwrap(); + let Some(crate::brt::pick::Choice::Hot(hot)) = merged.choice else { + panic!("expected a Hot, got {:?}", merged.choice); + }; + assert_eq!((hot.a, hot.leaf.as_option().unwrap().x), (1, 5)); +} + #[test] fn messages_from_another_crate_agree() { macro_rules! sample { @@ -1211,17 +1337,29 @@ fn messages_from_another_crate_agree() { kids: vec![crate::xe::Leaf::default()], ..Default::default() }; - crate::$m::Holder { + // One holder for each member of its oneof, whose message member + // is from another crate, and one with none. + [ + None, + Some(crate::$m::holder::Pick::N(5)), + Some(crate::$m::holder::Pick::Pl(Box::new(leaf(6, "pl")))), + ] + .into_iter() + .map(|pick| crate::$m::Holder { leaf: buffa::MessageField::some(leaf(1, "a")), leaves: vec![leaf(2, "b"), leaf(3, "")], tail: 4, + pick, ..Default::default() - } + }) + .collect::>() }}; } - let wire = assert_same_codec(&sample!(xfu), &sample!(xft)); - assert_same_chained::(&wire); - assert_same_on_corrupt_input::(&wire, true); + for (u, t) in sample!(xfu).iter().zip(&sample!(xft)) { + let wire = assert_same_codec(u, t); + assert_same_chained::(&wire); + assert_same_on_corrupt_input::(&wire, true); + } } /// The wire form of `Cold` with `hot` set to a `Hot` with `back` set to a @@ -1358,7 +1496,9 @@ fn a_child_without_a_table_is_encoded_into_every_kind_of_sink() { #[test] fn a_holder_of_a_bytes_typed_message_is_unrolled_and_decodes_without_copying() { - use crate::tbz::{Blob, HoldsBlob, HoldsBlobs}; + use crate::tbz::{ + holds_blob_in_oneof, Blob, HoldsBlob, HoldsBlobInOneof, HoldsBlobs, HoldsPick, + }; use buffa::bytes::Bytes; use buffa::MessageField; // The plan ran on this schema, so `Plain` has a table, and `HoldsBlob` @@ -1368,6 +1508,9 @@ fn a_holder_of_a_bytes_typed_message_is_unrolled_and_decodes_without_copying() { assert!(generated.contains("static __BUFFA_TABLE_HoldsPlain")); assert!(!generated.contains("__BUFFA_TABLE_Blob")); assert!(!generated.contains("__BUFFA_TABLE_HoldsBlob")); + // Also the one that holds it in a oneof member, and its own holder. + assert!(!generated.contains("__BUFFA_TABLE_HoldsBlobInOneof")); + assert!(!generated.contains("__BUFFA_TABLE_HoldsPick")); let blob = |fill: u8| Blob { data: Bytes::from(vec![fill; 64]), @@ -1396,4 +1539,1078 @@ fn a_holder_of_a_bytes_typed_message_is_unrolled_and_decodes_without_copying() { .blobs .iter() .all(|b| aliases(&b.data) && aliases(&b.chunks[0]))); + + // The oneof member decodes without copying too. + let msg = HoldsPick { + inner: MessageField::some(HoldsBlobInOneof { + pick: Some(holds_blob_in_oneof::Pick::Blob(Box::new(blob(7)))), + ..Default::default() + }), + ..Default::default() + }; + let src = Bytes::from(msg.encode_to_vec()); + let range = src.as_ptr() as usize..src.as_ptr() as usize + src.len(); + let aliases = |b: &Bytes| range.contains(&(b.as_ptr() as usize)); + let decoded = HoldsPick::decode(&mut src.clone()).unwrap(); + assert_eq!(decoded, msg); + let Some(holds_blob_in_oneof::Pick::Blob(held)) = &decoded.inner.as_option().unwrap().pick + else { + panic!("expected a Blob"); + }; + assert!(aliases(&held.data) && aliases(&held.chunks[0])); +} + +// --------------------------------------------------------------------------- +// Oneofs +// --------------------------------------------------------------------------- + +macro_rules! oneof_samples { + ($name:ident, $m:ident) => { + mod $name { + use crate::$m::{ + interleaved as il, kinds::V, outer as ou, sparse as sp, tree as tr, Color, + Interleaved, Kinds, Leaf, Outer, Pair, Sparse, Tree, + }; + use buffa::{EnumValue, MessageField}; + + pub fn leaf(id: i32, label: &str) -> Leaf { + Leaf { + id, + label: label.into(), + ..Default::default() + } + } + + /// A message for every member, with a value that is not the + /// default and one that is, and a message with none. + pub fn kinds() -> Vec { + [ + V::I32(-5), + V::I32(0), + V::I64(i64::MIN), + V::I64(0), + V::U32(u32::MAX), + V::U32(0), + V::U64(u64::MAX), + V::U64(0), + V::S32(-3), + V::S32(0), + V::S64(i64::MIN), + V::S64(0), + V::F32(7), + V::F32(0), + V::F64(u64::MAX), + V::F64(0), + V::Sf32(-1), + V::Sf32(0), + V::Sf64(i64::MIN), + V::Sf64(0), + V::Fl(1.5), + V::Fl(0.0), + V::Db(-2.25), + V::Db(0.0), + V::B(true), + V::B(false), + V::S("héllo".into()), + V::S(String::new()), + V::By(vec![0, 1, 255]), + V::By(vec![]), + V::E(EnumValue::from(Color::GREEN)), + V::E(EnumValue::from(Color::COLOR_UNSPECIFIED)), + V::E(EnumValue::from(9)), + V::M(Box::new(leaf(4, "m"))), + V::M(Box::default()), + ] + .into_iter() + .map(|v| Kinds { + v: Some(v), + ..Default::default() + }) + .chain([Kinds::default()]) + .collect() + } + + /// Every combination of what the two oneofs hold, around ordinary + /// fields of numbers below, between and above theirs. + pub fn interleaved() -> Vec { + let mut out = Vec::new(); + for first in [ + None, + Some(il::First::F1(0)), + Some(il::First::F1(7)), + Some(il::First::F2("f".into())), + ] { + for second in [ + None, + Some(il::Second::S1(Box::new(leaf(1, "a")))), + Some(il::Second::S2(vec![1, 2])), + ] { + out.push(Interleaved { + a: 1, + first: first.clone(), + b: 2, + second, + tail: "t".into(), + r: vec![1, 2], + opt: Some(0), + ..Default::default() + }); + } + } + out + } + + pub fn tree() -> Tree { + let node = |node| Tree { + node: Some(node), + ..Default::default() + }; + node(tr::Node::Left(Box::new(node(tr::Node::Pair(Box::new( + Pair { + a: MessageField::some(node(tr::Node::Leaf(3))), + b: MessageField::some(node(tr::Node::Name("n".into()))), + ..Default::default() + }, + )))))) + } + + pub fn sparse() -> Vec { + [ + Some(sp::O::Lo(0)), + Some(sp::O::Far(-1)), + Some(sp::O::End("e".into())), + None, + ] + .into_iter() + .map(|o| Sparse { + o, + mid: 5, + ..Default::default() + }) + .collect() + } + + pub fn outer() -> Vec { + let sub = |pick| ou::Sub { + pick, + ..Default::default() + }; + let picks = || { + [ + Some(ou::sub::Pick::Type(1)), + Some(ou::sub::Pick::Match("m".into())), + Some(ou::sub::Pick::Self_(0)), + None, + ] + }; + let mut out = Vec::new(); + for pick in picks() { + for o in [ + None, + Some(ou::O::X(Box::new(sub(pick.clone())))), + Some(ou::O::Nested(Box::default())), + Some(ou::O::Leaf(Box::new(leaf(2, "l")))), + ] { + out.push(Outer { + sub: MessageField::some(sub(pick.clone())), + o, + subs: vec![sub(None), sub(pick.clone())], + ..Default::default() + }); + } + } + out + } + } + }; +} + +oneof_samples!(oneofs_u, tc4u); +oneof_samples!(oneofs_t, tc4t); + +#[test] +fn a_oneof_member_of_every_type_agrees() { + let (unrolled, table) = (oneofs_u::kinds(), oneofs_t::kinds()); + assert_eq!(unrolled.len(), table.len()); + for (u, t) in unrolled.iter().zip(&table) { + let wire = assert_same_codec(u, t); + assert_same_chained::(&wire); + assert_same_on_corrupt_input::(&wire, true); + } +} + +#[test] +fn a_member_that_is_set_is_written_even_with_its_default_value() { + for (i, kinds) in oneofs_t::kinds().iter().enumerate() { + let wire = kinds.encode_to_vec(); + assert_eq!(wire.is_empty(), kinds.v.is_none(), "sample {i}"); + } + let zero = crate::tc4t::Kinds { + v: Some(crate::tc4t::kinds::V::I32(0)), + ..Default::default() + }; + assert_eq!(zero.encode_to_vec(), [0x08, 0x00]); +} + +#[test] +fn a_oneof_is_written_where_its_lowest_member_is_whichever_member_is_set() { + let (unrolled, table) = (oneofs_u::interleaved(), oneofs_t::interleaved()); + for (u, t) in unrolled.iter().zip(&table) { + let wire = assert_same_codec(u, t); + assert_same_chained::(&wire); + assert_same_on_corrupt_input::( + &wire, true, + ); + } + // `first` (2, 5) is written at 2, before `b` (3), and `second` (4, 7) + // at 4, before `tail` (6), whichever of their members is set. + let msg = &table[table.len() - 1]; + assert_eq!( + msg.encode_to_vec(), + [ + 0x08, 0x01, // a = 1 + 0x2a, 0x01, b'f', // first: f2 = "f", at 2 in the order + 0x18, 0x02, // b = 2 + 0x3a, 0x02, 0x01, 0x02, // second: s2, at 4 in the order + 0x32, 0x01, b't', // tail + 0x42, 0x02, 0x01, 0x02, // r, packed + 0x48, 0x00, // opt = 0 + ] + ); +} + +#[test] +fn recursion_through_a_oneof_agrees() { + let wire = assert_same_codec(&oneofs_u::tree(), &oneofs_t::tree()); + assert_same_chained::(&wire); + assert_same_on_corrupt_input::(&wire, true); +} + +#[test] +fn oneof_members_outside_the_dense_lookup_agree() { + for (u, t) in oneofs_u::sparse().iter().zip(&oneofs_t::sparse()) { + let wire = assert_same_codec(u, t); + assert_same_on_corrupt_input::(&wire, false); + } +} + +#[test] +fn oneofs_in_nested_messages_and_keyword_named_members_agree() { + let (unrolled, table) = (oneofs_u::outer(), oneofs_t::outer()); + assert!(!table.is_empty()); + for (u, t) in unrolled.iter().zip(&table) { + let wire = assert_same_codec(u, t); + assert_same_on_corrupt_input::(&wire, true); + } +} + +/// The `Kinds` that the table codec decodes `wire` to, which must decode. +fn kinds_from(wire: &[u8]) -> crate::tc4t::Kinds { + ::decode_from_slice(wire).unwrap() +} + +/// The places in [`oneof_wires`] of the inputs that a test reads back. +const LAST_MEMBER_WINS: usize = 0; +const MESSAGE_MERGES_INTO_ITSELF: usize = 2; +const MESSAGE_REPLACED_BY_NEW_ONE: usize = 3; +const OPEN_ENUM_KEEPS_ANY_NUMBER: usize = 6; + +/// Inputs that exercise how a decode picks and replaces the member. +fn oneof_wires() -> Vec> { + let cat = |parts: &[Vec]| parts.concat(); + let leaf = |id: u64, label: &[u8]| { + let mut payload = varint_field(1, id); + payload.extend(length_delimited_field(2, label)); + payload + }; + vec![ + // The last member wins, whatever order the numbers come in. + cat(&[ + varint_field(1, 5), + length_delimited_field(14, b"hi"), + varint_field(3, 9), + ]), + cat(&[ + length_delimited_field(14, b"hi"), + varint_field(1, 5), + varint_field(1, 6), + ]), + // A message member merges into itself, and a member of another number + // replaces it and starts a new one from nothing. + cat(&[ + length_delimited_field(17, &varint_field(1, 4)), + length_delimited_field(17, &length_delimited_field(2, b"x")), + ]), + cat(&[ + length_delimited_field(17, &leaf(4, b"x")), + varint_field(1, 1), + length_delimited_field(17, &varint_field(1, 2)), + ]), + cat(&[ + length_delimited_field(17, &leaf(4, b"x")), + length_delimited_field(17, &[]), + ]), + // Unknown fields between members. + cat(&[varint_field(99, 1), varint_field(1, 5), varint_field(98, 2)]), + // An open enum keeps a number it does not name, and a default one is set. + varint_field(16, 9), + varint_field(16, 0), + // A string that is not UTF-8, and one that is empty. + length_delimited_field(14, &[0xff]), + cat(&[varint_field(1, 5), length_delimited_field(14, &[0xff])]), + length_delimited_field(14, &[]), + // A wrong wire type for each kind of member. + length_delimited_field(1, b"x"), + varint_field(14, 1), + varint_field(15, 1), + varint_field(17, 1), + cat(&[varint_field(1, 5), length_delimited_field(3, &[1])]), + // A varint that is too long, and a sub-message that runs past its + // buffer. + vec![ + 0x08, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01, + ], + vec![0x8a, 0x01, 0x05, 0x08], + ] +} + +#[test] +fn decoding_picks_and_replaces_the_member_the_way_the_unrolled_codec_does() { + for wire in oneof_wires() { + assert_same_decode::(&wire, true); + assert_same_chained::(&wire); + } + // And what those come to, for the ones that decode. + use crate::tc4t::kinds::V; + assert_eq!( + kinds_from(&oneof_wires()[LAST_MEMBER_WINS]).v, + Some(V::U32(9)), + "the last member wins" + ); + let merged = kinds_from(&oneof_wires()[MESSAGE_MERGES_INTO_ITSELF]).v; + let Some(V::M(m)) = merged else { + panic!("{merged:?}") + }; + assert_eq!((m.id, m.label.as_str()), (4, "x")); + let replaced = kinds_from(&oneof_wires()[MESSAGE_REPLACED_BY_NEW_ONE]).v; + let Some(V::M(m)) = replaced else { + panic!("{replaced:?}") + }; + assert_eq!( + (m.id, m.label.as_str()), + (2, ""), + "a new message starts empty" + ); + assert_eq!( + kinds_from(&oneof_wires()[OPEN_ENUM_KEEPS_ANY_NUMBER]).v, + Some(V::E(buffa::EnumValue::from(9))) + ); +} + +/// A tag: field `number` with wire type `wire_type`. +fn tag(number: u32, wire_type: u32) -> Vec { + let mut tag = Vec::new(); + let mut v = u64::from(number << 3 | wire_type); + while v >= 0x80 { + tag.push(v as u8 | 0x80); + v >>= 7; + } + tag.push(v as u8); + tag +} + +/// Merge `wire` into a copy of each start value, one per codec, with `opts`, +/// and require the same result and the same message afterwards, whether the +/// merge succeeded or failed part of the way. +#[track_caller] +fn assert_same_merge(opts: &buffa::DecodeOptions, unrolled: &U, table: &T, wire: &[u8]) +where + U: Message + Debug + Clone, + T: Message + Debug + Clone, +{ + let (mut u, mut t) = (unrolled.clone(), table.clone()); + let (ru, rt) = ( + opts.merge_from_slice(&mut u, wire), + opts.merge_from_slice(&mut t, wire), + ); + let context = format!("merging {wire:02x?} into {unrolled:?}"); + assert_eq!(ru, rt, "results differ {context}"); + // A table with a type name prefix names its messages differently. + assert_eq!( + format!("{u:?}"), + format!("{t:?}").replace("Rpc", ""), + "messages differ afterwards {context} ({ru:?})" + ); + assert_eq!( + u.encode_to_vec(), + t.encode_to_vec(), + "encodings differ afterwards {context}" + ); +} + +/// The wire types of the members of `Kinds`, by field number. +fn kinds_wire_type(number: u32) -> u32 { + match number { + 7 | 9 | 11 => 5, + 8 | 10 | 12 => 1, + 14 | 15 | 17 => 2, + _ => 0, + } +} + +/// Inputs that fail in a member of every kind, in each of the ways it can: a +/// tag with nothing after it, a value that is cut short, a wire type the +/// member does not have, and a string that is not UTF-8. +fn failing_kinds_wires() -> Vec> { + let mut wires = Vec::new(); + for number in 1..=17 { + let wt = kinds_wire_type(number); + wires.push(tag(number, wt)); + wires.push([tag(number, wt), vec![0x80]].concat()); + wires.push([tag(number, wt), vec![0x05, 0x01]].concat()); + for other in [0, 1, 2, 5] { + if other != wt { + wires.push([tag(number, other), vec![0; 10]].concat()); + } + } + } + wires.push([tag(14, 2), vec![0x01, 0xff]].concat()); + // The message member: a length that is not a message's, one that is too + // large, and content that does not decode, after a field that does. + wires.push([tag(17, 2), vec![0x80, 0x80, 0x80, 0x80, 0x08]].concat()); + wires.push([tag(17, 2), vec![0x03, 0x08, 0x05, 0x10]].concat()); + wires.push([tag(17, 2), vec![0x04, 0x08, 0x05, 0x12, 0x03, b'x']].concat()); + wires +} + +#[test] +fn a_member_that_fails_to_decode_leaves_the_message_as_unrolled_code_does() { + let opts = buffa::DecodeOptions::new(); + let (unrolled, table) = (oneofs_u::kinds(), oneofs_t::kinds()); + let valid_first = varint_field(3, 9); + for wire in failing_kinds_wires() { + for prefix in [&[][..], &valid_first[..]] { + let wire = [prefix, &wire[..]].concat(); + for (u, t) in unrolled.iter().zip(&table) { + assert_same_merge(&opts, u, t, &wire); + } + } + } + // The truncated message member into a string, the case a message member + // that is installed before it decodes gets wrong. + let text = crate::tc4u::Kinds { + v: Some(crate::tc4u::kinds::V::S("abc".into())), + ..Default::default() + }; + let mut merged = text.clone(); + assert!(merged.merge_from_slice(&[0x8a, 0x01]).is_err()); + assert_eq!(merged, text, "unrolled code keeps the string"); + let mut merged = crate::tc4t::Kinds::decode_from_slice(&text.encode_to_vec()).unwrap(); + assert!(merged.merge_from_slice(&[0x8a, 0x01]).is_err()); + assert_eq!(merged.v, Some(crate::tc4t::kinds::V::S("abc".into()))); +} + +#[test] +fn a_message_member_that_fails_in_a_message_with_two_oneofs_agrees() { + let opts = buffa::DecodeOptions::new(); + let wires: Vec> = vec![ + tag(4, 2), + [tag(4, 2), vec![0x05, 0x08]].concat(), + [tag(4, 2), vec![0x03, 0x08, 0x05, 0x10]].concat(), + [tag(7, 2), vec![0x05, 0x01]].concat(), + [tag(2, 0), vec![0x80]].concat(), + [varint_field(2, 1), tag(5, 2), vec![0x05, b'a']].concat(), + [tag(9, 0), vec![0x80]].concat(), + ]; + for wire in wires { + for (u, t) in oneofs_u::interleaved().iter().zip(&oneofs_t::interleaved()) { + assert_same_merge(&opts, u, t, &wire); + } + } +} + +#[test] +fn the_message_limits_leave_the_message_as_unrolled_code_does() { + // Too deep, from a message that has a member set and from one that does + // not, at the limit and one either side of it. + let deep = |depth: usize| { + let mut wire = varint_field(1, 1); + for _ in 0..depth { + wire = length_delimited_field(2, &wire); + } + wire + }; + let start_u = |left: bool| crate::tc4u::Tree { + node: left.then(|| crate::tc4u::tree::Node::Name("abc".into())), + ..Default::default() + }; + let start_t = |left: bool| crate::tc4t::Tree { + node: left.then(|| crate::tc4t::tree::Node::Name("abc".into())), + ..Default::default() + }; + for limit in [1, 3, 10] { + let opts = buffa::DecodeOptions::new().with_recursion_limit(limit); + for depth in (limit as usize - 1)..=(limit as usize + 2) { + for left in [false, true] { + assert_same_merge(&opts, &start_u(left), &start_t(left), &deep(depth)); + } + } + } + let opts = buffa::DecodeOptions::new().with_recursion_limit(3); + let mut tree = start_t(true); + assert_eq!( + opts.merge_from_slice(&mut tree, &deep(6)), + Err(DecodeError::RecursionLimitExceeded) + ); + // And a message member that is larger than a message can be, or larger + // than the size limit. + let too_large = [tag(2, 2), vec![0x80, 0x80, 0x80, 0x80, 0x08]].concat(); + let small = buffa::DecodeOptions::new().with_max_message_size(4); + for opts in [buffa::DecodeOptions::new(), small] { + for wire in [&too_large, &deep(3)] { + for left in [false, true] { + assert_same_merge(&opts, &start_u(left), &start_t(left), wire); + } + } + } + let mut tree = start_t(true); + assert_eq!( + tree.merge_from_slice(&too_large), + Err(DecodeError::MessageTooLarge) + ); + assert_eq!( + tree.node, + Some(crate::tc4t::tree::Node::Name("abc".into())), + "the oneof keeps the member that was set" + ); +} + +#[test] +fn a_thousand_levels_of_recursion_through_a_oneof_is_the_same_error() { + let mut wire = varint_field(1, 1); + for _ in 0..1000 { + wire = length_delimited_field(2, &wire); + } + assert_same_decode::(&wire, true); + assert_eq!( + ::decode_from_slice(&wire), + Err(DecodeError::RecursionLimitExceeded) + ); + // The same through the other message and the member that holds it. + let pair = |inner: &[u8]| length_delimited_field(3, &length_delimited_field(1, inner)); + let mut wire = varint_field(1, 1); + for _ in 0..1000 { + wire = pair(&wire); + } + assert_same_decode::(&wire, true); + assert_eq!( + ::decode_from_slice(&wire), + Err(DecodeError::RecursionLimitExceeded) + ); +} + +#[test] +fn merging_into_a_message_that_holds_a_member_agrees() { + let opts = buffa::DecodeOptions::new(); + let start = || { + ( + crate::tc4u::Kinds { + v: Some(crate::tc4u::kinds::V::M(Box::new(oneofs_u::leaf(1, "a")))), + ..Default::default() + }, + crate::tc4t::Kinds { + v: Some(crate::tc4t::kinds::V::M(Box::new(oneofs_t::leaf(1, "a")))), + ..Default::default() + }, + ) + }; + for wire in oneof_wires() { + let (unrolled, table) = start(); + assert_same_merge(&opts, &unrolled, &table, &wire); + } +} + +#[test] +fn a_closed_enum_member_the_enum_does_not_know_goes_to_the_unknown_fields() { + use crate::tc2t::closed_oneof::Pick; + // n = 5, then shade = 7 (Color has no 7), then a known shade, then n again. + let unknown = [varint_field(2, 5), varint_field(1, 7)].concat(); + let decoded = ::decode_from_slice(&unknown).unwrap(); + assert_eq!( + decoded.pick, + Some(Pick::N(5)), + "the unknown value is not a member" + ); + assert_eq!( + decoded.encode_to_vec(), + [varint_field(2, 5), varint_field(1, 7)].concat() + ); + let known = [unknown.clone(), varint_field(1, 2)].concat(); + assert_eq!( + ::decode_from_slice(&known) + .unwrap() + .pick, + Some(Pick::Shade(crate::tc2t::Color::GREEN)) + ); + for wire in [ + unknown, + known, + varint_field(1, 7), + [varint_field(1, 2), varint_field(3, 1)].concat(), + [ + length_delimited_field(4, &varint_field(1, 3)), + length_delimited_field(4, &varint_field(1, 4)), + varint_field(5, 6), + ] + .concat(), + [varint_field(2, 5), length_delimited_field(3, &[0xff])].concat(), + ] { + assert_same_decode::(&wire, true); + assert_same_chained::(&wire); + } + let sample = |shade| crate::tc2t::ClosedOneof { + pick: Some(Pick::Shade(shade)), + tail: Some(1), + ..Default::default() + }; + let wire = sample(crate::tc2t::Color::BLUE).encode_to_vec(); + assert_same_on_corrupt_input::(&wire, true); +} + +#[test] +fn a_table_with_a_prefix_and_inline_oneof_members_agrees() { + // `tc4x` has prefixed type names and no unknown fields, and stores the + // message members of `Kinds`, `Interleaved`, `Sparse` and `Outer` inline. + fn same(wire: &[u8]) { + assert_eq!(outcome::(wire), outcome::(wire), "{wire:02x?}"); + } + for kinds in oneofs_u::kinds() { + same::(&kinds.encode_to_vec()); + } + for msg in oneofs_u::interleaved() { + same::(&msg.encode_to_vec()); + } + for msg in oneofs_u::sparse() { + same::(&msg.encode_to_vec()); + } + for msg in oneofs_u::outer() { + same::(&msg.encode_to_vec()); + } + same::(&oneofs_u::tree().encode_to_vec()); + // A message member decoded twice merges into the inline value. + let wire = oneof_wires()[MESSAGE_MERGES_INTO_ITSELF].clone(); + same::(&wire); + let decoded = ::decode_from_slice(&wire).unwrap(); + assert_eq!(decoded.encode_to_vec(), kinds_from(&wire).encode_to_vec()); +} + +#[test] +fn a_member_that_fails_to_decode_in_an_inline_layout_leaves_the_message_as_unrolled_code_does() { + // `tc4x` stores the message member of `Kinds` inline, where a failed + // merge into a new default member must leave the old member in place too. + let opts = buffa::DecodeOptions::new(); + let valid_first = varint_field(3, 9); + for u in oneofs_u::kinds() { + let x = ::decode_from_slice(&u.encode_to_vec()).unwrap(); + for wire in failing_kinds_wires() { + for prefix in [&[][..], &valid_first[..]] { + assert_same_merge(&opts, &u, &x, &[prefix, &wire[..]].concat()); + } + } + } +} + +/// A `Twins` for each member and one with none, in the module `$m`. +macro_rules! twins_samples { + ($m:ident) => {{ + use crate::$m::{twins::Pick, Color, Leaf, Twins}; + use buffa::EnumValue; + let leaf = |id, label: &str| { + Box::new(Leaf { + id, + label: label.into(), + ..Default::default() + }) + }; + [ + None, + Some(Pick::A(leaf(1, "a"))), + Some(Pick::B(leaf(2, "b"))), + Some(Pick::B(Box::default())), + Some(Pick::C(EnumValue::from(Color::RED))), + Some(Pick::D(EnumValue::from(Color::GREEN))), + Some(Pick::D(EnumValue::from(9))), + Some(Pick::N(4)), + ] + .into_iter() + .map(|pick| Twins { + pick, + ..Default::default() + }) + .collect::>() + }}; +} + +#[test] +fn members_that_share_a_message_or_enum_type_agree() { + let opts = buffa::DecodeOptions::new(); + let (unrolled, table) = (twins_samples!(tc4u), twins_samples!(tc4t)); + let leaf = |id: u64| varint_field(1, id); + let wires = [ + // `a`, then `b`, replaces it, and `b` again merges into itself. + [ + length_delimited_field(1, &leaf(1)), + length_delimited_field(2, &leaf(2)), + length_delimited_field(2, &length_delimited_field(2, b"x")), + ] + .concat(), + [varint_field(3, 1), varint_field(4, 2), varint_field(3, 9)].concat(), + // Cut short members of each of the shared types. + [ + length_delimited_field(1, &leaf(1)), + tag(2, 2), + vec![0x05, 0x08], + ] + .concat(), + [varint_field(3, 1), tag(4, 0), vec![0x80]].concat(), + ]; + for (u, t) in unrolled.iter().zip(&table) { + let wire = assert_same_codec(u, t); + assert_same_chained::(&wire); + assert_same_on_corrupt_input::(&wire, true); + // `tc4x` stores the members inline, in a table with prefixed names. + let x = ::decode_from_slice(&wire).unwrap(); + assert_eq!(x.encode_to_vec(), wire); + for wire in &wires { + assert_same_merge(&opts, u, t, wire); + assert_same_merge(&opts, u, &x, wire); + } + } + for wire in &wires { + assert_same_decode::(wire, false); + } +} + +/// A `WideOneof` for each member of its oneof and one with none, in the module +/// `$m`. +macro_rules! wide_oneof_samples { + ($m:ident) => {{ + use crate::$m::{wide_oneof::Pick, Leaf, WideColor, WideOneof}; + use buffa::EnumValue; + [ + None, + Some(Pick::A(7)), + Some(Pick::B("wide".into())), + Some(Pick::C(Box::new(Leaf { + x: 3, + ..Default::default() + }))), + Some(Pick::D(EnumValue::from(WideColor::WIDE_RED))), + ] + .into_iter() + .map(|pick| WideOneof { + f1: 1, + f254: 9, + pick, + ..Default::default() + }) + .collect::>() + }}; +} + +#[test] +fn a_oneof_in_a_message_too_wide_for_a_dense_lookup_agrees() { + let opts = buffa::DecodeOptions::new(); + let (unrolled, table) = (wide_oneof_samples!(wideu), wide_oneof_samples!(widet)); + for (u, t) in unrolled.iter().zip(&table) { + let wire = assert_same_codec(u, t); + assert_same_chained::(&wire); + assert_same_on_corrupt_input::( + &wire, true, + ); + // Each member replaces the one that is set, and one that is cut short + // leaves it. + for wire in [ + [varint_field(255, 1), length_delimited_field(256, b"s")].concat(), + [ + length_delimited_field(257, &varint_field(1, 2)), + length_delimited_field(257, &varint_field(1, 3)), + ] + .concat(), + [varint_field(258, 1), tag(257, 2), vec![0x05, 0x08]].concat(), + [varint_field(1, 5), tag(256, 2), vec![0x02, b'a']].concat(), + ] { + assert_same_merge(&opts, u, t, &wire); + } + } +} + +// --------------------------------------------------------------------------- +// Oneofs: a custom pointer, names, and a closed enum without unknown fields +// --------------------------------------------------------------------------- + +macro_rules! pointer_samples { + ($name:ident, $m:ident) => { + mod $name { + use crate::box_type::CustomBox; + use crate::$m::{ + held::Pick, + names::{MyChoice, Type}, + Held, Leaf, Names, + }; + use buffa::MessageField; + + pub fn leaf(id: i32, label: &str) -> Leaf { + Leaf { + id, + label: label.into(), + ..Default::default() + } + } + + fn boxed(value: T) -> CustomBox { + CustomBox(Box::new(value)) + } + + pub fn held() -> Vec { + let held = |pick| Held { + pick, + single: MessageField::some(leaf(9, "single")), + ..Default::default() + }; + let bare = |pick| Held { + pick, + ..Default::default() + }; + let nested = bare(Some(Pick::Again(boxed(bare(Some(Pick::Leaf(boxed( + leaf(2, "deep"), + )))))))); + vec![ + held(None), + held(Some(Pick::N(0))), + held(Some(Pick::S("s".into()))), + held(Some(Pick::Leaf(boxed(Leaf::default())))), + held(Some(Pick::Leaf(boxed(leaf(1, "a"))))), + held(Some(Pick::Again(boxed(nested)))), + ] + } + + /// Every combination of members of the three oneofs of `Names`, + /// including none. + pub fn names() -> Vec { + let types = || { + [ + None, + Some(Type::Third(3)), + Some(Type::First("f".into())), + Some(Type::Second(boxed(leaf(2, "second")))), + ] + }; + let matches = || { + [ + None, + Some(crate::$m::names::Match::MatchNum(0)), + Some(crate::$m::names::Match::MatchText("t".into())), + ] + }; + let choices = || { + [ + None, + Some(MyChoice::FooBar(8)), + Some(MyChoice::BazQux("b".into())), + Some(MyChoice::TheLeaf(boxed(leaf(6, "the")))), + ] + }; + let mut out = Vec::new(); + for r#type in types() { + for r#match in matches() { + for my_choice in choices() { + out.push(Names { + r#type: r#type.clone(), + r#match: r#match.clone(), + my_choice, + ..Default::default() + }); + } + } + } + out + } + } + }; +} + +pointer_samples!(pointers_u, tc5u); +pointer_samples!(pointers_t, tc5t); + +#[test] +fn a_custom_pointer_for_oneof_message_members_agrees() { + let (unrolled, table) = (pointers_u::held(), pointers_t::held()); + for (u, t) in unrolled.iter().zip(&table) { + let wire = assert_same_codec(u, t); + assert_same_chained::(&wire); + assert_same_on_corrupt_input::(&wire, true); + } + let leaf = + |id: u64, label: &[u8]| [varint_field(1, id), length_delimited_field(2, label)].concat(); + let opts = buffa::DecodeOptions::new(); + let wires = [ + // A member merges into itself, and another replaces it. + [ + length_delimited_field(2, &leaf(4, b"")), + length_delimited_field(2, &length_delimited_field(2, b"x")), + ] + .concat(), + [ + length_delimited_field(2, &leaf(4, b"x")), + varint_field(1, 1), + length_delimited_field(2, &varint_field(1, 2)), + ] + .concat(), + [ + varint_field(1, 1), + length_delimited_field(3, &length_delimited_field(2, &leaf(3, b"y"))), + length_delimited_field(3, &varint_field(1, 8)), + ] + .concat(), + // And what fails, part of the way through and after a member is set. + tag(2, 2), + [tag(2, 2), vec![0x05, 0x08]].concat(), + [tag(3, 2), vec![0x04, 0x1a, 0x02, 0x08, 0x01]].concat(), + [varint_field(1, 1), tag(2, 2), vec![0x03, 0x08, 0x05, 0x10]].concat(), + ]; + for wire in &wires { + assert_same_decode::(wire, true); + for (u, t) in unrolled.iter().zip(&table) { + assert_same_merge(&opts, u, t, wire); + } + } +} + +#[test] +fn oneofs_named_like_keywords_with_members_out_of_number_order_agree() { + let (unrolled, table) = (pointers_u::names(), pointers_t::names()); + assert_eq!(table.len(), 48); + for (u, t) in unrolled.iter().zip(&table) { + let wire = assert_same_codec(u, t); + assert_same_chained::(&wire); + } + // Each oneof is written where its lowest member number is, whichever of + // its members is set: `type` at 1, `match` at 4, `myChoice` at 6, so + // `third` (3) comes first, and then `matchNum` (5) and `fooBar` (8). + let msg = table + .iter() + .find(|m| { + use crate::tc5t::names::{Match, MyChoice, Type}; + matches!(m.r#type, Some(Type::Third(3))) + && matches!(m.r#match, Some(Match::MatchNum(0))) + && matches!(m.my_choice, Some(MyChoice::FooBar(8))) + }) + .unwrap(); + let in_order = [varint_field(3, 3), varint_field(5, 0), varint_field(8, 8)]; + assert_eq!(msg.encode_to_vec(), in_order.concat()); + // Decoding does not depend on that order. + let reversed = [ + in_order[2].clone(), + in_order[1].clone(), + in_order[0].clone(), + ] + .concat(); + assert_eq!( + ::decode_from_slice(&reversed).unwrap(), + *msg + ); + assert_same_decode::(&reversed, false); + // The last member of a oneof to arrive wins, from any of its numbers. + for wire in [ + [varint_field(3, 3), length_delimited_field(1, b"f")].concat(), + [length_delimited_field(1, b"f"), varint_field(3, 3)].concat(), + [ + length_delimited_field(2, &varint_field(1, 2)), + varint_field(3, 3), + length_delimited_field(2, &length_delimited_field(2, b"l")), + ] + .concat(), + [ + varint_field(8, 8), + length_delimited_field(7, b"b"), + varint_field(4, 1), + ] + .concat(), + ] { + assert_same_decode::(&wire, false); + assert_same_chained::(&wire); + } + let wire = msg.encode_to_vec(); + assert_same_on_corrupt_input::(&wire, false); +} + +#[test] +fn a_closed_enum_member_the_enum_lacks_is_dropped_when_nothing_keeps_unknown_fields() { + use crate::tc6t::{closed::Pick, Color}; + // n = 5, then shade = 7 (Color has no 7): the member stays, and the number + // is not kept. + let wire = [varint_field(2, 5), varint_field(1, 7)].concat(); + let decoded = ::decode_from_slice(&wire).unwrap(); + assert_eq!(decoded.pick, Some(Pick::N(5))); + assert_eq!(decoded.encode_to_vec(), varint_field(2, 5)); + // With nothing set, it stays unset. + let decoded = ::decode_from_slice(&varint_field(1, 7)).unwrap(); + assert_eq!(decoded, crate::tc6t::Closed::default()); + // A number it has replaces the member. + let wire = [varint_field(2, 5), varint_field(1, 2)].concat(); + assert_eq!( + ::decode_from_slice(&wire) + .unwrap() + .pick, + Some(Pick::Shade(Color::GREEN)) + ); + let inner = |id: u64| length_delimited_field(4, &varint_field(1, id)); + let opts = buffa::DecodeOptions::new(); + let starts = |pick: Option, t: Option| { + ( + crate::tc6u::Closed { + pick, + ..Default::default() + }, + crate::tc6t::Closed { + pick: t, + ..Default::default() + }, + ) + }; + let states = [ + starts(None, None), + starts( + Some(crate::tc6u::closed::Pick::Shade(crate::tc6u::Color::BLUE)), + Some(Pick::Shade(Color::BLUE)), + ), + starts( + Some(crate::tc6u::closed::Pick::S("abc".into())), + Some(Pick::S("abc".into())), + ), + ]; + for wire in [ + varint_field(1, 7), + varint_field(1, 0), + varint_field(1, u64::MAX), + [varint_field(1, 3), varint_field(1, 9)].concat(), + [varint_field(1, 9), varint_field(1, 3)].concat(), + [varint_field(2, 1), varint_field(1, 4), varint_field(5, 6)].concat(), + [inner(1), varint_field(1, 7), inner(2)].concat(), + [tag(1, 0), vec![0x80]].concat(), + [tag(1, 2), vec![0x00]].concat(), + ] { + assert_same_decode::(&wire, true); + assert_same_chained::(&wire); + for (u, t) in &states { + assert_same_merge(&opts, u, t, &wire); + } + } + let sample = crate::tc6t::Closed { + pick: Some(Pick::Shade(Color::BLUE)), + tail: Some(1), + }; + let wire = sample.encode_to_vec(); + assert_same_on_corrupt_input::(&wire, true); } diff --git a/buffa/src/table/decode.rs b/buffa/src/table/decode.rs index 1054ad01..1802ba11 100644 --- a/buffa/src/table/decode.rs +++ b/buffa/src/table/decode.rs @@ -6,8 +6,8 @@ use super::scalar::Sc; use super::{ Bool, Double, Entry, EnumVt, Fixed32, Fixed64, Float, Int32, Int64, Kind, MessageTable, - Sfixed32, Sfixed64, Sint32, Sint64, Uint32, Uint64, IMPLICIT, NO_UNKNOWN, OPTIONAL, PACKED, - REPEATED, REQUIRED, + OneofVt, Sfixed32, Sfixed64, Sint32, Sint64, Uint32, Uint64, IMPLICIT, NO_UNKNOWN, OPTIONAL, + PACKED, REPEATED, REQUIRED, }; use crate::alloc::{string::String, vec::Vec}; use crate::bytes::Buf; @@ -220,10 +220,247 @@ macro_rules! merge_dispatch { (@arm Msg $ty:ident $card:ident $table:ident $e:ident $base:ident $slot:ident $tag:ident $buf:ident $ctx:ident) => { merge_msg::<$card>($table, $e, $slot, $tag, $buf, $ctx) }; + (@arm Oneof $ty:ident $card:ident $table:ident $e:ident $base:ident $slot:ident $tag:ident $buf:ident $ctx:ident) => { + merge_oneof($table, $e, $base, $slot, $tag, $buf, $ctx) + }; } kind_table!(merge_dispatch); +/// Decode the member of a oneof that the entry `e` describes into the oneof. +/// +/// # Safety +/// +/// `slot` points to the `Option` of the oneof enum that `e`'s group describes, +/// inside the live message at `base` of the type `table` describes. +#[inline(never)] +unsafe fn merge_oneof( + table: &MessageTable, + e: &Entry, + base: *mut u8, + slot: *mut u8, + tag: Tag, + buf: &mut &[u8], + ctx: DecodeContext<'_>, +) -> Result<(), DecodeError> { + let m = table.member(e); + let payload_entry = Entry { + kind: m.kind, + aux: m.aux, + ..*e + }; + let oneof = OneofSlot { + group: table.group(m), + slot, + }; + // SAFETY: forwarded from the caller. + unsafe { merge_payload(table, &payload_entry, &oneof, base, tag, buf, ctx) } +} + +/// A oneof inside the message that is being decoded: the `Option` of its enum +/// and its descriptor. +struct OneofSlot<'a> { + group: &'a OneofVt, + slot: *mut u8, +} + +impl OneofSlot<'_> { + /// Make the member `number` the one that is set, and return a pointer to + /// its value. + /// + /// # Safety + /// + /// `slot` points to a live `Option` of the enum that `group` describes, + /// and `number` is a member of it. + #[inline] + unsafe fn place(&self, number: u32) -> *mut u8 { + // SAFETY: forwarded from the caller. + unsafe { (self.group.place)(self.slot, number) } + } + + /// Decode into the member `number`: in place if it is the one that is set, + /// and otherwise into a new default member, which becomes the one that is + /// set only if `f` succeeds. + /// + /// # Safety + /// + /// As for [`place`](Self::place). + #[inline] + unsafe fn place_with( + &self, + number: u32, + f: &mut dyn FnMut(*mut u8) -> Result<(), DecodeError>, + ) -> Result<(), DecodeError> { + // SAFETY: forwarded from the caller. + unsafe { (self.group.place_with)(self.slot, number, f) } + } +} + +/// Defines `merge_payload`, which decodes a oneof member's value by its +/// payload kind. Every arm decodes the value before it replaces the member +/// that is set, so a value that is rejected, or that a closed enum does not +/// know, leaves the oneof as it was. The exception is a message member that +/// is the one that is set, which is merged into as it is decoded, so a failure +/// part of the way through keeps the fields that had been merged. +macro_rules! merge_payload_dispatch { + ($fname:ident; $($name:ident: $fam:ident $ty:ident $card:ident;)*) => { + /// # Safety + /// + /// `payload_entry` is the entry of a member of the oneof `oneof` + /// describes, with its payload's kind and aux index, and the oneof is + /// inside the live message at `base` of the type `table` describes. + #[inline] + unsafe fn $fname( + table: &MessageTable, + payload_entry: &Entry, + oneof: &OneofSlot<'_>, + base: *mut u8, + tag: Tag, + buf: &mut &[u8], + ctx: DecodeContext<'_>, + ) -> Result<(), DecodeError> { + let number = payload_entry.number(); + // SAFETY: each arm stores into the member the entry describes. + unsafe { + match payload_entry.kind { + $(Kind::$name => merge_payload_dispatch!( + @arm $fam $ty table payload_entry oneof number base tag buf ctx + ),)* + #[allow(unreachable_patterns)] + _ => unreachable!("`Table::new` checked the payload kinds"), + } + } + } + }; + (@arm Scalar $ty:ident $table:ident $pe:ident $o:ident $n:ident $base:ident $tag:ident $buf:ident $ctx:ident) => { + merge_oneof_scalar::<$ty>($o, $n, $tag, $buf) + }; + (@arm Str $ty:ident $table:ident $pe:ident $o:ident $n:ident $base:ident $tag:ident $buf:ident $ctx:ident) => { + merge_oneof_str($o, $n, $tag, $buf) + }; + (@arm Bytes $ty:ident $table:ident $pe:ident $o:ident $n:ident $base:ident $tag:ident $buf:ident $ctx:ident) => { + merge_oneof_bytes($o, $n, $tag, $buf) + }; + (@arm Enum $ty:ident $table:ident $pe:ident $o:ident $n:ident $base:ident $tag:ident $buf:ident $ctx:ident) => { + merge_oneof_enum($table, $pe, $o, $base, $tag, $buf, $ctx) + }; + (@arm Msg $ty:ident $table:ident $pe:ident $o:ident $n:ident $base:ident $tag:ident $buf:ident $ctx:ident) => { + merge_oneof_msg($table, $pe, $o, $tag, $buf, $ctx) + }; +} + +payload_kind_table!(merge_payload_dispatch, merge_payload); + +/// # Safety +/// +/// `oneof` is a live oneof whose member `number` has the scalar type `S`. +#[inline] +unsafe fn merge_oneof_scalar( + oneof: &OneofSlot<'_>, + number: u32, + tag: Tag, + buf: &mut &[u8], +) -> Result<(), DecodeError> { + check_wire_type(tag, S::WIRE)?; + let value = S::read(buf)?; + // SAFETY: the caller's contract gives the member's type. + unsafe { *oneof.place(number).cast::() = value }; + Ok(()) +} + +/// # Safety +/// +/// `oneof` is a live oneof whose member `number` is a `String`. +#[inline] +unsafe fn merge_oneof_str( + oneof: &OneofSlot<'_>, + number: u32, + tag: Tag, + buf: &mut &[u8], +) -> Result<(), DecodeError> { + check_wire_type(tag, WireType::LengthDelimited)?; + let value = types::decode_string(buf)?; + // SAFETY: the caller's contract gives the member's type. + unsafe { *oneof.place(number).cast::() = value }; + Ok(()) +} + +/// # Safety +/// +/// `oneof` is a live oneof whose member `number` is a `Vec`. +#[inline] +unsafe fn merge_oneof_bytes( + oneof: &OneofSlot<'_>, + number: u32, + tag: Tag, + buf: &mut &[u8], +) -> Result<(), DecodeError> { + check_wire_type(tag, WireType::LengthDelimited)?; + let value = types::decode_bytes(buf)?; + // SAFETY: the caller's contract gives the member's type. + unsafe { *oneof.place(number).cast::>() = value }; + Ok(()) +} + +/// A closed enum's value with no variant goes to the unknown fields, like an +/// ordinary field, and leaves the member that is set as it is. +/// +/// # Safety +/// +/// `payload_entry` is the entry of an enum member of the live oneof `oneof`, +/// with its payload's kind and aux index, which is inside the live message at +/// `base` of the type `table` describes. +#[inline] +unsafe fn merge_oneof_enum( + table: &MessageTable, + payload_entry: &Entry, + oneof: &OneofSlot<'_>, + base: *mut u8, + tag: Tag, + buf: &mut &[u8], + ctx: DecodeContext<'_>, +) -> Result<(), DecodeError> { + check_wire_type(tag, WireType::Varint)?; + let raw = types::decode_int32(buf)?; + let vt = table.enum_vt(payload_entry); + if !(vt.accepts)(raw) { + // SAFETY: forwarded from the caller. + return unsafe { enum_reject(table, payload_entry, base, raw, ctx) }; + } + // SAFETY: the member is stored in the shape `vt` was built for. + let stored = unsafe { (vt.set)(oneof.place(payload_entry.number()), raw) }; + debug_assert!(stored, "`accepts` said the enum stores {raw}"); + Ok(()) +} + +/// # Safety +/// +/// `payload_entry` is the entry of a message member of the live oneof `oneof`, +/// with its payload's kind and aux index, whose descriptor is for the +/// message's type and reaches it through a pointer to the message. +#[inline] +unsafe fn merge_oneof_msg( + table: &MessageTable, + payload_entry: &Entry, + oneof: &OneofSlot<'_>, + tag: Tag, + buf: &mut &[u8], + ctx: DecodeContext<'_>, +) -> Result<(), DecodeError> { + check_wire_type(tag, WireType::LengthDelimited)?; + let vt = table.msg_vt(payload_entry); + // SAFETY: the member is a message of the type `vt.child` reaches, through + // a pointer to it, and `place_with` hands the closure a pointer to a live + // one. A member that is already set is merged into, as a singular message + // field is. Any other member is replaced only by a message that decoded, + // so a failure leaves the oneof as it was. + unsafe { + oneof.place_with(payload_entry.number(), &mut |child| { + vt.child.merge_sub(child, buf, ctx) + }) + } +} + /// # Safety /// /// `slot` points to a field of scalar type `S` in the shape `C` names. @@ -427,7 +664,28 @@ unsafe fn enum_store( ctx: DecodeContext<'_>, ) -> Result<(), DecodeError> { // SAFETY: `slot` matches the shape `vt` was built for. - if unsafe { (vt.set)(slot, raw) } || table.unknown == NO_UNKNOWN { + if unsafe { (vt.set)(slot, raw) } { + return Ok(()); + } + // SAFETY: forwarded from the caller. + unsafe { enum_reject(table, e, base, raw, ctx) } +} + +/// Handle a value that a closed enum has no variant for: keep it as an unknown +/// field, or drop it if the message keeps none. +/// +/// # Safety +/// +/// `base` points to a live message of the type `table` describes. +#[inline] +unsafe fn enum_reject( + table: &MessageTable, + e: &Entry, + base: *mut u8, + raw: i32, + ctx: DecodeContext<'_>, +) -> Result<(), DecodeError> { + if table.unknown == NO_UNKNOWN { return Ok(()); } ctx.register_unknown_field()?; diff --git a/buffa/src/table/encode.rs b/buffa/src/table/encode.rs index fda7f6af..ce91f74c 100644 --- a/buffa/src/table/encode.rs +++ b/buffa/src/table/encode.rs @@ -81,14 +81,16 @@ fn put_tag(e: &Entry, buf: &mut K) { } } +/// Defines `$fname`, the write of one field by kind, for the kinds listed, as +/// `size_dispatch!` does. macro_rules! write_dispatch { - ($($name:ident: $fam:ident $ty:ident $card:ident;)*) => { + ($fname:ident; $($name:ident: $fam:ident $ty:ident $card:ident;)*) => { /// # Safety /// /// `slot` points to the field `e` describes, in a live message of the /// type `table` describes. #[inline] - unsafe fn write_kind( + unsafe fn $fname( table: &MessageTable, e: &Entry, slot: *const u8, @@ -99,6 +101,8 @@ macro_rules! write_dispatch { unsafe { match e.kind { $(Kind::$name => write_dispatch!(@arm $fam $ty $card table e slot cache buf),)* + #[allow(unreachable_patterns)] + _ => unreachable!("`Table::new` checked the kinds of the entries"), } } } @@ -118,9 +122,44 @@ macro_rules! write_dispatch { (@arm Msg $ty:ident $card:ident $table:ident $e:ident $slot:ident $cache:ident $buf:ident) => { write_msg::<$card, K>($table, $e, $slot, $cache, $buf) }; + (@arm Oneof $ty:ident LEADER $table:ident $e:ident $slot:ident $cache:ident $buf:ident) => { + write_oneof::($table, $e, $slot, $cache, $buf) + }; + // The members that follow the leader are written with it. + (@arm Oneof $ty:ident ONEOF $table:ident $e:ident $slot:ident $cache:ident $buf:ident) => { + () + }; } -kind_table!(write_dispatch); +kind_table!(write_dispatch, write_kind); +payload_kind_table!(write_dispatch, write_payload); + +/// Write the oneof that its leader `e` describes. +/// +/// # Safety +/// +/// `e` is a [`Kind::OneofLeader`] entry, and `slot` points to the `Option` of +/// the oneof enum that its group describes, in a live message of the type +/// `table` describes. +#[inline(never)] +unsafe fn write_oneof( + table: &MessageTable, + e: &Entry, + slot: *const u8, + cache: &mut SizeCache, + buf: &mut K, +) { + let m = table.member(e); + // SAFETY: `slot` is a live `Option` for the `E` the group was built for. + let (number, payload) = unsafe { (table.group(m).get)(slot) }; + if number == 0 { + return; + } + let payload_entry = table.payload_entry(m.group, number); + // SAFETY: `payload` points to a value of the payload kind of member + // `number`, per the oneof enum's `OneofEnum` implementation. + unsafe { write_payload(table, &payload_entry, payload, cache, buf) }; +} /// # Safety /// diff --git a/buffa/src/table/mod.rs b/buffa/src/table/mod.rs index e8bca0cf..b23f084c 100644 --- a/buffa/src/table/mod.rs +++ b/buffa/src/table/mod.rs @@ -52,6 +52,12 @@ //! fields are copied out of the slice it is decoded from, where unrolled //! code decoding from a `Bytes` shares them with the input. //! +//! # Oneofs +//! +//! A oneof has one entry per member, and its members are reached through +//! [`OneofEnum`], which generated code implements for the oneof's enum and +//! whose documentation describes the mechanism. +//! //! # Where the code is compiled //! //! `compute_size`, decoding from a contiguous buffer and encoding into a @@ -70,6 +76,7 @@ use crate::bytes::Buf; use crate::encoding::{Tag, WireType}; use crate::{DecodeContext, DecodeError, EncodeSink, SizeCache, UnknownFields}; +pub use oneof::{Member, OneofEnum, OneofVt}; pub use shape::{ EnumShape, EnumVt, ImplicitClosed, ImplicitOpen, MsgSlot, MsgVt, OptionalClosed, OptionalOpen, RepVt, RepeatedClosed, RepeatedOpen, @@ -115,21 +122,29 @@ macro_rules! __buffa_offset_of_unavailable { #[rustversion::before(1.77)] pub use __buffa_offset_of_unavailable as offset_of; -// Cardinalities. The `Msg` kinds use `IMPLICIT` for a singular field. +// Cardinalities. The `Msg` kinds use `IMPLICIT` for a singular field. The two +// `Oneof` kinds are written with the cardinality names `LEADER` and `ONEOF`, +// which are only tokens that the kind macros match. const IMPLICIT: u8 = 0; const REQUIRED: u8 = 1; const OPTIONAL: u8 = 2; const REPEATED: u8 = 3; const PACKED: u8 = 4; -/// Calls `$callback!` with every [`Kind`] as `Name: Family Type Cardinality;`. +/// Calls `$callback!` with every [`Kind`] as `Name: Family Type Cardinality;`, +/// after `$fname;` if one is given, which names the function a dispatch macro +/// defines. /// -/// The families are `Scalar`, `Str`, `Bytes`, `Enum` and `Msg`. The one list -/// generates the enum and the three dispatch functions, so a kind cannot be -/// added to one and not the others. +/// The families are `Scalar`, `Str`, `Bytes`, `Enum`, `Msg` and `Oneof`. This +/// list generates the [`Kind`] enum and the three dispatch functions over +/// entries (size, write and merge), so a kind cannot be added to one and not +/// the others. [`payload_kind_table!`] is a second list, of the kinds a oneof +/// member's value can have, which generates the three dispatch functions over +/// those values. macro_rules! kind_table { - ($callback:ident) => { + ($callback:ident $(, $fname:ident)?) => { $callback! { + $($fname;)? Int32Implicit: Scalar Int32 IMPLICIT; Int32Required: Scalar Int32 REQUIRED; Int32Optional: Scalar Int32 OPTIONAL; @@ -210,6 +225,36 @@ macro_rules! kind_table { EnumPacked: Enum Enum PACKED; MsgSingular: Msg Msg IMPLICIT; MsgRepeated: Msg Msg REPEATED; + OneofLeader: Oneof Oneof LEADER; + OneofFollower: Oneof Oneof ONEOF; + } + }; +} + +/// [`kind_table!`] for the kinds that a oneof member's payload can have: a +/// value that is written whenever the member is set, so always the `Required` +/// cardinality. +macro_rules! payload_kind_table { + ($callback:ident $(, $fname:ident)?) => { + $callback! { + $($fname;)? + Int32Required: Scalar Int32 REQUIRED; + Int64Required: Scalar Int64 REQUIRED; + Uint32Required: Scalar Uint32 REQUIRED; + Uint64Required: Scalar Uint64 REQUIRED; + Sint32Required: Scalar Sint32 REQUIRED; + Sint64Required: Scalar Sint64 REQUIRED; + BoolRequired: Scalar Bool REQUIRED; + Fixed32Required: Scalar Fixed32 REQUIRED; + Fixed64Required: Scalar Fixed64 REQUIRED; + Sfixed32Required: Scalar Sfixed32 REQUIRED; + Sfixed64Required: Scalar Sfixed64 REQUIRED; + FloatRequired: Scalar Float REQUIRED; + DoubleRequired: Scalar Double REQUIRED; + StrRequired: Str Str REQUIRED; + BytesRequired: Bytes Bytes REQUIRED; + EnumRequired: Enum Enum REQUIRED; + MsgSingular: Msg Msg IMPLICIT; } }; } @@ -278,6 +323,9 @@ macro_rules! define_kind { // descriptor, which the generated entry names. (@slot $name:ident Enum $ty:ident $card:ident) => {}; (@slot $name:ident Msg $ty:ident $card:ident) => {}; + // A oneof member's field is the `Option` of the oneof's enum, which its + // aux descriptors check. + (@slot $name:ident Oneof $ty:ident $card:ident) => {}; (@wire Scalar $ty:ident PACKED) => { WireType::LengthDelimited as u32 }; (@wire Scalar $ty:ident $card:ident) => { <$ty as Sc>::WIRE as u32 }; (@wire Str $ty:ident $card:ident) => { WireType::LengthDelimited as u32 }; @@ -285,15 +333,21 @@ macro_rules! define_kind { (@wire Msg $ty:ident $card:ident) => { WireType::LengthDelimited as u32 }; (@wire Enum $ty:ident PACKED) => { WireType::LengthDelimited as u32 }; (@wire Enum $ty:ident $card:ident) => { WireType::Varint as u32 }; + (@wire Oneof $ty:ident $card:ident) => { + panic!("a oneof member's wire type is its payload kind's, so build its entry with `Entry::oneof_member`") + }; (@shape IMPLICIT) => { IMPLICIT }; (@shape REQUIRED) => { IMPLICIT }; (@shape OPTIONAL) => { OPTIONAL }; (@shape REPEATED) => { REPEATED }; (@shape PACKED) => { REPEATED }; + (@shape ONEOF) => { IMPLICIT }; + (@shape LEADER) => { IMPLICIT }; (@aux Scalar $card:ident) => { None }; (@aux Str $card:ident) => { None }; (@aux Bytes $card:ident) => { None }; (@aux Enum $card:ident) => { Some(AuxKind::Enum) }; + (@aux Oneof $card:ident) => { Some(AuxKind::Member) }; (@aux Msg REPEATED) => { Some(AuxKind::Rep) }; (@aux Msg $card:ident) => { Some(AuxKind::Msg) }; } @@ -308,10 +362,24 @@ pub trait KindSlot { kind_table!(define_kind); +macro_rules! define_payload_check { + ($($name:ident: $fam:ident $ty:ident $card:ident;)*) => { + impl Kind { + /// Whether a oneof member's payload can be of this kind. + const fn is_oneof_payload(self) -> bool { + matches!(self, $(Kind::$name)|*) + } + } + }; +} + +payload_kind_table!(define_payload_check); + // After the macros above, whose textual scope covers only what follows them. mod bridge; mod decode; mod encode; +mod oneof; mod scalar; mod shape; mod size; @@ -322,6 +390,8 @@ enum AuxKind { Msg, Rep, Enum, + Group, + Member, } /// Per-field data that a kind needs beyond the field's offset. @@ -332,6 +402,11 @@ pub enum Aux { Rep(&'static RepVt), /// The descriptor of an enum field (the `Enum*` kinds). Enum(&'static EnumVt), + /// The descriptor of a oneof, which its members' [`Member`] aux items + /// refer to by index. No entry refers to it directly. + Group(&'static OneofVt), + /// One member of a oneof ([`Kind::OneofLeader`] or [`Kind::OneofFollower`]). + Member(Member), } impl Aux { @@ -340,6 +415,8 @@ impl Aux { Aux::Msg(_) => AuxKind::Msg, Aux::Rep(_) => AuxKind::Rep, Aux::Enum(_) => AuxKind::Enum, + Aux::Group(_) => AuxKind::Group, + Aux::Member(_) => AuxKind::Member, } } } @@ -395,6 +472,37 @@ impl Entry { } } + /// An entry for member `number` of a oneof stored `offset` bytes into the + /// message struct, whose value is of kind `payload`. `aux` is the index of + /// its [`Aux::Member`] item, and `leader` is whether it is the member with + /// the lowest number, which sizes and writes the oneof. + /// + /// # Panics + /// + /// Panics, at compile time when used to initialise a `static`, if + /// `payload` is not a kind that a oneof member can have, or as for + /// [`Entry::new`]. + #[must_use] + pub const fn oneof_member( + payload: Kind, + leader: bool, + number: u32, + offset: usize, + aux: u16, + ) -> Self { + assert!( + payload.is_oneof_payload(), + "a oneof member's payload must be a `Required` scalar, string, bytes or enum kind, or `MsgSingular`" + ); + let mut e = Self::new(payload, number, offset, aux); + e.kind = if leader { + Kind::OneofLeader + } else { + Kind::OneofFollower + }; + e + } + const fn number(&self) -> u32 { self.tag >> 3 } @@ -450,6 +558,49 @@ impl MessageTable { _ => unreachable!("`Table::new` checked that enum entries index enum descriptors"), } } + + #[inline] + fn member(&self, e: &Entry) -> Member { + match &self.aux[usize::from(e.aux)] { + Aux::Member(m) => *m, + _ => unreachable!("`Table::new` checked that oneof entries index `Member`s"), + } + } + + #[inline] + fn group(&self, m: Member) -> &'static OneofVt { + match &self.aux[usize::from(m.group)] { + Aux::Group(g) => g, + _ => unreachable!("`Table::new` checked that members index oneof descriptors"), + } + } + + /// The entry of the member `number` of the oneof whose descriptor is at + /// aux index `group`, with its kind and aux index replaced by those of its + /// payload, which the payload arms of the interpreters take. + /// + /// # Panics + /// + /// Panics if the message has no such oneof member, which only an incorrect + /// [`OneofEnum`] can cause. + #[inline] + fn payload_entry(&self, group: u16, number: u32) -> Entry { + let Some(e) = self.find(number) else { + oneof::no_such_member(number) + }; + if !matches!(e.kind, Kind::OneofLeader | Kind::OneofFollower) { + oneof::no_such_member(number) + } + let m = self.member(e); + if m.group != group { + oneof::no_such_member(number) + } + Entry { + kind: m.kind, + aux: m.aux, + ..*e + } + } } /// The static description of message type `M`, from which the interpreters @@ -475,8 +626,11 @@ impl Table { /// is not [`ABI`], if an entry lies outside `M`, if the entries are not in /// strictly increasing field-number order, if an entry's aux index is out /// of range or names a descriptor of the wrong variant or, for an enum, - /// the wrong cardinality, if `dense` disagrees with `entries`, or if - /// `unknown` does not leave room for an `UnknownFields`. + /// the wrong cardinality, if `dense` disagrees with `entries`, if + /// `unknown` does not leave room for an `UnknownFields`, or if a oneof is + /// inconsistent: a member whose group, payload kind, tag, offset or + /// leader kind disagrees with its oneof's descriptor, or a oneof whose + /// lowest-numbered member is not its only leader. /// /// # Safety /// @@ -491,7 +645,14 @@ impl Table { /// `Bytes*`: `Vec`, `Option>` or `Vec>`; /// - `Enum*`: the storage the entry's [`EnumVt`] was built for; /// - `MsgSingular`: the storage the [`MsgVt`] was built for, and - /// `MsgRepeated`: the `Vec` the [`RepVt`] was built for. + /// `MsgRepeated`: the `Vec` the [`RepVt`] was built for; + /// - `OneofLeader` and `OneofFollower`: an `Option`, where the + /// [`OneofVt`] of the member's group was built for `E` (with + /// [`OneofVt::with_messages`] if a member is a message), and `E`'s + /// [`OneofEnum`] implementation gives, for the member's number, a pointer + /// to a value of the member's payload kind, under the same rules as the + /// kinds above (for `MsgSingular`, a [`MsgVt::direct`] or + /// [`MsgVt::direct_via_message`] descriptor of the message). /// /// `unknown`, if present, must be the offset of a field of type /// `UnknownFields`. The `__table_entry!` macro checks the field types @@ -511,6 +672,7 @@ impl Table { regenerate it with the buffa-codegen that matches this buffa" ); let size = core::mem::size_of::(); + let mut leaders = 0; let mut i = 0; while i < entries.len() { let e = &entries[i]; @@ -538,9 +700,34 @@ impl Table { "buffa table: an enum entry's descriptor has the wrong cardinality for its kind" ); } + if let Aux::Msg(vt) = a { + assert!( + !vt.direct, + "buffa table: a message field's descriptor must not be a `MsgVt::direct` one" + ); + } + if let Aux::Member(m) = a { + if oneof::check_member(e, *m, aux) { + leaders += 1; + } + } + } + i += 1; + } + // Every oneof has a leader, and no two share one, so the messages + // written for its members are written once. + let mut groups = 0; + i = 0; + while i < aux.len() { + if let Aux::Group(_) = &aux[i] { + groups += 1; } i += 1; } + assert!( + leaders == groups, + "buffa table: every oneof descriptor needs exactly one leading member" + ); assert!( dense.is_empty() || entries.len() < 255, "buffa table: the dense lookup needs fewer than 255 entries" @@ -818,10 +1005,26 @@ macro_rules! __table { /// /// The scalar, string and bytes kinds have a fixed field type. The enum and /// message kinds take the type explicitly, as `aux = , slot = `, -/// where the type is the one their aux descriptor was built for. +/// where the type is the one their aux descriptor was built for. A oneof +/// member is written `oneof(, )`, where `` says +/// whether it is the oneof's lowest-numbered member, with the `Option` of the +/// oneof's enum as its slot type and the index of its [`Member`] as `aux`. #[doc(hidden)] #[macro_export] macro_rules! __table_entry { + ( + $msg:ty, $field:ident, oneof($payload:ident, $leader:expr), $number:expr, + aux = $aux:expr, slot = $slot:ty $(,)? + ) => {{ + const _: fn(&$msg) -> *const $slot = |m| ::core::ptr::addr_of!(m.$field); + $crate::table::Entry::oneof_member( + $crate::table::Kind::$payload, + $leader, + $number, + $crate::table::offset_of!($msg, $field), + $aux, + ) + }}; ($msg:ty, $field:ident, $kind:ident, $number:expr $(,)?) => {{ // A raw pointer, unlike a reference, cannot be deref-coerced, so this // needs the field's type to be exactly the slot type. diff --git a/buffa/src/table/oneof.rs b/buffa/src/table/oneof.rs new file mode 100644 index 00000000..40c2aa3a --- /dev/null +++ b/buffa/src/table/oneof.rs @@ -0,0 +1,335 @@ +//! Oneof fields: the trait generated code implements for a oneof enum and the +//! descriptor the interpreters reach its members through. [`OneofEnum`] +//! documents the mechanism. + +use super::{Entry, Kind}; +use crate::DecodeError; + +/// How the interpreters reach the members of the oneof enum `Self`, which +/// generated code implements for the enum of every oneof of a table message. +/// +/// A oneof is stored as an `Option`, where `E` is an enum with one variant +/// per member. The layout of that enum is unspecified, so a table cannot +/// address a member by offset as it does an ordinary field. The table has one +/// entry per member instead, of kind [`Kind::OneofLeader`] or +/// [`Kind::OneofFollower`], and every one has the offset of the `Option`. +/// This trait gives the interpreters the number of the current member, a +/// pointer to its value, and a way to set another member. +/// +/// A member is written when it is set, whatever its value, so its value is +/// described by a `Required` kind, its *payload kind*, and is sized and +/// written by the arm of that kind, as an ordinary field of that kind is. The +/// entry of the member with the lowest field number, the *leader*, sizes and +/// writes whichever member is set, at that position among the message's +/// fields, as unrolled code does. The other entries only decode, so a message +/// with a oneof writes the same bytes under either strategy. +/// +/// Decoding does what unrolled code does, through arms of its own for the +/// members of a oneof. A value is decoded before it replaces the member that +/// is set, so one that is rejected leaves the oneof as it was, and a closed +/// enum's number with no variant goes to the unknown fields. A message member +/// that is already the one that is set is merged into, and any other is +/// decoded into a new default member that is set only if the decoding +/// succeeds. +/// +/// # Contract +/// +/// The interpreters trust an implementation, so an incorrect one makes the +/// table's unsafe accesses wrong, although the trait is safe to implement: a +/// [`Table`](super::Table) is built with `unsafe` code that takes on the +/// contract below. Generated code meets it by construction, and a check that +/// it would break is a panic where the interpreters can make one. +/// +/// - [`number`](Self::number) is the number of a member of this oneof, one +/// that has an entry in the message's table. +/// - [`payload`](Self::payload) and [`payload_mut`](Self::payload_mut) return +/// non-null, aligned pointers, valid for as long as the borrow of `self`, to +/// the same value, and its type is the one the member's payload kind names: +/// `i32` for `Int32Required`, `String` for `StrRequired`, the storage of the +/// enum for `EnumRequired`, and the message itself, reached through its +/// pointer if it is boxed, for `MsgSingular`. +/// - `with_default(n)` is `None` only if `n` is not a member of this oneof, +/// and otherwise a value whose `number()` is `n`. +pub trait OneofEnum: Sized { + /// The field number of the member `self` holds. + fn number(&self) -> u32; + + /// A pointer to the value of the member `self` holds. + fn payload(&self) -> *const u8; + + /// As [`payload`](Self::payload), for writing. + fn payload_mut(&mut self) -> *mut u8; + + /// A value holding the default value of the member `number`, or `None` if + /// `Self` has no such member. + fn with_default(number: u32) -> Option; +} + +/// Descriptor of a oneof: how to read and replace the member of an +/// `Option` through its address. +pub struct OneofVt { + /// The offset of the `Option` in the message struct. + pub(super) offset: u32, + /// The lowest field number among the members. + pub(super) first: u32, + /// The number of the member that is set, or `0`, and a pointer to its + /// value. + /// + /// # Safety + /// + /// The argument points to a live `Option`. + pub(super) get: unsafe fn(*const u8) -> (u32, *const u8), + /// Make the member `number` the one that is set, keeping its value if it + /// already is, and return a pointer to its value. + /// + /// # Safety + /// + /// The argument points to a live `Option`, and `number` is a member of + /// `E`. + pub(super) place: unsafe fn(*mut u8, u32) -> *mut u8, + /// Decode a message member the way unrolled code does: if the member + /// `number` is the one that is set, run the function on its value, which + /// merges into it. Otherwise run it on the value of a new default member, + /// and make that the member that is set only if the function succeeds, so + /// a failure leaves the oneof as it was. + /// + /// # Safety + /// + /// The first argument points to a live `Option`, and `number` is a + /// member of `E`. + pub(super) place_with: unsafe fn(*mut u8, u32, PlaceFn<'_>) -> Result<(), DecodeError>, + /// Whether `place_with` works, which a oneof with a message member needs. + pub(super) messages: bool, +} + +/// The function that [`OneofVt`]'s `place_with` runs on a member's value. +pub(super) type PlaceFn<'a> = &'a mut dyn FnMut(*mut u8) -> Result<(), DecodeError>; + +/// # Safety +/// +/// `slot` points to a live `Option`. +unsafe fn get_impl(slot: *const u8) -> (u32, *const u8) { + // SAFETY: the caller passes a pointer to a live `Option`. + match unsafe { &*slot.cast::>() } { + Some(e) => { + let payload = e.payload(); + debug_assert!(!payload.is_null(), "`OneofEnum::payload` returned null"); + (e.number(), payload) + } + None => (0, core::ptr::null()), + } +} + +/// # Safety +/// +/// `slot` points to a live `Option`. +unsafe fn place_impl(slot: *mut u8, number: u32) -> *mut u8 { + // SAFETY: the caller passes a pointer to a live `Option`. + let e = unsafe { &mut *slot.cast::>() }; + if e.as_ref().map(E::number) != Some(number) { + *e = Some(new_member(number)); + } + match e { + Some(e) => { + let payload = e.payload_mut(); + debug_assert!(!payload.is_null(), "`OneofEnum::payload_mut` returned null"); + payload + } + None => no_such_member(number), + } +} + +/// # Safety +/// +/// `slot` points to a live `Option`. +unsafe fn place_with_impl( + slot: *mut u8, + number: u32, + f: PlaceFn<'_>, +) -> Result<(), DecodeError> { + // SAFETY: the caller passes a pointer to a live `Option`. + let e = unsafe { &mut *slot.cast::>() }; + let is_set = matches!(e, Some(current) if current.number() == number); + let mut fresh: Option = None; + let member = if is_set { + e.as_mut() + } else { + fresh = Some(new_member(number)); + fresh.as_mut() + }; + let Some(member) = member else { + no_such_member(number) + }; + let payload = member.payload_mut(); + debug_assert!(!payload.is_null(), "`OneofEnum::payload_mut` returned null"); + let result = f(payload); + if result.is_ok() && !is_set { + // The swap sets the new member and leaves the one it replaces in + // `fresh`, so whichever member is discarded, that one or a new one + // that failed to decode, is dropped where `fresh` goes out of scope. + core::mem::swap(e, &mut fresh); + } + result +} + +/// A value of `E` holding the default of the member `number`. A +/// [`OneofEnum`] that has no such member, or whose value for it is numbered +/// otherwise, is a bug in the implementation, which cannot be allowed to +/// address a different member's value. +fn new_member(number: u32) -> E { + match E::with_default(number) { + Some(e) if e.number() == number => e, + _ => no_such_member(number), + } +} + +/// The message has an entry, or an accessor, for a member that its oneof enum +/// does not have, which only a bug in generated code can cause. +#[cold] +#[inline(never)] +pub(super) fn no_such_member(number: u32) -> ! { + panic!("buffa table: oneof member {number} does not match the oneof enum") +} + +/// The `place_with` of a oneof built without message members, which `Table::new` +/// does not let a message member index. +/// +/// # Safety +/// +/// As for [`OneofVt`]'s `place_with`. +unsafe fn place_with_unavailable( + _: *mut u8, + number: u32, + _: PlaceFn<'_>, +) -> Result<(), DecodeError> { + no_such_member(number) +} + +impl OneofVt { + /// Describe the oneof stored as an `Option` at `offset` in the message + /// struct, whose lowest member number is `first`, none of whose members + /// is a message. [`with_messages`](Self::with_messages) describes one that + /// has a message member, and `Table::new` rejects a message member of + /// this one. + /// + /// # Panics + /// + /// Panics, at compile time when used to initialise a `static`, if + /// `offset` does not fit in a `u32` or `first` is not a field number. + #[must_use] + pub const fn new(offset: usize, first: u32) -> Self { + assert!(offset <= u32::MAX as usize, "field offset out of range"); + assert!(first >= 1 && first < (1 << 29), "field number out of range"); + Self { + offset: offset as u32, + first, + get: get_impl::, + place: place_impl::, + place_with: place_with_unavailable, + messages: false, + } + } + + /// As [`new`](Self::new), for a oneof that has a message member. The code + /// that decodes into a member in place is instantiated for `E` only for + /// such a oneof, which keeps the others smaller. + /// + /// # Panics + /// + /// As for [`new`](Self::new). + #[must_use] + pub const fn with_messages(offset: usize, first: u32) -> Self { + let mut vt = Self::new::(offset, first); + vt.place_with = place_with_impl::; + vt.messages = true; + vt + } +} + +/// One member of a oneof, referred to by the aux index of its entry. +#[derive(Clone, Copy, Debug)] +pub struct Member { + /// The aux index of the oneof's [`Aux::Group`](super::Aux::Group). + pub(super) group: u16, + /// The aux index of the payload's descriptor, for a payload kind that has + /// one. + pub(super) aux: u16, + /// The kind of the member's value. + pub(super) kind: Kind, +} + +impl Member { + /// A member of the oneof whose group is at aux index `group`, holding a + /// value of kind `kind`, whose descriptor, if it needs one, is at aux + /// index `aux`. + #[must_use] + pub const fn new(group: u16, kind: Kind, aux: u16) -> Self { + Self { group, aux, kind } + } +} + +/// Check the member `m` of the entry `e` against the rest of the aux array, +/// and return whether it is the leader of its oneof. +/// +/// # Panics +/// +/// Panics, at compile time when used to initialise a `static`, if the member is +/// inconsistent with the oneof it names or with its payload's descriptor. +pub(super) const fn check_member(e: &Entry, m: Member, aux: &[super::Aux]) -> bool { + assert!( + (m.group as usize) < aux.len(), + "buffa table: a oneof member's group index is out of range" + ); + let super::Aux::Group(g) = &aux[m.group as usize] else { + panic!("buffa table: a oneof member's group index is not a oneof descriptor") + }; + assert!( + m.kind.is_oneof_payload(), + "buffa table: a oneof member's payload kind is not one a payload can have" + ); + assert!( + e.tag & 7 == m.kind.wire_type(), + "buffa table: a oneof member's tag does not have its payload kind's wire type" + ); + assert!( + g.messages || m.kind as u8 != super::Kind::MsgSingular as u8, + "buffa table: a oneof with a message member must be built with `OneofVt::with_messages`" + ); + if let Some(want) = m.kind.aux_kind() { + assert!( + (m.aux as usize) < aux.len(), + "buffa table: a oneof member's payload aux index is out of range" + ); + let a = &aux[m.aux as usize]; + assert!( + a.kind() as u8 == want as u8, + "buffa table: a oneof member's payload descriptor is the wrong variant for its kind" + ); + if let super::Aux::Enum(vt) = a { + assert!( + vt.card == super::IMPLICIT, + "buffa table: a oneof member's enum descriptor must be for a singular field" + ); + } + if let super::Aux::Msg(vt) = a { + assert!( + vt.direct, + "buffa table: a oneof member's message descriptor must be a `MsgVt::direct` or `MsgVt::direct_via_message` one" + ); + } + } + assert!( + e.offset == g.offset, + "buffa table: a oneof member's offset differs from its oneof's" + ); + assert!( + e.number() >= g.first, + "buffa table: a oneof member is numbered below its oneof's lowest" + ); + let leader = e.kind as u8 == Kind::OneofLeader as u8; + assert!( + leader == (e.number() == g.first), + "buffa table: the leader of a oneof must be exactly the member with its lowest number" + ); + leader +} diff --git a/buffa/src/table/shape.rs b/buffa/src/table/shape.rs index 903bf20c..e3017a33 100644 --- a/buffa/src/table/shape.rs +++ b/buffa/src/table/shape.rs @@ -53,6 +53,10 @@ pub struct MsgVt { /// /// The argument points to a live `F`. pub(super) get: unsafe fn(*const u8) -> *const u8, + /// Whether the descriptor is for a message reached through a pointer to + /// the message itself ([`direct`](Self::direct)), which is what a oneof + /// member needs, and not through a field's storage. + pub(super) direct: bool, } /// # Safety @@ -74,6 +78,23 @@ unsafe fn get_impl(slot: *const u8) -> *const u8 { } } +/// The accessors of a message that the table reaches through a pointer to +/// the message itself: the message is there whenever the pointer is. +/// +/// # Safety +/// +/// `slot` points to a live message, which is the one that is returned. +unsafe fn direct_place(slot: *mut u8) -> *mut u8 { + slot +} + +/// # Safety +/// +/// `slot` points to a live message, which is the one that is returned. +unsafe fn direct_get(slot: *const u8) -> *const u8 { + slot +} + impl MsgVt { /// Describe a field of type `F`, whose message is a table message and /// `table` is its table. @@ -83,6 +104,7 @@ impl MsgVt { child: Child::Table(&table.raw), place: place_impl::, get: get_impl::, + direct: false, } } @@ -101,6 +123,72 @@ impl MsgVt { child: Child::of::(), place: place_impl::, get: get_impl::, + direct: false, + } + } + + /// Describe a message that is reached through a pointer to the message + /// itself, which is how a oneof member's payload of message type is + /// reached, whether the oneof stores it boxed or inline. `table` + /// describes the message `M`, the type the oneof's accessors give a + /// pointer to. Generated code names `M`, so a table of another message is + /// a type error: + /// + /// ``` + /// use buffa::table::{MsgVt, Table}; + /// + /// struct Point { + /// x: i32, + /// } + /// static POINT: Table = buffa::__table!( + /// Point, + /// abi = buffa::table::ABI, + /// entries = [buffa::__table_entry!(Point, x, Int32Implicit, 1)], + /// dense = &[0, 1], + /// aux = [], + /// unknown = none, + /// ); + /// let _ = MsgVt::direct::(&POINT); + /// ``` + /// + /// ```compile_fail,E0308 + /// use buffa::table::{MsgVt, Table}; + /// + /// struct Point { + /// x: i32, + /// } + /// struct Other; + /// static POINT: Table = buffa::__table!( + /// Point, + /// abi = buffa::table::ABI, + /// entries = [buffa::__table_entry!(Point, x, Int32Implicit, 1)], + /// dense = &[0, 1], + /// aux = [], + /// unknown = none, + /// ); + /// // The payload is an `Other`, but the table is a `Point`'s. + /// let _ = MsgVt::direct::(&POINT); + /// ``` + #[must_use] + pub const fn direct(table: &'static Table) -> Self { + Self { + child: Child::Table(&table.raw), + place: direct_place, + get: direct_get, + direct: true, + } + } + + /// As [`direct`](Self::direct), for a message whose table is not visible + /// here, which is reached through its [`Message`] impl; see + /// [`new_via_message`](Self::new_via_message). + #[must_use] + pub const fn direct_via_message() -> Self { + Self { + child: Child::of::(), + place: direct_place, + get: direct_get, + direct: true, } } } @@ -221,6 +309,9 @@ pub struct EnumVt { /// /// As for `set`. pub(super) len: unsafe fn(*const u8) -> usize, + /// Whether `set` would store `raw`: `false` for a closed enum that has no + /// variant with that number. + pub(super) accepts: fn(i32) -> bool, } /// A way of storing an enum field, implemented by the marker types below. @@ -262,6 +353,15 @@ pub unsafe trait EnumShape { unsafe fn len(_slot: *const u8) -> usize { 0 } + + /// Whether [`set`](Self::set) would store `raw`. + /// + /// It must agree with `set`. A oneof member is placed, replacing the + /// member that was set, before `set` runs, so `merge_oneof_enum` asks + /// `accepts` first and leaves the oneof as it was for a value that fails. + /// A `set` that then refused a value `accepts` allowed would leave a + /// default member where another was set. + fn accepts(raw: i32) -> bool; } impl EnumVt { @@ -273,12 +373,13 @@ impl EnumVt { set: S::set, get: S::get, len: S::len, + accepts: S::accepts, } } } macro_rules! enum_shape { - ($(#[$m:meta])* $name:ident, $card:ident, $slot:ty, $set:expr, $get:expr, $len:expr) => { + ($(#[$m:meta])* $name:ident, $card:ident, $slot:ty, $set:expr, $get:expr, $len:expr, $accepts:expr) => { $(#[$m])* pub struct $name(PhantomData); @@ -308,6 +409,11 @@ macro_rules! enum_shape { let s = unsafe { &*slot.cast::<$slot>() }; ($len)(s) } + + #[inline] + fn accepts(raw: i32) -> bool { + ($accepts)(raw) + } } }; } @@ -317,40 +423,46 @@ enum_shape!( ImplicitOpen, IMPLICIT, EnumValue, |s: &mut EnumValue, raw| { *s = EnumValue::from(raw); true }, |s: &EnumValue, _| Some(s.to_i32()), - |_: &EnumValue| 0 + |_: &EnumValue| 0, + |_: i32| true ); enum_shape!( /// A closed enum with implicit presence: `E`. ImplicitClosed, IMPLICIT, E, |s: &mut E, raw| match E::from_i32(raw) { Some(v) => { *s = v; true } None => false }, |s: &E, _| Some(s.to_i32()), - |_: &E| 0 + |_: &E| 0, + |raw| E::from_i32(raw).is_some() ); enum_shape!( /// An open enum with explicit presence: `Option>`. OptionalOpen, OPTIONAL, Option>, |s: &mut Option>, raw| { *s = Some(EnumValue::from(raw)); true }, |s: &Option>, _| s.as_ref().map(EnumValue::to_i32), - |_: &Option>| 0 + |_: &Option>| 0, + |_: i32| true ); enum_shape!( /// A closed enum with explicit presence: `Option`. OptionalClosed, OPTIONAL, Option, |s: &mut Option, raw| match E::from_i32(raw) { Some(v) => { *s = Some(v); true } None => false }, |s: &Option, _| s.as_ref().map(Enumeration::to_i32), - |_: &Option| 0 + |_: &Option| 0, + |raw| E::from_i32(raw).is_some() ); enum_shape!( /// A repeated open enum: `Vec>`. RepeatedOpen, REPEATED, Vec>, |s: &mut Vec>, raw| { s.push(EnumValue::from(raw)); true }, |s: &Vec>, i| s.get(i).map(EnumValue::to_i32), - |s: &Vec>| s.len() + |s: &Vec>| s.len(), + |_: i32| true ); enum_shape!( /// A repeated closed enum: `Vec`. RepeatedClosed, REPEATED, Vec, |s: &mut Vec, raw| match E::from_i32(raw) { Some(v) => { s.push(v); true } None => false }, |s: &Vec, i| s.get(i).map(Enumeration::to_i32), - |s: &Vec| s.len() + |s: &Vec| s.len(), + |raw| E::from_i32(raw).is_some() ); diff --git a/buffa/src/table/size.rs b/buffa/src/table/size.rs index f983cfb8..15bcf038 100644 --- a/buffa/src/table/size.rs +++ b/buffa/src/table/size.rs @@ -34,14 +34,17 @@ pub(super) unsafe fn compute_size( crate::saturate_size(size) } +/// Defines `$fname`, the size of one field by kind, for the kinds listed. Two +/// lists are used, all kinds for `size_kind` and the payload kinds of a oneof +/// member for `size_payload`. macro_rules! size_dispatch { - ($($name:ident: $fam:ident $ty:ident $card:ident;)*) => { + ($fname:ident; $($name:ident: $fam:ident $ty:ident $card:ident;)*) => { /// # Safety /// /// `slot` points to the field `e` describes, in a live message of the /// type `table` describes. #[inline] - unsafe fn size_kind( + unsafe fn $fname( table: &MessageTable, e: &Entry, slot: *const u8, @@ -52,6 +55,10 @@ macro_rules! size_dispatch { unsafe { match e.kind { $(Kind::$name => size_dispatch!(@arm $fam $ty $card table e tl slot cache),)* + // A list of the payload kinds leaves the other kinds out, and + // `Table::new` rules them out of a payload. + #[allow(unreachable_patterns)] + _ => unreachable!("`Table::new` checked the kinds of the entries"), } } } @@ -71,9 +78,43 @@ macro_rules! size_dispatch { (@arm Msg $ty:ident $card:ident $table:ident $e:ident $tl:ident $slot:ident $cache:ident) => { size_msg::<$card>($table, $e, $tl, $slot, $cache) }; + (@arm Oneof $ty:ident LEADER $table:ident $e:ident $tl:ident $slot:ident $cache:ident) => { + size_oneof($table, $e, $slot, $cache) + }; + // The members that follow the leader are sized with it. + (@arm Oneof $ty:ident ONEOF $table:ident $e:ident $tl:ident $slot:ident $cache:ident) => { + 0 + }; } -kind_table!(size_dispatch); +kind_table!(size_dispatch, size_kind); +payload_kind_table!(size_dispatch, size_payload); + +/// The size of the oneof that its leader `e` describes. +/// +/// # Safety +/// +/// `e` is a [`Kind::OneofLeader`] entry, and `slot` points to the `Option` of +/// the oneof enum that its group describes, in a live message of the type +/// `table` describes. +#[inline(never)] +unsafe fn size_oneof( + table: &MessageTable, + e: &Entry, + slot: *const u8, + cache: &mut SizeCache, +) -> u64 { + let m = table.member(e); + // SAFETY: `slot` is a live `Option` for the `E` the group was built for. + let (number, payload) = unsafe { (table.group(m).get)(slot) }; + if number == 0 { + return 0; + } + let payload_entry = table.payload_entry(m.group, number); + // SAFETY: `payload` points to a value of the payload kind of member + // `number`, per the oneof enum's `OneofEnum` implementation. + unsafe { size_payload(table, &payload_entry, payload, cache) } +} /// # Safety /// diff --git a/buffa/src/table/tests.rs b/buffa/src/table/tests.rs index dee82f4a..e892bffb 100644 --- a/buffa/src/table/tests.rs +++ b/buffa/src/table/tests.rs @@ -1512,3 +1512,1117 @@ fn a_child_can_be_written_by_calling_write_to_on_a_buffer() { assert_eq!(out.len(), size as usize); assert_eq!(out, msg.encode_to_vec()); } + +// --------------------------------------------------------------------------- +// Oneofs +// --------------------------------------------------------------------------- + +/// `oneof pick { int32 num = 2; string text = 4; bytes blob = 5; Color open = 7; +/// Color strict = 8; Inner child = 9; }`, with the message boxed. +#[derive(Clone, Debug, PartialEq)] +enum Pick { + Num(i32), + Text(String), + Blob(Vec), + Open(EnumValue), + Strict(Color), + Child(crate::alloc::boxed::Box), +} + +impl OneofEnum for Pick { + fn number(&self) -> u32 { + match self { + Self::Num(_) => 2, + Self::Text(_) => 4, + Self::Blob(_) => 5, + Self::Open(_) => 7, + Self::Strict(_) => 8, + Self::Child(_) => 9, + } + } + + fn payload(&self) -> *const u8 { + match self { + Self::Num(v) => (v as *const i32).cast(), + Self::Text(v) => (v as *const String).cast(), + Self::Blob(v) => (v as *const Vec).cast(), + Self::Open(v) => (v as *const EnumValue).cast(), + Self::Strict(v) => (v as *const Color).cast(), + Self::Child(v) => (&**v as *const Inner).cast(), + } + } + + fn payload_mut(&mut self) -> *mut u8 { + match self { + Self::Num(v) => (v as *mut i32).cast(), + Self::Text(v) => (v as *mut String).cast(), + Self::Blob(v) => (v as *mut Vec).cast(), + Self::Open(v) => (v as *mut EnumValue).cast(), + Self::Strict(v) => (v as *mut Color).cast(), + Self::Child(v) => (&mut **v as *mut Inner).cast(), + } + } + + fn with_default(number: u32) -> Option { + Some(match number { + 2 => Self::Num(Default::default()), + 4 => Self::Text(Default::default()), + 5 => Self::Blob(Default::default()), + 7 => Self::Open(Default::default()), + 8 => Self::Strict(Default::default()), + 9 => Self::Child(Default::default()), + _ => return None, + }) + } +} + +/// `oneof other { float x = 6; Inner y = 10; }`, with the message inline. +#[derive(Clone, Debug, PartialEq)] +enum Alt { + X(f32), + Y(Inner), +} + +impl OneofEnum for Alt { + fn number(&self) -> u32 { + match self { + Self::X(_) => 6, + Self::Y(_) => 10, + } + } + + fn payload(&self) -> *const u8 { + match self { + Self::X(v) => (v as *const f32).cast(), + Self::Y(v) => (v as *const Inner).cast(), + } + } + + fn payload_mut(&mut self) -> *mut u8 { + match self { + Self::X(v) => (v as *mut f32).cast(), + Self::Y(v) => (v as *mut Inner).cast(), + } + } + + fn with_default(number: u32) -> Option { + Some(match number { + 6 => Self::X(Default::default()), + 10 => Self::Y(Default::default()), + _ => return None, + }) + } +} + +/// `int32 a = 1; oneof pick {...}; int32 b = 3; oneof other {...}`, keeping +/// unknown fields. `pick` has members either side of `b`, so where its +/// members are written shows. +#[derive(Clone, Debug, Default, PartialEq)] +struct Holder { + a: i32, + pick: Option, + b: i32, + other: Option, + unknown: UnknownFields, +} + +static HOLDER: Table = crate::__table!( + Holder, + abi = ABI, + entries = [ + crate::__table_entry!(Holder, a, Int32Implicit, 1), + crate::__table_entry!( + Holder, + pick, + oneof(Int32Required, true), + 2, + aux = 2, + slot = Option + ), + crate::__table_entry!(Holder, b, Int32Implicit, 3), + crate::__table_entry!( + Holder, + pick, + oneof(StrRequired, false), + 4, + aux = 3, + slot = Option + ), + crate::__table_entry!( + Holder, + pick, + oneof(BytesRequired, false), + 5, + aux = 4, + slot = Option + ), + crate::__table_entry!( + Holder, + other, + oneof(FloatRequired, true), + 6, + aux = 5, + slot = Option + ), + crate::__table_entry!( + Holder, + pick, + oneof(EnumRequired, false), + 7, + aux = 6, + slot = Option + ), + crate::__table_entry!( + Holder, + pick, + oneof(EnumRequired, false), + 8, + aux = 7, + slot = Option + ), + crate::__table_entry!( + Holder, + pick, + oneof(MsgSingular, false), + 9, + aux = 10, + slot = Option + ), + crate::__table_entry!( + Holder, + other, + oneof(MsgSingular, false), + 10, + aux = 12, + slot = Option + ), + ], + dense = &dense::<11>(&[1, 2, 3, 4, 5, 6, 7, 8, 9, 10]), + aux = [ + Aux::Group(&OneofVt::with_messages::( + crate::table::offset_of!(Holder, pick), + 2 + )), + Aux::Group(&OneofVt::with_messages::( + crate::table::offset_of!(Holder, other), + 6 + )), + Aux::Member(Member::new(0, Kind::Int32Required, 0)), + Aux::Member(Member::new(0, Kind::StrRequired, 0)), + Aux::Member(Member::new(0, Kind::BytesRequired, 0)), + Aux::Member(Member::new(1, Kind::FloatRequired, 0)), + Aux::Member(Member::new(0, Kind::EnumRequired, 8)), + Aux::Member(Member::new(0, Kind::EnumRequired, 9)), + Aux::Enum(&EnumVt::new::>()), + Aux::Enum(&EnumVt::new::>()), + Aux::Member(Member::new(0, Kind::MsgSingular, 11)), + Aux::Msg(&MsgVt::direct::(&INNER)), + Aux::Member(Member::new(1, Kind::MsgSingular, 11)), + ], + unknown = unknown, +); + +table_message!(Holder, HOLDER); + +/// `oneof pick { int32 n = 1; Hand hand = 2; }`, with the message boxed and +/// reached through its `Message` impl. +#[derive(Clone, Debug, PartialEq)] +enum ViaPick { + N(i32), + Hand(crate::alloc::boxed::Box), +} + +impl OneofEnum for ViaPick { + fn number(&self) -> u32 { + match self { + Self::N(_) => 1, + Self::Hand(_) => 2, + } + } + + fn payload(&self) -> *const u8 { + match self { + Self::N(v) => (v as *const i32).cast(), + Self::Hand(v) => (&**v as *const Hand).cast(), + } + } + + fn payload_mut(&mut self) -> *mut u8 { + match self { + Self::N(v) => (v as *mut i32).cast(), + Self::Hand(v) => (&mut **v as *mut Hand).cast(), + } + } + + fn with_default(number: u32) -> Option { + Some(match number { + 1 => Self::N(Default::default()), + 2 => Self::Hand(Default::default()), + _ => return None, + }) + } +} + +#[derive(Clone, Debug, Default, PartialEq)] +struct ViaHolder { + pick: Option, + unknown: UnknownFields, +} + +static VIA_HOLDER: Table = crate::__table!( + ViaHolder, + abi = ABI, + entries = [ + crate::__table_entry!( + ViaHolder, + pick, + oneof(Int32Required, true), + 1, + aux = 1, + slot = Option + ), + crate::__table_entry!( + ViaHolder, + pick, + oneof(MsgSingular, false), + 2, + aux = 2, + slot = Option + ), + ], + dense = &dense::<3>(&[1, 2]), + aux = [ + Aux::Group(&OneofVt::with_messages::( + crate::table::offset_of!(ViaHolder, pick), + 1 + )), + Aux::Member(Member::new(0, Kind::Int32Required, 0)), + Aux::Member(Member::new(0, Kind::MsgSingular, 3)), + Aux::Msg(&MsgVt::direct_via_message::()), + ], + unknown = unknown, +); + +table_message!(ViaHolder, VIA_HOLDER); + +fn via_hand(n: i32, s: &str) -> ViaHolder { + ViaHolder { + pick: Some(ViaPick::Hand(crate::alloc::boxed::Box::new(Hand { + n, + s: s.into(), + ..Hand::default() + }))), + ..ViaHolder::default() + } +} + +#[test] +fn a_oneof_member_reached_through_its_message_impl_round_trips_into_every_sink() { + for msg in [ + ViaHolder::default(), + ViaHolder { + pick: Some(ViaPick::N(-3)), + ..ViaHolder::default() + }, + via_hand(0, ""), + via_hand(4, "hand"), + ] { + let wire = msg.encode_to_vec(); + assert_eq!(wire.len() as u32, msg.encoded_len()); + assert_eq!(ViaHolder::decode_from_slice(&wire).unwrap(), msg); + // A rope is not written through the cursor, so the child is staged. + let mut rope = Rope::new(); + msg.encode(&mut rope); + assert_eq!(&rope.to_contiguous_bytes()[..], &wire[..]); + } +} + +#[test] +fn a_oneof_member_reached_through_its_message_impl_merges_in_place() { + // hand { n = 1 }, then hand { s = "x" }: one message with both. + let wire = [0x12, 0x02, 0x08, 0x01, 0x12, 0x03, 0x12, 0x01, b'x']; + assert_eq!(ViaHolder::decode_from_slice(&wire).unwrap(), via_hand(1, "x")); + // Another member replaces it, and then a message starts from a default. + let wire = [0x12, 0x02, 0x08, 0x01, 0x08, 0x05, 0x12, 0x03, 0x12, 0x01, b'y']; + assert_eq!(ViaHolder::decode_from_slice(&wire).unwrap(), via_hand(0, "y")); +} + +#[test] +fn a_oneof_member_reached_through_its_message_impl_that_fails_leaves_the_member_alone() { + let mut msg = via_hand(1, "keep"); + let bad = [0x12, 0x03, 0x12, 0x05, b'x']; + assert!(msg.merge_from_slice(&bad).is_err()); + assert_eq!(msg, via_hand(1, "keep")); + // Where another member was set, the discarded default is not left behind. + let mut msg = ViaHolder { + pick: Some(ViaPick::N(9)), + ..ViaHolder::default() + }; + assert!(msg.merge_from_slice(&bad).is_err()); + assert_eq!(msg.pick, Some(ViaPick::N(9))); +} + +fn child(id: i32) -> Pick { + Pick::Child(crate::alloc::boxed::Box::new(Inner { + id, + ..Inner::default() + })) +} + +fn holder(pick: Pick) -> Holder { + Holder { + pick: Some(pick), + ..Holder::default() + } +} + +#[test] +fn a_oneof_is_written_where_its_lowest_member_would_be() { + // a = 1; the child (9) of `pick`, whose lowest member is 2, comes before b + // = 7 (3), not after it. + let msg = Holder { + a: 1, + pick: Some(child(5)), + b: 7, + ..Holder::default() + }; + let wire = [0x08, 0x01, 0x4a, 0x02, 0x08, 0x05, 0x18, 0x07]; + assert_eq!(msg.encode_to_vec(), wire); + assert_eq!(msg.encoded_len() as usize, wire.len()); + assert_eq!(Holder::decode_from_slice(&wire).unwrap(), msg); + + // The other oneof is written at 6, after b, when it is the one that is set. + let msg = Holder { + b: 7, + other: Some(Alt::X(1.0)), + ..Holder::default() + }; + assert_eq!( + msg.encode_to_vec(), + [0x18, 0x07, 0x35, 0x00, 0x00, 0x80, 0x3f] + ); +} + +#[test] +fn every_member_round_trips_and_a_default_value_is_still_written() { + let picks = [ + Pick::Num(0), + Pick::Num(-1), + Pick::Text(String::new()), + Pick::Text("héllo".into()), + Pick::Blob(Vec::new()), + Pick::Blob(vec![1, 2, 3]), + Pick::Open(EnumValue::from(0)), + Pick::Open(EnumValue::from(9)), + Pick::Strict(Color::Red), + Pick::Strict(Color::Blue), + Pick::Child(Default::default()), + child(3), + ]; + for pick in picks { + let msg = holder(pick); + let wire = msg.encode_to_vec(); + assert!(!wire.is_empty(), "{msg:?} wrote nothing"); + assert_eq!(wire.len() as u32, msg.encoded_len()); + assert_eq!(Holder::decode_from_slice(&wire).unwrap(), msg); + } + for other in [ + Alt::X(0.0), + Alt::X(-2.5), + Alt::Y(Inner::default()), + Alt::Y(Inner { + id: 4, + label: "y".into(), + ..Inner::default() + }), + ] { + let msg = Holder { + other: Some(other), + ..Holder::default() + }; + let wire = msg.encode_to_vec(); + assert!(!wire.is_empty()); + assert_eq!(Holder::decode_from_slice(&wire).unwrap(), msg); + } + assert!(Holder::default().encode_to_vec().is_empty()); + assert_eq!(holder(Pick::Num(0)).encode_to_vec(), [0x10, 0x00]); +} + +#[test] +fn the_last_member_on_the_wire_wins() { + // num = 5, then text = "hi". + let wire = [0x10, 0x05, 0x22, 0x02, b'h', b'i']; + assert_eq!( + Holder::decode_from_slice(&wire).unwrap().pick, + Some(Pick::Text("hi".into())) + ); + // text, then num, then num again. + let wire = [0x22, 0x02, b'h', b'i', 0x10, 0x05, 0x10, 0x06]; + assert_eq!( + Holder::decode_from_slice(&wire).unwrap().pick, + Some(Pick::Num(6)) + ); + // The two oneofs are independent. + let wire = [0x10, 0x05, 0x35, 0x00, 0x00, 0x00, 0x40]; + let msg = Holder::decode_from_slice(&wire).unwrap(); + assert_eq!(msg.pick, Some(Pick::Num(5))); + assert_eq!(msg.other, Some(Alt::X(2.0))); +} + +#[test] +fn a_message_member_that_is_set_is_merged_into_and_a_different_one_replaces_it() { + // child {id = 5}, child {label = "ab"}: merged. + let wire = [0x4a, 0x02, 0x08, 0x05, 0x4a, 0x04, 0x12, 0x02, b'a', b'b']; + let merged = Holder::decode_from_slice(&wire).unwrap(); + assert_eq!( + merged.pick, + Some(Pick::Child(crate::alloc::boxed::Box::new(Inner { + id: 5, + label: "ab".into(), + ..Inner::default() + }))) + ); + // Then num = 7 replaces the child, and a new child starts from nothing. + let mut wire = wire.to_vec(); + wire.extend_from_slice(&[0x10, 0x07]); + assert_eq!( + Holder::decode_from_slice(&wire).unwrap().pick, + Some(Pick::Num(7)) + ); + wire.extend_from_slice(&[0x4a, 0x02, 0x08, 0x01]); + assert_eq!(Holder::decode_from_slice(&wire).unwrap().pick, Some(child(1))); + + // The same for the inline message of the other oneof. + let wire = [0x52, 0x02, 0x08, 0x05, 0x52, 0x04, 0x12, 0x02, b'a', b'b']; + assert_eq!( + Holder::decode_from_slice(&wire).unwrap().other, + Some(Alt::Y(Inner { + id: 5, + label: "ab".into(), + ..Inner::default() + })) + ); +} + +#[test] +fn merging_into_a_message_keeps_a_member_that_the_input_does_not_set() { + let mut msg = holder(Pick::Text("keep".into())); + msg.merge_from_slice(&[0x08, 0x02]).unwrap(); + assert_eq!(msg.a, 2); + assert_eq!(msg.pick, Some(Pick::Text("keep".into()))); +} + +#[test] +fn a_closed_enum_value_the_member_does_not_know_leaves_the_oneof_alone() { + // text = "hi", then strict (8) = 7, which Color has no variant for. + let wire = [0x22, 0x02, b'h', b'i', 0x40, 0x07]; + let msg = Holder::decode_from_slice(&wire).unwrap(); + assert_eq!(msg.pick, Some(Pick::Text("hi".into()))); + let unknown: Vec<_> = msg.unknown.iter().map(|u| u.number).collect(); + assert_eq!(unknown, [8]); + assert!(matches!( + msg.unknown.iter().next().unwrap().data, + UnknownFieldData::Varint(7) + )); + // Nothing was set before, and nothing is now. + assert_eq!(Holder::decode_from_slice(&[0x40, 0x07]).unwrap().pick, None); + // A known value replaces the member, and an open enum keeps any value. + assert_eq!( + Holder::decode_from_slice(&[0x22, 0x00, 0x40, 0x02]) + .unwrap() + .pick, + Some(Pick::Strict(Color::Blue)) + ); + assert_eq!( + Holder::decode_from_slice(&[0x22, 0x00, 0x38, 0x09]) + .unwrap() + .pick, + Some(Pick::Open(EnumValue::from(9))) + ); +} + +#[test] +fn a_rejected_member_value_leaves_the_oneof_as_it_was() { + // For every member: a value that is cut short, a string that is not UTF-8, + // and a wire type the member does not have. Each fails without touching + // the member that is set, whichever it is, or setting one. + let failing: [&[u8]; 9] = [ + &[0x10, 0x80], + &[0x12, 0x00], + &[0x22, 0x01, 0xff], + &[0x22, 0x05, b'a'], + &[0x2a, 0x05, 0x01], + &[0x38, 0x80], + &[0x40, 0x80], + &[0x4a, 0x05, 0x08], + &[0x4a], + ]; + let initial: [fn() -> Option; 3] = [ + || Some(Pick::Num(5)), + || Some(Pick::Text("abc".into())), + || None, + ]; + for initial in initial { + for wire in failing { + let mut msg = Holder { + pick: initial(), + ..Holder::default() + }; + assert!(msg.merge_from_slice(wire).is_err(), "{wire:02x?}"); + assert_eq!(msg.pick, initial(), "{wire:02x?}"); + } + } + // The float member of the other oneof, in the same way. + for wire in [&[0x35, 0x00][..], &[0x32, 0x00][..]] { + let mut msg = Holder { + other: Some(Alt::Y(Inner::default())), + ..Holder::default() + }; + assert!(msg.merge_from_slice(wire).is_err(), "{wire:02x?}"); + assert_eq!(msg.other, Some(Alt::Y(Inner::default())), "{wire:02x?}"); + } + assert!(matches!( + Holder::decode_from_slice(&[0x22, 0x01, 0xff]), + Err(DecodeError::InvalidUtf8) + )); +} + +#[test] +fn a_member_with_the_wrong_wire_type_is_an_error() { + // num (varint) sent as length-delimited; text sent as a varint; a message + // sent as a fixed32. + for wire in [[0x12, 0x00], [0x20, 0x01], [0x4d, 0x00]] { + assert!(matches!( + Holder::decode_from_slice(&wire), + Err(DecodeError::WireTypeMismatch { .. }) + )); + } +} + +#[test] +fn a_message_member_that_is_cut_short_is_an_error() { + let wire = holder(child(5)).encode_to_vec(); + for end in 0..wire.len() { + // A prefix ends the message at a field boundary or is an error, and + // never yields a member the input did not hold. + if let Ok(msg) = Holder::decode_from_slice(&wire[..end]) { + assert_eq!(msg, Holder::default(), "prefix of {end}"); + } + } +} + +#[test] +fn a_message_member_that_fails_leaves_a_different_member_as_it_was() { + // The child (9) has no length, is longer than the input, or holds a field + // that does not decode; the oneof keeps the text. + for wire in [ + &[0x4a][..], + &[0x4a, 0x05, 0x08][..], + &[0x4a, 0x02, 0x08, 0x80][..], + &[0x4a, 0x03, 0x08, 0x05, 0x10][..], + ] { + let mut msg = holder(Pick::Text("abc".into())); + assert!(msg.merge_from_slice(wire).is_err(), "{wire:02x?}"); + assert_eq!(msg.pick, Some(Pick::Text("abc".into())), "{wire:02x?}"); + let mut none = Holder::default(); + assert!(none.merge_from_slice(wire).is_err(), "{wire:02x?}"); + assert_eq!(none.pick, None, "{wire:02x?}"); + } +} + +#[test] +fn a_message_member_that_fails_keeps_what_it_merged_into_the_member_that_is_set() { + // id = 5 merges into the child, and then the label has the wrong wire + // type, as it would for a singular message field. + let mut msg = holder(child(1)); + assert!(msg + .merge_from_slice(&[0x4a, 0x03, 0x08, 0x05, 0x10]) + .is_err()); + assert_eq!(msg.pick, Some(child(5))); +} + +#[test] +fn a_message_member_past_the_recursion_limit_is_an_error_that_changes_nothing() { + let mut msg = holder(Pick::Text("abc".into())); + let wire = [0x4a, 0x02, 0x08, 0x05]; + let err = crate::DecodeOptions::new() + .with_recursion_limit(0) + .merge_from_slice(&mut msg, &wire) + .unwrap_err(); + assert_eq!(err, DecodeError::RecursionLimitExceeded); + assert_eq!(msg.pick, Some(Pick::Text("abc".into()))); +} + +#[test] +fn a_message_member_larger_than_the_limit_is_an_error_that_changes_nothing() { + let mut msg = holder(Pick::Text("abc".into())); + // A length of 2 GiB. + let wire = [0x4a, 0x80, 0x80, 0x80, 0x80, 0x08]; + assert_eq!( + msg.merge_from_slice(&wire), + Err(DecodeError::MessageTooLarge) + ); + assert_eq!(msg.pick, Some(Pick::Text("abc".into()))); +} + +#[test] +fn oneof_members_decode_from_a_buffer_of_two_chunks() { + let msg = Holder { + a: 1, + pick: Some(Pick::Text("split".into())), + b: 2, + other: Some(Alt::Y(Inner { + id: 3, + label: "in".into(), + ..Inner::default() + })), + ..Holder::default() + }; + let wire = msg.encode_to_vec(); + for split in 0..=wire.len() { + let (head, tail) = wire.split_at(split); + let mut chained = head.chain(tail); + let mut decoded = Holder::default(); + with_ctx(|ctx| decoded.merge(&mut chained, ctx)).unwrap(); + assert_eq!(decoded, msg, "split at {split}"); + } +} + +#[test] +fn oneof_members_encode_into_every_sink() { + let msg = Holder { + a: 1, + pick: Some(Pick::Blob(vec![0xcd; 300])), + other: Some(Alt::Y(Inner { + id: 3, + ..Inner::default() + })), + ..Holder::default() + }; + let expected = msg.encode_to_vec(); + let mut rope = Rope::new(); + msg.encode(&mut rope); + assert_eq!(&rope.to_contiguous_bytes()[..], &expected[..]); + let mut bytes_mut = crate::bytes::BytesMut::new(); + msg.encode(&mut bytes_mut); + assert_eq!(&bytes_mut[..], &expected[..]); +} + +#[test] +fn find_reaches_every_oneof_member() { + for (number, kind) in [(2, Kind::OneofLeader), (9, Kind::OneofFollower)] { + assert_eq!(HOLDER.raw.find(number).unwrap().kind, kind); + } + let pe = HOLDER.raw.payload_entry(0, 4); + assert_eq!(pe.kind, Kind::StrRequired); + assert_eq!(pe.number(), 4); + let pe = HOLDER.raw.payload_entry(0, 9); + assert_eq!(pe.kind, Kind::MsgSingular); + assert_eq!(pe.aux, 11); +} + +#[test] +#[should_panic(expected = "does not match the oneof enum")] +fn a_member_of_another_oneof_is_not_a_member_of_this_one() { + // 6 is a member of `other`, which is group 1. + let _ = HOLDER.raw.payload_entry(0, 6); +} + +#[test] +#[should_panic(expected = "does not match the oneof enum")] +fn a_field_that_is_not_in_a_oneof_is_not_a_member() { + let _ = HOLDER.raw.payload_entry(0, 1); +} + +#[test] +fn the_accessors_keep_a_member_that_is_set_and_replace_any_other() { + let vt = OneofVt::with_messages::(0, 2); + let mut slot = Some(Pick::Text("kept".into())); + let slot_ptr = (&mut slot as *mut Option).cast::(); + // SAFETY: `slot` is a live `Option` and 4 and 2 are members of it. + unsafe { + let (number, payload) = (vt.get)(slot_ptr); + assert_eq!(number, 4); + assert_eq!(*payload.cast::(), "kept"); + let payload = (vt.place)(slot_ptr, 4); + assert_eq!(*payload.cast::(), "kept"); + let payload = (vt.place)(slot_ptr, 2); + assert_eq!(*payload.cast::(), 0); + *payload.cast::() = 8; + } + assert_eq!(slot, Some(Pick::Num(8))); + let mut none: Option = None; + // SAFETY: as above. + let (number, payload) = unsafe { (vt.get)((&mut none as *mut Option).cast()) }; + assert_eq!((number, payload.is_null()), (0, true)); +} + +#[test] +fn place_with_merges_into_the_member_that_is_set_and_replaces_another_only_on_success() { + let vt = OneofVt::with_messages::(0, 2); + // The pointer is derived from `slot` again for each call, because reading + // `slot` in between ends the use of an earlier one. + fn raw(slot: &mut Option) -> *mut u8 { + (slot as *mut Option).cast() + } + let mut slot = Some(Pick::Text("kept".into())); + // SAFETY: `slot` is a live `Option` and 4, 2 and 5 are members. + unsafe { + // The member that is set is decoded into where it is, and stays set + // when the decoding fails part of the way. + let r = (vt.place_with)(raw(&mut slot), 4, &mut |p| { + p.cast::().as_mut().unwrap().push_str("+more"); + Err(DecodeError::UnexpectedEof) + }); + assert_eq!(r, Err(DecodeError::UnexpectedEof)); + assert_eq!(slot, Some(Pick::Text("kept+more".into()))); + // Another member is decoded into a new default one, which a failure + // discards. + let r = (vt.place_with)(raw(&mut slot), 2, &mut |p| { + assert_eq!(*p.cast::(), 0); + *p.cast::() = 9; + Err(DecodeError::UnexpectedEof) + }); + assert_eq!(r, Err(DecodeError::UnexpectedEof)); + assert_eq!(slot, Some(Pick::Text("kept+more".into()))); + // Success replaces it. + let r = (vt.place_with)(raw(&mut slot), 2, &mut |p| { + *p.cast::() = 9; + Ok(()) + }); + assert_eq!(r, Ok(())); + assert_eq!(slot, Some(Pick::Num(9))); + } + // An oneof that is unset stays unset on failure. + let mut none: Option = None; + // SAFETY: as above. + let r = unsafe { + (vt.place_with)((&mut none as *mut Option).cast(), 5, &mut |_| { + Err(DecodeError::UnexpectedEof) + }) + }; + assert_eq!(r, Err(DecodeError::UnexpectedEof)); + assert_eq!(none, None); +} + +#[test] +#[should_panic(expected = "does not match the oneof enum")] +fn place_with_a_member_the_enum_does_not_have_panics() { + let vt = OneofVt::with_messages::(0, 2); + let mut slot: Option = None; + // SAFETY: `slot` is a live `Option`; 99 is not a member, which the + // accessor reports by panicking. + let _ = unsafe { (vt.place_with)((&mut slot as *mut Option).cast(), 99, &mut |_| Ok(())) }; +} + +/// An enum whose `with_default` returns a member other than the one asked +/// for, which breaks the contract of [`OneofEnum`]. +#[derive(Debug, PartialEq)] +struct LyingEnum; + +impl OneofEnum for LyingEnum { + fn number(&self) -> u32 { + 1 + } + fn payload(&self) -> *const u8 { + core::ptr::null() + } + fn payload_mut(&mut self) -> *mut u8 { + core::ptr::null_mut() + } + fn with_default(_number: u32) -> Option { + Some(LyingEnum) + } +} + +/// An enum whose payload pointers are null, which breaks the contract of +/// [`OneofEnum`]. +#[derive(Debug, PartialEq)] +struct NullPayload; + +impl OneofEnum for NullPayload { + fn number(&self) -> u32 { + 1 + } + fn payload(&self) -> *const u8 { + core::ptr::null() + } + fn payload_mut(&mut self) -> *mut u8 { + core::ptr::null_mut() + } + fn with_default(number: u32) -> Option { + (number == 1).then_some(NullPayload) + } +} + +#[test] +#[cfg(debug_assertions)] +#[should_panic(expected = "`OneofEnum::payload` returned null")] +fn a_null_payload_is_caught_when_the_member_is_read() { + let vt = OneofVt::new::(0, 1); + let mut slot = Some(NullPayload); + // SAFETY: `slot` is a live `Option`. + let _ = unsafe { (vt.get)((&mut slot as *mut Option).cast()) }; +} + +#[test] +#[cfg(debug_assertions)] +#[should_panic(expected = "`OneofEnum::payload_mut` returned null")] +fn a_null_payload_is_caught_when_the_member_is_placed() { + let vt = OneofVt::new::(0, 1); + let mut slot: Option = None; + // SAFETY: `slot` is a live `Option` and 1 is a member. + let _ = unsafe { (vt.place)((&mut slot as *mut Option).cast(), 1) }; +} + +#[test] +#[cfg(debug_assertions)] +#[should_panic(expected = "`OneofEnum::payload_mut` returned null")] +fn a_null_payload_is_caught_when_the_member_is_decoded_into() { + let vt = OneofVt::with_messages::(0, 1); + let mut slot: Option = None; + // SAFETY: as above. + let _ = unsafe { + (vt.place_with)((&mut slot as *mut Option).cast(), 1, &mut |_| Ok(())) + }; +} + +#[test] +#[should_panic(expected = "does not match the oneof enum")] +fn placing_a_member_whose_default_has_another_number_panics() { + let vt = OneofVt::new::(0, 1); + let mut slot: Option = None; + // SAFETY: `slot` is a live `Option`; the accessor checks the + // number of what `with_default` returns. + unsafe { (vt.place)((&mut slot as *mut Option).cast(), 2) }; +} + +#[test] +#[should_panic(expected = "does not match the oneof enum")] +fn place_with_a_member_whose_default_has_another_number_panics() { + let vt = OneofVt::with_messages::(0, 1); + let mut slot: Option = None; + // SAFETY: as above. + let _ = unsafe { (vt.place_with)((&mut slot as *mut Option).cast(), 2, &mut |_| Ok(())) }; +} + +#[test] +#[should_panic(expected = "does not match the oneof enum")] +fn placing_a_member_the_enum_does_not_have_panics() { + let vt = OneofVt::with_messages::(0, 2); + let mut slot: Option = None; + // SAFETY: `slot` is a live `Option`; 99 is not a member, which the + // accessor reports by panicking. + unsafe { (vt.place)((&mut slot as *mut Option).cast(), 99) }; +} + +/// Building a table that violates the checks on oneofs is a compile error in +/// a `static`, so these run the checks at run time. +#[test] +#[should_panic(expected = "must not be a `MsgVt::direct` one")] +fn an_ordinary_message_field_cannot_use_a_direct_descriptor() { + static AUX: [Aux; 1] = [Aux::Msg(&MsgVt::direct::(&INNER))]; + const E: Entry = Entry::new(Kind::MsgSingular, 1, 0, 0); + // SAFETY: the table is dropped without being used. + let _ = unsafe { Table::::new(ABI, &[E], &[], &AUX, None) }; +} + +mod invalid_oneof_tables { + use super::*; + + static PICK: OneofVt = OneofVt::with_messages::(0, 2); + static INT_MEMBERS: [Aux; 2] = [ + Aux::Group(&PICK), + Aux::Member(Member::new(0, Kind::Int32Required, 0)), + ]; + + /// `Table::new` on a `Holder`, which is never used to access a message. + fn holder_table(entries: &'static [Entry], aux: &'static [Aux]) -> Table { + // SAFETY: the table is dropped without being used. + unsafe { Table::new(ABI, entries, &[], aux, None) } + } + + /// A member of `pick` (whose lowest number is 2), at its offset, leading + /// it if it is number 2. + const fn member(kind: Kind, number: u32, aux: u16) -> Entry { + Entry::oneof_member(kind, number == 2, number, 0, aux) + } + + #[test] + fn a_valid_table_is_accepted() { + const E: Entry = member(Kind::Int32Required, 2, 1); + let _ = holder_table(&[E], &INT_MEMBERS); + } + + #[test] + #[should_panic(expected = "payload must be a `Required`")] + fn a_payload_must_be_a_kind_a_member_can_have() { + let _ = Entry::oneof_member(Kind::Int32Optional, true, 2, 0, 1); + } + + #[test] + #[should_panic(expected = "wire type is its payload kind's")] + fn an_entry_of_the_member_kind_needs_the_constructor_for_members() { + let _ = Entry::new(Kind::OneofFollower, 2, 0, 1); + } + + #[test] + #[should_panic(expected = "wrong variant")] + fn a_member_entry_needs_a_member_aux() { + const E: Entry = member(Kind::Int32Required, 2, 0); + let _ = holder_table(&[E], &INT_MEMBERS); + } + + #[test] + #[should_panic(expected = "group index is out of range")] + fn a_member_needs_its_group() { + static AUX: [Aux; 1] = [Aux::Member(Member::new(3, Kind::Int32Required, 0))]; + const E: Entry = member(Kind::Int32Required, 2, 0); + let _ = holder_table(&[E], &AUX); + } + + #[test] + #[should_panic(expected = "not a oneof descriptor")] + fn a_members_group_must_be_a_group() { + static AUX: [Aux; 1] = [Aux::Member(Member::new(0, Kind::Int32Required, 0))]; + const E: Entry = member(Kind::Int32Required, 2, 0); + let _ = holder_table(&[E], &AUX); + } + + #[test] + #[should_panic(expected = "payload kind is not one a payload can have")] + fn a_members_kind_must_be_a_payload_kind() { + static AUX: [Aux; 2] = [ + Aux::Group(&PICK), + Aux::Member(Member::new(0, Kind::Int32Optional, 0)), + ]; + const E: Entry = member(Kind::Int32Required, 2, 1); + let _ = holder_table(&[E], &AUX); + } + + #[test] + #[should_panic(expected = "wire type")] + fn a_members_tag_must_have_its_kinds_wire_type() { + static AUX: [Aux; 2] = [ + Aux::Group(&PICK), + Aux::Member(Member::new(0, Kind::FloatRequired, 0)), + ]; + const E: Entry = member(Kind::Int32Required, 2, 1); + let _ = holder_table(&[E], &AUX); + } + + #[test] + #[should_panic(expected = "payload aux index is out of range")] + fn a_message_member_needs_its_payload_descriptor() { + static AUX: [Aux; 2] = [ + Aux::Group(&PICK), + Aux::Member(Member::new(0, Kind::MsgSingular, 5)), + ]; + const E: Entry = member(Kind::MsgSingular, 2, 1); + let _ = holder_table(&[E], &AUX); + } + + #[test] + #[should_panic(expected = "wrong variant for its kind")] + fn a_members_payload_descriptor_must_be_of_its_kinds_variant() { + static AUX: [Aux; 3] = [ + Aux::Group(&PICK), + Aux::Member(Member::new(0, Kind::MsgSingular, 2)), + Aux::Enum(&EnumVt::new::>()), + ]; + const E: Entry = member(Kind::MsgSingular, 2, 1); + let _ = holder_table(&[E], &AUX); + } + + #[test] + #[should_panic(expected = "`MsgVt::direct_via_message` one")] + fn a_message_member_needs_a_descriptor_that_reaches_the_message_directly() { + static AUX: [Aux; 3] = [ + Aux::Group(&PICK), + Aux::Member(Member::new(0, Kind::MsgSingular, 2)), + Aux::Msg(&MsgVt::new::>(&INNER)), + ]; + const E: Entry = member(Kind::MsgSingular, 2, 1); + let _ = holder_table(&[E], &AUX); + } + + #[test] + #[should_panic(expected = "must be built with `OneofVt::with_messages`")] + fn a_message_member_needs_a_oneof_built_with_messages() { + static NO_MESSAGES: OneofVt = OneofVt::new::(0, 2); + static AUX: [Aux; 3] = [ + Aux::Group(&NO_MESSAGES), + Aux::Member(Member::new(0, Kind::MsgSingular, 2)), + Aux::Msg(&MsgVt::direct::(&INNER)), + ]; + const E: Entry = member(Kind::MsgSingular, 2, 1); + let _ = holder_table(&[E], &AUX); + } + + #[test] + fn a_oneof_built_without_messages_accepts_the_other_members() { + static NO_MESSAGES: OneofVt = OneofVt::new::(0, 2); + static AUX: [Aux; 2] = [ + Aux::Group(&NO_MESSAGES), + Aux::Member(Member::new(0, Kind::Int32Required, 0)), + ]; + const E: Entry = member(Kind::Int32Required, 2, 1); + let _ = holder_table(&[E], &AUX); + } + + #[test] + #[should_panic(expected = "singular field")] + fn a_members_enum_descriptor_must_be_singular() { + static AUX: [Aux; 3] = [ + Aux::Group(&PICK), + Aux::Member(Member::new(0, Kind::EnumRequired, 2)), + Aux::Enum(&EnumVt::new::>()), + ]; + const E: Entry = member(Kind::EnumRequired, 2, 1); + let _ = holder_table(&[E], &AUX); + } + + #[test] + #[should_panic(expected = "offset differs from its oneof's")] + fn a_members_offset_must_be_its_oneofs() { + const E: Entry = Entry::oneof_member(Kind::Int32Required, true, 2, 4, 1); + let _ = holder_table(&[E], &INT_MEMBERS); + } + + #[test] + #[should_panic(expected = "numbered below its oneof's lowest")] + fn a_member_cannot_be_numbered_below_its_oneofs_lowest() { + const E: Entry = member(Kind::Int32Required, 1, 1); + let _ = holder_table(&[E], &INT_MEMBERS); + } + + #[test] + #[should_panic(expected = "leader of a oneof must be exactly")] + fn the_lowest_member_must_lead() { + const E: Entry = Entry::oneof_member(Kind::Int32Required, false, 2, 0, 1); + let _ = holder_table(&[E], &INT_MEMBERS); + } + + #[test] + #[should_panic(expected = "leader of a oneof must be exactly")] + fn no_other_member_may_lead() { + const E: Entry = Entry::oneof_member(Kind::Int32Required, true, 3, 0, 1); + let _ = holder_table(&[E], &INT_MEMBERS); + } + + #[test] + #[should_panic(expected = "exactly one leading member")] + fn every_oneof_needs_a_leader() { + // The only member is numbered above the oneof's lowest, so nothing + // leads. + const E: Entry = member(Kind::Int32Required, 3, 1); + let _ = holder_table(&[E], &INT_MEMBERS); + } + + #[test] + #[should_panic(expected = "exactly one leading member")] + fn a_oneof_without_members_is_rejected() { + static AUX: [Aux; 1] = [Aux::Group(&PICK)]; + let _ = holder_table(&[], &AUX); + } +} diff --git a/docs/guide.md b/docs/guide.md index 794709bb..564b771f 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -1191,7 +1191,7 @@ Buffa uses a two-pass model to avoid the exponential-time size computation that ### Smaller generated code: `codec_strategy` -By default every generated message contains its own size, write, and merge code, specialised to its fields. `CodecStrategy::Table` replaces it with one static table per message and interpreters in `buffa` that every message shares. On the WhatsApp schema (`whatsapp.proto` from `waproto`: 334 top-level messages, 752 with the nested ones, 3,477 fields), built with `Box` message fields, no unknown-field preservation, fat LTO, and `panic = "abort"`, the text section of the binary at `opt-level = "z"` went from 1,670 KB to 1,010 KB (−40%). 43 of its 752 messages (23 of 334 top-level) stay unrolled because they have a `oneof` or a `map`. [#463](https://github.com/anthropics/buffa/issues/463) describes the method. The cost is speed on messages made of many small fields, where encoding takes up to about 3.5 times as long as with the default `CodecStrategy::Unrolled` and decoding up to 1.6 times. Messages dominated by bulk data, such as large strings, bytes, and packed arrays, show no difference. +By default every generated message contains its own size, write, and merge code, specialised to its fields. `CodecStrategy::Table` replaces it with one static table per message and interpreters in `buffa` that every message shares. On the WhatsApp schema (`whatsapp.proto` from `waproto`: 334 top-level messages, 752 with the nested ones, 3,477 fields), built with `Box` message fields, no unknown-field preservation, fat LTO, and `panic = "abort"`, the text section of the binary at `opt-level = "z"` went from 1,669 KB to 961 KB (−42%). 3 of its 752 messages (2 of 334 top-level) stay unrolled because they have a `map`. [#463](https://github.com/anthropics/buffa/issues/463) describes the method. The cost is speed on messages made of many small fields, where encoding takes up to about 3.5 times as long as with the default `CodecStrategy::Unrolled` and decoding up to 1.6 times. Messages dominated by bulk data, such as large strings, bytes, and packed arrays, show no difference. ```rust,ignore // build.rs @@ -1204,7 +1204,7 @@ buffa_build::Config::new() .compile()?; ``` -Apart from the holders listed below, a table message may hold any message. It reaches a child that has a table through it, and any other child (a message you set to `Unrolled`, one generated by another crate, or a well-known type such as `Timestamp`) through its `Message` impl, which costs a function call per child. A `codec_strategy_in` rule selects the message it names and the messages nested in it, and does not extend to the messages it holds. In the example, `.wa.Message` and `.wa.Receipt` stay specialised and the messages that hold them use the table. Setting a message to `Unrolled` therefore does not keep the messages that hold it unrolled: to keep a whole path specialised, set its holders to `Unrolled` too. +Apart from the holders listed below, a table message may hold any message, including as a member of a `oneof`. It reaches a child that has a table through it, and any other child (a message you set to `Unrolled`, one generated by another crate, or a well-known type such as `Timestamp`) through its `Message` impl, which costs a function call per child. A `codec_strategy_in` rule selects the message it names and the messages nested in it, and does not extend to the messages it holds. In the example, `.wa.Message` and `.wa.Receipt` stay specialised and the messages that hold them use the table. Setting a message to `Unrolled` therefore does not keep the messages that hold it unrolled: to keep a whole path specialised, set its holders to `Unrolled` too. The option changes only the binary `Message` implementation. The wire format and the JSON, text, view, and reflection code are the same under both strategies, and a table message encodes to the same bytes and decodes the same accepted input as its unrolled twin. It differs in these ways: @@ -1216,7 +1216,7 @@ The option changes only the binary `Message` implementation. The wire format and These stay unrolled, whatever the setting: -- a message with a `oneof`, a `map`, or a group field; +- a message with a `map` or a group field; - 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;