diff --git a/.changes/unreleased/breaking-changes-20260627-002930.yaml b/.changes/unreleased/breaking-changes-20260627-002930.yaml new file mode 100644 index 00000000..a0a2e54c --- /dev/null +++ b/.changes/unreleased/breaking-changes-20260627-002930.yaml @@ -0,0 +1,8 @@ +kind: Breaking changes +body: |- + Singular message fields are now stored inline by default: codegen emits `MessageField>` (laid out as `Option`, no per-field heap allocation) for every non-recursive field. Recursive fields are detected and stay on `Box` automatically. (#248) + + To restore the old behaviour for specific fields (e.g. large or rarely-set submessages), use `box_type_in(PointerRepr::Box, &[".pkg.Msg.field"])`; for the old global default, `box_type(PointerRepr::Box)`. Explicit `MessageField` type annotations now mean the boxed form and will mismatch the new default — drop the annotation and let `P` infer from the field's declared type (or, for a standalone value with no pinning context, write `MessageField::>::some(x)`). + + `box_type_in` / `box_type_custom_in` now normalize a missing leading dot on each path; previously a dotless path silently matched nothing. +time: 2026-06-27T00:29:30.000000000Z diff --git a/DESIGN.md b/DESIGN.md index 06cdc05f..c2848049 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -342,7 +342,7 @@ if msg.address.is_set() { ... } msg.address.get_or_insert_default().street = "123 Main St".into(); ``` -`MessageField` is heap-allocated (`Option>` internally) so the struct size stays small, but the Deref impl provides transparent read access through a lazily-initialized `&'static T` default singleton. +`MessageField` stores the message inline by default (`Option>` ≡ `Option` — no per-field heap allocation), with recursive fields and explicit `box_type_in(PointerRepr::Box, …)` opt-outs falling back to `Option>`. The Deref impl provides transparent read access through a lazily-initialized `&'static T` default singleton. ### 4. EnumValue\ — Type-Safe Open Enums diff --git a/benchmarks/buffa/Cargo.toml b/benchmarks/buffa/Cargo.toml index c6cf6e00..4344d35a 100644 --- a/benchmarks/buffa/Cargo.toml +++ b/benchmarks/buffa/Cargo.toml @@ -23,13 +23,14 @@ buffa-build = { path = "../../buffa-build" } criterion = "0.5" [features] -default = ["api_response", "log_record", "analytics_event", "google_message1", "media_frame", "packed_tile", "reflect", "lazy"] +default = ["api_response", "log_record", "analytics_event", "google_message1", "media_frame", "packed_tile", "mesh", "reflect", "lazy"] api_response = [] log_record = [] analytics_event = [] google_message1 = [] media_frame = [] packed_tile = [] +mesh = [] reflect = [] lazy = [] # `iso` gates the per-message isolated targets out of the default `task bench` run. @@ -38,12 +39,12 @@ iso = [] [[bench]] name = "protobuf" harness = false -required-features = ["api_response", "log_record", "analytics_event", "google_message1", "media_frame", "packed_tile", "lazy"] +required-features = ["api_response", "log_record", "analytics_event", "google_message1", "media_frame", "packed_tile", "mesh", "lazy"] [[bench]] name = "reflect" harness = false -required-features = ["api_response", "log_record", "analytics_event", "google_message1", "media_frame", "packed_tile", "reflect", "lazy"] +required-features = ["api_response", "log_record", "analytics_event", "google_message1", "media_frame", "packed_tile", "mesh", "reflect", "lazy"] [[bench]] name = "api_response" @@ -75,6 +76,11 @@ name = "packed_tile" harness = false required-features = ["iso", "packed_tile"] +[[bench]] +name = "mesh" +harness = false +required-features = ["iso", "mesh"] + # Build at the release profile (lto, single codegen unit) for a fair cross-impl # comparison — the same profile a consumer building in release gets, and the one # the per-release benchmark history pins. These crates are excluded from the root diff --git a/benchmarks/buffa/benches/analytics_event.rs b/benchmarks/buffa/benches/analytics_event.rs index 1a3df01b..4993ed6c 100644 --- a/benchmarks/buffa/benches/analytics_event.rs +++ b/benchmarks/buffa/benches/analytics_event.rs @@ -7,6 +7,7 @@ feature = "log_record", feature = "media_frame", feature = "packed_tile", + feature = "mesh", feature = "google_message1", feature = "reflect", feature = "lazy" diff --git a/benchmarks/buffa/benches/api_response.rs b/benchmarks/buffa/benches/api_response.rs index d30eecb2..aa018d8a 100644 --- a/benchmarks/buffa/benches/api_response.rs +++ b/benchmarks/buffa/benches/api_response.rs @@ -7,6 +7,7 @@ feature = "analytics_event", feature = "media_frame", feature = "packed_tile", + feature = "mesh", feature = "google_message1", feature = "reflect", feature = "lazy" diff --git a/benchmarks/buffa/benches/google_message1.rs b/benchmarks/buffa/benches/google_message1.rs index d14abc55..82f7d8f3 100644 --- a/benchmarks/buffa/benches/google_message1.rs +++ b/benchmarks/buffa/benches/google_message1.rs @@ -8,6 +8,7 @@ feature = "analytics_event", feature = "media_frame", feature = "packed_tile", + feature = "mesh", feature = "reflect", feature = "lazy" ))] diff --git a/benchmarks/buffa/benches/log_record.rs b/benchmarks/buffa/benches/log_record.rs index 6e137b49..31539d77 100644 --- a/benchmarks/buffa/benches/log_record.rs +++ b/benchmarks/buffa/benches/log_record.rs @@ -7,6 +7,7 @@ feature = "analytics_event", feature = "media_frame", feature = "packed_tile", + feature = "mesh", feature = "google_message1", feature = "reflect", feature = "lazy" diff --git a/benchmarks/buffa/benches/media_frame.rs b/benchmarks/buffa/benches/media_frame.rs index 650593fb..6b3c0b01 100644 --- a/benchmarks/buffa/benches/media_frame.rs +++ b/benchmarks/buffa/benches/media_frame.rs @@ -7,6 +7,7 @@ feature = "log_record", feature = "analytics_event", feature = "packed_tile", + feature = "mesh", feature = "google_message1", feature = "reflect", feature = "lazy" diff --git a/benchmarks/buffa/benches/mesh.rs b/benchmarks/buffa/benches/mesh.rs new file mode 100644 index 00000000..dc9c2a87 --- /dev/null +++ b/benchmarks/buffa/benches/mesh.rs @@ -0,0 +1,37 @@ +// Isolated benchmark for the `mesh` message: only its own decoder is compiled. +// Run with `--no-default-features --features iso,mesh` (or `task bench-iso -- mesh`). +// Guard: isolation is lost if another message (or reflect/lazy) is compiled in, +// which happens if you forget --no-default-features (the default set enables all). +#[cfg(any( + feature = "api_response", + feature = "log_record", + feature = "analytics_event", + feature = "media_frame", + feature = "packed_tile", + feature = "google_message1", + feature = "reflect", + feature = "lazy" +))] +compile_error!("isolated `mesh` bench requires --no-default-features: another message/reflect/lazy feature is enabled, which defeats per-message isolation"); +include!("common.rs"); +use bench_buffa::bench::{__buffa::view::TriMeshView, TriMesh}; + +fn run(c: &mut Criterion) { + let data = include_bytes!("../../datasets/mesh.pb"); + benchmark_decode::(c, "buffa/mesh", data); + benchmark_json::(c, "buffa/mesh", data); + let ds = load_dataset(data); + let bytes = total_payload_bytes(&ds); + let mut g = c.benchmark_group("buffa/mesh"); + g.throughput(Throughput::Bytes(bytes)); + g.bench_function("decode_view", |b| { + b.iter(|| { + for p in &ds.payload { + criterion::black_box(TriMeshView::decode_view(p).unwrap()); + } + }) + }); + g.finish(); +} +criterion::criterion_group!(grp, run); +criterion::criterion_main!(grp); diff --git a/benchmarks/buffa/benches/packed_tile.rs b/benchmarks/buffa/benches/packed_tile.rs index 86d31af8..fe162f82 100644 --- a/benchmarks/buffa/benches/packed_tile.rs +++ b/benchmarks/buffa/benches/packed_tile.rs @@ -8,6 +8,7 @@ feature = "analytics_event", feature = "media_frame", feature = "google_message1", + feature = "mesh", feature = "reflect", feature = "lazy" ))] diff --git a/benchmarks/buffa/benches/protobuf.rs b/benchmarks/buffa/benches/protobuf.rs index 6031c725..e96015a4 100644 --- a/benchmarks/buffa/benches/protobuf.rs +++ b/benchmarks/buffa/benches/protobuf.rs @@ -8,7 +8,7 @@ use bench_buffa::bench::__buffa::lazy_view::{ }; use bench_buffa::bench::__buffa::view::{ analytics_event::PropertyView, AnalyticsEventView, ApiResponseView, LogRecordView, - MediaFrameView, PackedSignedView, PackedTileView, + MediaFrameView, PackedSignedView, PackedTileView, TriMeshView, }; use bench_buffa::bench::__buffa::{oneof, view::oneof as view_oneof}; use bench_buffa::bench::*; @@ -664,6 +664,28 @@ fn bench_packed_tile(c: &mut Criterion) { ); } +// Many small singular sub-messages per repeated element: the regression target +// for `PointerRepr::Inline` (issue #248). The owned decode/merge cost is +// dominated by per-submessage allocation under the `Box` default. +fn bench_mesh(c: &mut Criterion) { + benchmark_decode::(c, "buffa/mesh", include_bytes!("../../datasets/mesh.pb")); +} + +fn bench_mesh_view(c: &mut Criterion) { + let dataset = load_dataset(include_bytes!("../../datasets/mesh.pb")); + let bytes = total_payload_bytes(&dataset); + let mut group = c.benchmark_group("buffa/mesh"); + group.throughput(Throughput::Bytes(bytes)); + group.bench_function("decode_view", |b| { + b.iter(|| { + for payload in &dataset.payload { + criterion::black_box(TriMeshView::decode_view(payload).unwrap()); + } + }); + }); + group.finish(); +} + // Control for the packed-varint reservation: every element is a negative // (10-byte) varint, the worst case for the old byte-length reserve. fn bench_packed_signed(c: &mut Criterion) { @@ -740,6 +762,7 @@ criterion_group!( bench_google_message1, bench_media_frame, bench_packed_tile, + bench_mesh, bench_packed_signed, ); @@ -751,6 +774,7 @@ criterion_group!( bench_google_message1_view, bench_media_frame_view, bench_packed_tile_view, + bench_mesh_view, bench_packed_signed_view, bench_api_response_view_encode, bench_log_record_view_encode, diff --git a/benchmarks/buffa/build.rs b/benchmarks/buffa/build.rs index 91b57fe2..dac5ac2b 100644 --- a/benchmarks/buffa/build.rs +++ b/benchmarks/buffa/build.rs @@ -12,6 +12,7 @@ fn main() { ("ANALYTICS_EVENT", "../proto/iso/analytics_event.proto"), ("MEDIA_FRAME", "../proto/iso/media_frame.proto"), ("PACKED_TILE", "../proto/iso/packed_tile.proto"), + ("MESH", "../proto/iso/mesh.proto"), ( "GOOGLE_MESSAGE1", "../proto/benchmark_message1_proto3.proto", diff --git a/benchmarks/buffa/src/lib.rs b/benchmarks/buffa/src/lib.rs index 40b378cd..0083b103 100644 --- a/benchmarks/buffa/src/lib.rs +++ b/benchmarks/buffa/src/lib.rs @@ -10,7 +10,8 @@ feature = "log_record", feature = "analytics_event", feature = "media_frame", - feature = "packed_tile" + feature = "packed_tile", + feature = "mesh" ))] #[allow( clippy::derivable_impls, diff --git a/benchmarks/datasets/mesh.pb b/benchmarks/datasets/mesh.pb new file mode 100644 index 00000000..8e95f2ea Binary files /dev/null and b/benchmarks/datasets/mesh.pb differ diff --git a/benchmarks/gen-datasets/Cargo.lock b/benchmarks/gen-datasets/Cargo.lock index 9d84012b..1e1c55dc 100644 --- a/benchmarks/gen-datasets/Cargo.lock +++ b/benchmarks/gen-datasets/Cargo.lock @@ -14,34 +14,24 @@ version = "2.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "843867be96c8daad0d758b57df9392b6d8d271134fce549de6ce169ff98a92af" -[[package]] -name = "borsh" -version = "1.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cfd1e3f8955a5d7de9fab72fc8373fade9fb8a703968cb200ae3dc6cf08e185a" -dependencies = [ - "bytes", - "cfg_aliases", -] - [[package]] name = "buffa" -version = "0.7.1" +version = "0.8.0" dependencies = [ "bytes", - "compact_str", - "ecow", + "foldhash", "hashbrown 0.15.5", "once_cell", + "rustversion", "serde", "serde_json", - "smol_str", + "smoothutf8", "thiserror", ] [[package]] name = "buffa-build" -version = "0.7.1" +version = "0.8.0" dependencies = [ "buffa", "buffa-codegen", @@ -50,7 +40,7 @@ dependencies = [ [[package]] name = "buffa-codegen" -version = "0.7.1" +version = "0.8.0" dependencies = [ "buffa", "buffa-descriptor", @@ -63,9 +53,10 @@ dependencies = [ [[package]] name = "buffa-descriptor" -version = "0.7.1" +version = "0.8.0" dependencies = [ "buffa", + "rustversion", "serde", "serde_json", ] @@ -76,47 +67,12 @@ version = "1.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1e748733b7cbc798e1434b6ac524f0c1ff2ab456fe201501e6497c8417a4fc33" -[[package]] -name = "castaway" -version = "0.2.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dec551ab6e7578819132c713a93c022a05d60159dc86e7a7050223577484c55a" -dependencies = [ - "rustversion", -] - [[package]] name = "cfg-if" version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" -[[package]] -name = "cfg_aliases" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" - -[[package]] -name = "compact_str" -version = "0.9.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9dfdd1c2274d9aa354115b09dc9a901d6c5576818cdf70d14cae2bdb47df00ab" -dependencies = [ - "castaway", - "cfg-if", - "itoa", - "rustversion", - "ryu", - "static_assertions", -] - -[[package]] -name = "ecow" -version = "0.2.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "78e4f79b296fbaab6ce2e22d52cb4c7f010fe0ebe7a32e34fa25885fd797bd02" - [[package]] name = "equivalent" version = "1.0.2" @@ -351,12 +307,6 @@ version = "1.0.22" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b39cdef0fa800fc44525c84ccb54a029961a8215f9619753635a9c0d2538d46d" -[[package]] -name = "ryu" -version = "1.0.23" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" - [[package]] name = "semver" version = "1.0.27" @@ -407,20 +357,19 @@ dependencies = [ ] [[package]] -name = "smol_str" -version = "0.3.2" +name = "simdutf8" +version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9676b89cd56310a87b93dec47b11af744f34d5fc9f367b829474eec0a891350d" -dependencies = [ - "borsh", - "serde", -] +checksum = "e3a9fe34e3e7a50316060351f37187a3f546bce95496156754b601a5fa71b76e" [[package]] -name = "static_assertions" -version = "1.1.0" +name = "smoothutf8" +version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" +checksum = "36358427d32ecdb1624616deed99eccfef0a167fe5bf40ddb51efe6980bc1ec8" +dependencies = [ + "simdutf8", +] [[package]] name = "syn" diff --git a/benchmarks/gen-datasets/src/main.rs b/benchmarks/gen-datasets/src/main.rs index cbe19e70..4231ac4d 100644 --- a/benchmarks/gen-datasets/src/main.rs +++ b/benchmarks/gen-datasets/src/main.rs @@ -204,6 +204,27 @@ fn gen_packed_tile(rng: &mut impl Rng) -> PackedTile { } } +fn gen_mesh(rng: &mut impl Rng) -> TriMesh { + let vert = |rng: &mut dyn rand::RngCore| tri_mesh::Vertex { + px: rng.random_range(-1.0..1.0), + py: rng.random_range(-1.0..1.0), + pz: rng.random_range(-1.0..1.0), + ..Default::default() + }; + TriMesh { + faces: (0..256) + .map(|_| tri_mesh::Face { + a: buffa::MessageField::some(vert(rng)), + b: buffa::MessageField::some(vert(rng)), + c: buffa::MessageField::some(vert(rng)), + n: buffa::MessageField::some(vert(rng)), + ..Default::default() + }) + .collect(), + ..Default::default() + } +} + fn gen_packed_signed(rng: &mut impl Rng) -> PackedSigned { // All-negative values so every element is a 10-byte varint: the worst case // for the byte-length reservation the count-based reserve replaces. @@ -299,5 +320,12 @@ fn main() { .collect(), ); + write_dataset( + "mesh", + "bench.TriMesh", + output_dir, + (0..NUM_PAYLOADS).map(|_| gen_mesh(&mut rng)).collect(), + ); + println!("Datasets written to {}", output_dir.display()); } diff --git a/benchmarks/proto/bench_messages.proto b/benchmarks/proto/bench_messages.proto index 68d1ff16..29c5110e 100644 --- a/benchmarks/proto/bench_messages.proto +++ b/benchmarks/proto/bench_messages.proto @@ -109,6 +109,28 @@ message PackedTile { repeated uint64 checksums = 2; // one larger packed varint field } +// Many small singular sub-messages per repeated element (issue #248). Each +// `Face` holds four singular `Vertex` fields, so a boxed `MessageField` +// costs four heap allocations per face on decode that an inline pointer +// (`PointerRepr::Inline`, the default) eliminates. None of the other benchmark +// messages has more than one singular message field per payload, so this is the +// regression target for the inline-by-default storage. +// Target payload: ~18KB (256 faces). +message TriMesh { + message Vertex { + float px = 1; + float py = 2; + float pz = 3; + } + message Face { + Vertex a = 1; + Vertex b = 2; + Vertex c = 3; + Vertex n = 4; + } + repeated Face faces = 1; +} + // Control for the packed-varint reservation path's worst case: negative // `int32`/`int64` always encode to 10 bytes per element, so reserving from the // payload byte length over-allocated ~10x. Reserving from the counted varint diff --git a/benchmarks/proto/iso/mesh.proto b/benchmarks/proto/iso/mesh.proto new file mode 100644 index 00000000..20838787 --- /dev/null +++ b/benchmarks/proto/iso/mesh.proto @@ -0,0 +1,17 @@ +syntax = "proto3"; +package bench; + +message TriMesh { + message Vertex { + float px = 1; + float py = 2; + float pz = 3; + } + message Face { + Vertex a = 1; + Vertex b = 2; + Vertex c = 3; + Vertex n = 4; + } + repeated Face faces = 1; +} diff --git a/buffa-build/src/lib.rs b/buffa-build/src/lib.rs index 03533312..f7aa5ce5 100644 --- a/buffa-build/src/lib.rs +++ b/buffa-build/src/lib.rs @@ -1049,9 +1049,15 @@ impl Config { } /// Map the matching message fields to a [`PointerRepr`] other than the - /// default `Box`. Rules are matched with proto-segment-aware prefix logic; - /// the **last** matching rule wins, so add a broad rule first and narrower - /// overrides after. + /// default `Inline`. Rules are matched with proto-segment-aware prefix + /// logic; the **last** matching rule wins, so add a broad rule first and + /// narrower overrides after. A leading dot is added to each path if + /// missing. + /// + /// The default `Inline` is recursion-aware (recursive fields stay on + /// `Box`), so this knob is for opting *out*: `PointerRepr::Box` for large + /// or rarely-set submessages where reserving `size_of::()` in the parent + /// is wasteful, or `PointerRepr::Custom` for a third-party pointer. /// /// Applies to singular (and proto2 optional/required) message fields and to /// **boxed** oneof message/group variants (matched by the variant's path). @@ -1069,15 +1075,25 @@ impl Config { pub fn box_type_in(mut self, repr: PointerRepr, paths: &[impl AsRef]) -> Self { self.codegen_config .pointer_fields - .extend(paths.iter().map(|p| (p.as_ref().to_string(), repr.clone()))); + .extend(paths.iter().map(|p| { + let p = p.as_ref(); + // Normalize to the leading-dot form: matching and the + // exact-path Inline recursion error both depend on it. + let p = if p.starts_with('.') { + p.to_string() + } else { + format!(".{p}") + }; + (p, repr.clone()) + })); self } /// Map every message field (and boxed oneof variant) to the given [`PointerRepr`]. /// Convenience for `.box_type_in(repr, &["."])`. Call before any - /// [`box_type_in`](Self::box_type_in) overrides, since the last matching rule - /// wins. An inline pointer inflates each parent struct, so prefer narrow - /// rules over a blanket default. + /// [`box_type_in`](Self::box_type_in) overrides, since the last matching + /// rule wins. `box_type(PointerRepr::Box)` restores the pre-0.9 boxed + /// default for every singular message field. #[must_use] pub fn box_type(mut self, repr: PointerRepr) -> Self { self.codegen_config @@ -1847,6 +1863,22 @@ mod tests { assert_eq!(names.reflect, "reflection"); } + #[test] + fn box_type_in_normalizes_leading_dot() { + // Without normalization a dotless path would silently match nothing, + // and the exact-path Inline recursion error would never fire for it. + let config = Config::new() + .box_type_in(PointerRepr::Box, &["my.pkg.Msg.inner", ".my.pkg.Other"]) + .codegen_config; + assert_eq!( + config.pointer_fields, + vec![ + (".my.pkg.Msg.inner".to_string(), PointerRepr::Box), + (".my.pkg.Other".to_string(), PointerRepr::Box), + ] + ); + } + #[test] fn unbox_oneof_in_normalizes_leading_dot() { // Without normalization a dotless path would silently match nothing, diff --git a/buffa-codegen/src/context.rs b/buffa-codegen/src/context.rs index c938a689..0fa7452a 100644 --- a/buffa-codegen/src/context.rs +++ b/buffa-codegen/src/context.rs @@ -95,6 +95,12 @@ pub struct CodeGenContext<'a> { /// Built once by [`resolve_unboxed_variants`](crate::oneof::resolve_unboxed_variants); /// never contains recursive variants. See [`oneof_unboxed`](Self::oneof_unboxed). unboxed_oneof_variants: HashSet, + /// Singular message-field paths (leading-dot form) whose resolved + /// [`PointerRepr`](crate::PointerRepr) is `Inline`. Built once by + /// [`resolve_inlined_fields`](crate::oneof::resolve_inlined_fields); never + /// contains recursive fields. [`pointer_repr`](Self::pointer_repr) demotes + /// any raw `Inline` not in this set to `Box`. + inlined_message_fields: HashSet, /// Non-fatal diagnostics accumulated during generation (e.g. an enum whose /// idiomatic CamelCase aliases were suppressed by a naming conflict). /// @@ -248,8 +254,17 @@ impl<'a> CodeGenContext<'a> { let mut enum_closedness = HashMap::new(); let mut comment_map = HashMap::new(); let mut nested_module_names = HashMap::new(); - let unboxed_oneof_variants = - crate::oneof::resolve_unboxed_variants(files, &config.unboxed_oneof_fields); + let msg_index = crate::oneof::message_index(files); + let unboxed_oneof_variants = crate::oneof::resolve_unboxed_variants( + &msg_index, + &config.unboxed_oneof_fields, + &config.pointer_fields, + ); + let inlined_message_fields = crate::oneof::resolve_inlined_fields( + &msg_index, + &config.unboxed_oneof_fields, + &config.pointer_fields, + ); // Pre-pass: collect every package and top-level message name in the // descriptor set so nested-types module deconfliction (issue #135) is @@ -416,6 +431,7 @@ impl<'a> CodeGenContext<'a> { comment_map, nested_module_names, unboxed_oneof_variants, + inlined_message_fields, warnings: std::cell::RefCell::new(Vec::new()), imports: std::cell::RefCell::new(crate::imports::ImportsPhase::Off), } @@ -927,13 +943,25 @@ impl<'a> CodeGenContext<'a> { /// field at the given proto path. Last matching rule wins (proto-segment /// prefix match); fields matching no rule use /// [`PointerRepr::Box`](crate::PointerRepr::Box). + /// + /// `Inline` is recursion-aware: a path whose raw rule resolves to + /// [`PointerRepr::Inline`](crate::PointerRepr::Inline) but is absent from + /// the precomputed `inlined_message_fields` set + /// (a recursive singular field, or a non-singular path such as a oneof + /// variant) is demoted to `Box` so the generated type stays sized. pub fn pointer_repr(&self, field_fqn: &str) -> crate::PointerRepr { - self.config + let raw = self + .config .pointer_fields .iter() .rev() .find(|(prefix, _)| matches_proto_prefix(prefix, field_fqn)) - .map_or(crate::PointerRepr::default(), |(_, repr)| repr.clone()) + .map_or(crate::PointerRepr::default(), |(_, repr)| repr.clone()); + if raw == crate::PointerRepr::Inline && !self.inlined_message_fields.contains(field_fqn) { + crate::PointerRepr::Box + } else { + raw + } } /// Resolve the [`RepeatedRepr`](crate::RepeatedRepr) for a `repeated` field diff --git a/buffa-codegen/src/lib.rs b/buffa-codegen/src/lib.rs index b3473b73..2cc76e4c 100644 --- a/buffa-codegen/src/lib.rs +++ b/buffa-codegen/src/lib.rs @@ -527,11 +527,23 @@ impl MapRepr { #[derive(Debug, Clone, PartialEq, Eq, Default)] #[non_exhaustive] pub enum PointerRepr { - /// `::buffa::alloc::boxed::Box` (inside `MessageField`) — the default. - /// Keeps generated output byte-identical to a build without the knob (the - /// `MessageField` pointer type parameter defaults to `Box`). - #[default] + /// `::buffa::alloc::boxed::Box` (inside `MessageField`). The opt-out + /// from the `Inline` default for large or rarely-set submessages, via + /// `box_type_in(PointerRepr::Box, paths)` (or `box_type(PointerRepr::Box)` + /// to restore the pre-0.9 global default). Box, + /// `::buffa::Inline` — store the message directly in the parent struct, + /// no heap allocation. `MessageField>` is laid out as + /// `Option`. The default. + /// + /// Recursion-aware: a singular field that would form an infinite-size cycle + /// (directly, mutually, or via an + /// [`unbox_oneof`](CodeGenConfig::unboxed_oneof_fields)-inlined oneof + /// variant) is silently kept on `Box`, so the default is always sized. An + /// *exact-path* `Inline` rule that names a recursive field is rejected at + /// codegen time. + #[default] + Inline, /// A custom pointer named by a Rust type-path **template** with a `*` /// placeholder for the message type. Must satisfy `buffa::ProtoBox` and /// be a crate-local newtype. @@ -572,6 +584,7 @@ impl PointerRepr { use quote::quote; match self { PointerRepr::Box => Ok(quote! { #message_field<#inner> }), + PointerRepr::Inline => Ok(quote! { #message_field<#inner, ::buffa::Inline<#inner>> }), PointerRepr::Custom(template) => { let ptr = parse_wildcard_type_path(template, inner)?; Ok(quote! { #message_field<#inner, #ptr> }) @@ -595,6 +608,9 @@ impl PointerRepr { use quote::quote; match self { PointerRepr::Box => Ok(quote! { ::buffa::MessageField::<#inner> }), + PointerRepr::Inline => { + Ok(quote! { ::buffa::MessageField::<#inner, ::buffa::Inline<#inner>> }) + } PointerRepr::Custom(template) => { let ptr = parse_wildcard_type_path(template, inner)?; Ok(quote! { ::buffa::MessageField::<#inner, #ptr> }) @@ -618,6 +634,7 @@ impl PointerRepr { use quote::quote; match self { PointerRepr::Box => Ok(quote! { ::buffa::alloc::boxed::Box<#inner> }), + PointerRepr::Inline => Ok(quote! { ::buffa::Inline<#inner> }), PointerRepr::Custom(template) => parse_wildcard_type_path(template, inner), } } @@ -638,6 +655,7 @@ impl PointerRepr { use quote::quote; match self { PointerRepr::Box => Ok(quote! { ::buffa::alloc::boxed::Box::new(#value) }), + PointerRepr::Inline => Ok(quote! { ::buffa::Inline(#value) }), PointerRepr::Custom(template) => { let ptr = parse_wildcard_type_path(template, inner)?; Ok(quote! { <#ptr as ::buffa::ProtoBox<#inner>>::new(#value) }) diff --git a/buffa-codegen/src/message.rs b/buffa-codegen/src/message.rs index f10016c2..3ae9afed 100644 --- a/buffa-codegen/src/message.rs +++ b/buffa-codegen/src/message.rs @@ -1546,7 +1546,27 @@ fn classify_field( }; // Configurable owned pointer for singular message fields (default `Box`). + // `Inline` is recursion-aware: a recursive field is silently demoted to + // `Box` under a prefix/blanket rule, but an *exact-path* `Inline` rule + // naming a recursive field is a hard error — the user asked for something + // impossible. Mirrors the `unbox_oneof_in` exact-path check in `oneof.rs`. let pointer_repr = ctx.pointer_repr(&field_fqn); + if pointer_repr != crate::PointerRepr::Inline + && ctx + .config + .pointer_fields + .iter() + .rev() + .find(|(prefix, _)| crate::context::matches_proto_prefix(prefix, &field_fqn)) + .is_some_and(|(p, r)| *p == field_fqn && *r == crate::PointerRepr::Inline) + { + return Err(CodeGenError::Other(format!( + "message field `{field_fqn}` is recursive and cannot be stored \ + inline: it would make the generated struct unsized. Remove the \ + exact `box_type_in(PointerRepr::Inline, &[\"{field_fqn}\"])` rule \ + — the default keeps recursive fields boxed automatically." + ))); + } // Configurable owned collection for `repeated` (non-map) fields (default // `Vec`). Map fields keep their configured map collection. diff --git a/buffa-codegen/src/oneof.rs b/buffa-codegen/src/oneof.rs index 9136521d..faa7e81e 100644 --- a/buffa-codegen/src/oneof.rs +++ b/buffa-codegen/src/oneof.rs @@ -77,18 +77,18 @@ pub(crate) fn variant_boxed(ctx: &CodeGenContext, ty: Type, variant_fqn: &str) - /// boxing site consistent with the enum declaration and builds the message /// index a single time. pub(crate) fn resolve_unboxed_variants( - files: &[FileDescriptorProto], + index: &std::collections::HashMap, rules: &[String], + pointer_fields: &[(String, crate::PointerRepr)], ) -> std::collections::HashSet { let mut resolved = std::collections::HashSet::new(); if rules.is_empty() { return resolved; } - let index = message_index(files); - for (msg_fqn, msg) in &index { + for (msg_fqn, msg) in index { for_each_message_variant(msg, msg_fqn, |variant_fqn, type_name| { if rule_matches(rules, &variant_fqn) - && !unboxing_is_recursive(&index, rules, msg_fqn, type_name) + && !inline_is_recursive(index, rules, pointer_fields, msg_fqn, type_name) { resolved.insert(variant_fqn); } @@ -97,6 +97,38 @@ pub(crate) fn resolve_unboxed_variants( resolved } +/// Resolve the set of singular message-field paths whose configured +/// [`PointerRepr`](crate::PointerRepr) is `Inline` and which can safely be +/// stored inline (no cycle through inline edges). +/// +/// Mirrors [`resolve_unboxed_variants`]: a field where the raw last-match-wins +/// repr is `Inline` is added unless [`inline_is_recursive`] reports a cycle +/// through inlined oneof variants and/or other inline singular fields. The +/// resulting set is the source of truth for +/// [`CodeGenContext::pointer_repr`](crate::context::CodeGenContext::pointer_repr), +/// which demotes any `Inline` not in this set to `Box`. +/// +/// Runs unconditionally now that `Inline` is the default. Cost is `O(F·(V+E))` +/// (a fresh DFS per candidate field); memoize per-target reachable sets if a +/// very large schema makes this noticeable. +pub(crate) fn resolve_inlined_fields( + index: &std::collections::HashMap, + rules: &[String], + pointer_fields: &[(String, crate::PointerRepr)], +) -> std::collections::HashSet { + let mut resolved = std::collections::HashSet::new(); + for (msg_fqn, msg) in index { + for_each_singular_message_field(msg, msg_fqn, |field_fqn, type_name| { + if raw_pointer_repr(pointer_fields, &field_fqn) == crate::PointerRepr::Inline + && !inline_is_recursive(index, rules, pointer_fields, msg_fqn, type_name) + { + resolved.insert(field_fqn); + } + }); + } + resolved +} + /// Whether any `unboxed_oneof_fields` rule matches the variant path. fn rule_matches(rules: &[String], variant_fqn: &str) -> bool { rules @@ -104,6 +136,20 @@ fn rule_matches(rules: &[String], variant_fqn: &str) -> bool { .any(|prefix| crate::context::matches_proto_prefix(prefix, variant_fqn)) } +/// Last-match-wins [`PointerRepr`](crate::PointerRepr) for `field_fqn` from the +/// raw `pointer_fields` config — no recursion demotion. The recursion check +/// uses this (not the post-demotion resolver) so the walk is order-independent. +fn raw_pointer_repr( + pointer_fields: &[(String, crate::PointerRepr)], + field_fqn: &str, +) -> crate::PointerRepr { + pointer_fields + .iter() + .rev() + .find(|(prefix, _)| crate::context::matches_proto_prefix(prefix, field_fqn)) + .map_or(crate::PointerRepr::default(), |(_, repr)| repr.clone()) +} + /// Invoke `f` for every message/group-typed real oneof member of `msg`, with /// the variant's leading-dot path and its target message name (no leading /// dot). `msg_fqn` has no leading dot. @@ -141,9 +187,44 @@ fn for_each_message_variant(msg: &DescriptorProto, msg_fqn: &str, mut f: impl Fn } } +/// Invoke `f` for every singular (non-repeated, non-oneof) message/group-typed +/// field of `msg`, with the field's leading-dot path and its target message +/// name (no leading dot). `msg_fqn` has no leading dot. +/// +/// These are the fields whose [`PointerRepr`](crate::PointerRepr) governs +/// inline-vs-heap storage; repeated and map fields are heap-backed and break +/// cycles regardless of pointer config. +fn for_each_singular_message_field( + msg: &DescriptorProto, + msg_fqn: &str, + mut f: impl FnMut(String, &str), +) { + use crate::generated::descriptor::field_descriptor_proto::Label; + for field in &msg.field { + if crate::impl_message::is_real_oneof_member(field) { + continue; + } + if !is_boxed_variant(field.r#type.unwrap_or_default()) { + continue; + } + if field.label.unwrap_or_default() == Label::LABEL_REPEATED { + continue; + } + let (Some(field_name), Some(type_name)) = + (field.name.as_deref(), field.type_name.as_deref()) + else { + continue; + }; + f( + format!(".{msg_fqn}.{field_name}"), + type_name.trim_start_matches('.'), + ); + } +} + /// Build a map from fully-qualified message name (no leading dot) to its /// descriptor, walking every file and its nested types. -fn message_index( +pub(crate) fn message_index( files: &[FileDescriptorProto], ) -> std::collections::HashMap { fn walk<'a>( @@ -175,23 +256,25 @@ fn message_index( map } -/// Returns `true` when storing a variant of message type `target` inline -/// inside `enclosing` would produce an unsized type. +/// Returns `true` when storing a value of message type `target` inline inside +/// `enclosing` would produce an unsized type. /// -/// `enclosing` and `target` are fully-qualified message names without a -/// leading dot. A cycle is only reachable through message-typed oneof variants -/// that are themselves stored inline (singular message fields are -/// `Option>`, repeated fields are `Vec`, and maps are heap-backed, so -/// none of those carry storage inline). The walk follows every *rule-matched* -/// edge from `target`; if it reaches `enclosing`, the opt-out is recursive. +/// `enclosing` and `target` are fully-qualified message names without a leading +/// dot. A cycle is reachable only through edges that store the target inline: +/// message-typed oneof variants matched by an `unboxed_oneof_fields` rule, and +/// singular message fields whose raw [`PointerRepr`](crate::PointerRepr) +/// resolves to `Inline`. Repeated fields, maps, and `Box`/custom-pointer fields +/// are heap-backed and break cycles. The walk follows every *rule-matched* edge +/// from `target`; if it reaches `enclosing`, inlining is recursive. /// -/// Following rule-matched (rather than finally-resolved) edges keeps the -/// check order-independent and conservative: an edge that resolution later -/// keeps boxed (because it is itself part of a cycle) can only cause a false -/// `true` here, which keeps more variants boxed — never an unsized type. -fn unboxing_is_recursive( +/// Following rule-matched (rather than finally-resolved) edges keeps the check +/// order-independent and conservative: an edge that resolution later keeps +/// boxed (because it is itself part of a cycle) can only cause a false `true` +/// here, which keeps more fields boxed — never an unsized type. +fn inline_is_recursive( index: &std::collections::HashMap, rules: &[String], + pointer_fields: &[(String, crate::PointerRepr)], enclosing: &str, target: &str, ) -> bool { @@ -212,6 +295,11 @@ fn unboxing_is_recursive( stack.push(type_name.to_string()); } }); + for_each_singular_message_field(msg, ¤t, |field_fqn, type_name| { + if raw_pointer_repr(pointer_fields, &field_fqn) == crate::PointerRepr::Inline { + stack.push(type_name.to_string()); + } + }); } false } diff --git a/buffa-codegen/src/tests/generation.rs b/buffa-codegen/src/tests/generation.rs index 9e58fc1c..42d6eba3 100644 --- a/buffa-codegen/src/tests/generation.rs +++ b/buffa-codegen/src/tests/generation.rs @@ -1398,7 +1398,7 @@ fn test_message_nested_message_field() { "missing Outer: {content}" ); assert!( - content.contains("pub inner: ::buffa::MessageField"), + content.contains("pub inner: ::buffa::MessageField>"), "missing MessageField: {content}" ); // impl Message should use the two-pass size computation for sub-messages. diff --git a/buffa-codegen/src/tests/idiomatic_imports.rs b/buffa-codegen/src/tests/idiomatic_imports.rs index 96b7133e..9ca09e3f 100644 --- a/buffa-codegen/src/tests/idiomatic_imports.rs +++ b/buffa-codegen/src/tests/idiomatic_imports.rs @@ -91,7 +91,8 @@ fn cross_package_field_gets_use_and_short_name() { pkg.content ); assert!( - pkg.content.contains("pub d: MessageField,"), + pkg.content + .contains("pub d: MessageField>,"), "field should use the short names: {}", pkg.content ); @@ -126,8 +127,9 @@ fn flag_off_keeps_qualified_paths_in_fpp_mode() { .expect("should generate"); let pkg = pkg_file(&files, "test.pkg.rs"); assert!( - pkg.content - .contains("pub d: ::buffa::MessageField,"), + pkg.content.contains( + "pub d: ::buffa::MessageField>," + ), "flag-off output must keep qualified paths: {}", pkg.content ); @@ -237,7 +239,8 @@ fn extern_reference_gets_use_directive() { pkg.content ); assert!( - pkg.content.contains("pub at: MessageField,"), + pkg.content + .contains("pub at: MessageField>,"), "field should use the short names: {}", pkg.content ); @@ -288,7 +291,8 @@ fn package_item_collision_falls_down_ladder() { pkg.content ); assert!( - pkg.content.contains("pub d: MessageField,"), + pkg.content + .contains("pub d: MessageField>,"), "collided leaf should be parent-qualified: {}", pkg.content ); @@ -338,8 +342,9 @@ fn reflection_pool_reexport_name_is_reserved() { pkg.content ); assert!( - pkg.content - .contains("pub p: MessageField,"), + pkg.content.contains( + "pub p: MessageField>," + ), "reference should fall to parent-module qualification: {}", pkg.content ); @@ -382,10 +387,11 @@ fn nested_and_oneof_scopes_stay_qualified() { .expect("should generate"); let pkg = pkg_file(&files, "test.pkg.rs"); // Nested-message struct (one module below root): no `use` visible - // there, so the field keeps its qualified path. + // there, so the field keeps its qualified path. The inline pointer's + // type argument carries the same qualified path. assert!( pkg.content - .contains("pub d: ::buffa::MessageField,"), + .contains("::buffa::Inline"), "nested-message field must stay qualified: {}", pkg.content ); @@ -473,8 +479,8 @@ fn output_is_stable_under_file_reordering() { "use block must not depend on file generation order" ); for field in [ - "pub t: MessageField,", - "pub t: MessageField,", + "pub t: MessageField>,", + "pub t: MessageField>,", ] { assert!( fwd.content.contains(field) && rev.content.contains(field), diff --git a/buffa-codegen/src/tests/naming.rs b/buffa-codegen/src/tests/naming.rs index c7326dae..559b78aa 100644 --- a/buffa-codegen/src/tests/naming.rs +++ b/buffa-codegen/src/tests/naming.rs @@ -175,7 +175,7 @@ fn test_type_name_prefix_declarations_and_references() { // Cross-references use the prefixed names. assert!( - content.contains("MessageField"), + content.contains("MessageField>"), "message field must reference prefixed type: {content}" ); assert!( diff --git a/buffa-codegen/src/tests/pointer_repr.rs b/buffa-codegen/src/tests/pointer_repr.rs index d7a4d6d6..918e30b4 100644 --- a/buffa-codegen/src/tests/pointer_repr.rs +++ b/buffa-codegen/src/tests/pointer_repr.rs @@ -61,6 +61,35 @@ fn custom_type_path_threads_pointer_param() { ); } +#[test] +fn inline_type_path_uses_buffa_inline() { + let mf = quote! { ::buffa::MessageField }; + let got = PointerRepr::Inline + .type_path(&mf, "e! { Inner }) + .unwrap(); + assert_eq!( + got.to_string(), + quote! { ::buffa::MessageField> }.to_string() + ); +} + +#[test] +fn inline_some_path_carries_pointer() { + let got = PointerRepr::Inline.some_path("e! { Inner }).unwrap(); + assert_eq!( + got.to_string(), + quote! { ::buffa::MessageField::> }.to_string() + ); +} + +#[test] +fn inline_pointer_new_is_tuple_constructor() { + let got = PointerRepr::Inline + .pointer_new("e! { Inner }, "e! { v }) + .unwrap(); + assert_eq!(got.to_string(), quote! { ::buffa::Inline(v) }.to_string()); +} + #[test] fn custom_some_path_carries_pointer() { let repr = PointerRepr::Custom("::my::SBox<*>".to_string()); diff --git a/buffa-codegen/tests/codegen_integration.rs b/buffa-codegen/tests/codegen_integration.rs index 0187c09b..984817ec 100644 --- a/buffa-codegen/tests/codegen_integration.rs +++ b/buffa-codegen/tests/codegen_integration.rs @@ -335,7 +335,7 @@ fn inline_nested_message() { assert!(content.contains("pub struct Outer")); assert!(content.contains("pub mod outer")); assert!(content.contains("pub struct Inner")); - assert!(content.contains("::buffa::MessageField")); + assert!(content.contains("::buffa::MessageField>")); } #[test] @@ -1615,3 +1615,179 @@ fn prefix_unbox_rule_on_recursive_variant_skips_without_error() { "non-recursive sibling should be inline under the prefix rule: {content}" ); } + +#[test] +fn default_inline_pointer_skips_recursive_fields() { + // PointerRepr::Inline (the default) stores every NON-recursive singular + // message field inline; recursive fields are silently kept on Box. + // Mirrors blanket_unbox_oneof_skips_recursive_variants for #248. + let config = no_views(); + let content = generate_proto( + r#" + syntax = "proto3"; + package test; + message Small { int32 value = 1; } + message Node { + Node child = 1; + Small leaf = 2; + } + "#, + &config, + ); + assert!( + content.contains("pub child: ::buffa::MessageField,"), + "recursive singular field must stay on Box (default pointer) under the blanket rule: {content}" + ); + assert!( + content.contains("pub leaf: ::buffa::MessageField>"), + "non-recursive singular field should be inline under the blanket rule: {content}" + ); +} + +#[test] +fn default_inline_pointer_keeps_mutually_recursive_boxed() { + // Mutual recursion: A.b -> B and B.a -> A form a cycle only when BOTH + // fields are inline. The default Inline must keep both on Box; the + // non-recursive A.c field inlines. + let config = no_views(); + let content = generate_proto( + r#" + syntax = "proto3"; + package test; + message A { B b = 1; C c = 2; } + message B { A a = 1; } + message C { int32 value = 1; } + "#, + &config, + ); + assert!( + content.contains("pub b: ::buffa::MessageField,"), + "mutually recursive field A.b must stay on Box: {content}" + ); + assert!( + content.contains("pub a: ::buffa::MessageField,"), + "mutually recursive field B.a must stay on Box: {content}" + ); + assert!( + content.contains("pub c: ::buffa::MessageField>"), + "non-recursive field A.c should be inline under the blanket rule: {content}" + ); +} + +#[test] +fn exact_inline_rule_on_recursive_field_is_rejected() { + // An exact-path PointerRepr::Inline rule naming a recursive singular + // field is a hard codegen error — the user asked for an unsized struct. + let proto = dedent( + r#" + syntax = "proto3"; + package test; + message Node { Node child = 1; } + "#, + ); + let dir = tempfile::tempdir().expect("temp dir"); + let proto_path = dir.path().join("test.proto"); + std::fs::write(&proto_path, &proto).expect("write proto"); + let fds = compile_protos( + &[proto_path.to_str().unwrap()], + &[dir.path().to_str().unwrap()], + ); + + let mut config = no_views(); + config.pointer_fields.push(( + ".test.Node.child".to_string(), + buffa_codegen::PointerRepr::Inline, + )); + let result = buffa_codegen::generate(&fds.file, &["test.proto".into()], &config); + let err = result.expect_err("inlining a recursive singular field should error"); + let msg = err.to_string(); + assert!( + msg.contains("recursive") && msg.contains(".test.Node.child"), + "error should explain the recursion and name the field: {err}" + ); +} + +#[test] +fn box_type_opt_out_restores_boxed_default() { + // PointerRepr::Box is the opt-out from the Inline default: a path-scoped + // rule keeps the matched fields on Box; a "." blanket restores the + // pre-0.9 boxed default everywhere. + let mut config = no_views(); + config + .pointer_fields + .push((".test.Node".to_string(), buffa_codegen::PointerRepr::Box)); + let content = generate_proto( + r#" + syntax = "proto3"; + package test; + message Leaf { int32 value = 1; } + message Node { Leaf leaf = 1; } + message Elsewhere { Leaf leaf = 1; } + "#, + &config, + ); + assert!( + content.contains("pub leaf: ::buffa::MessageField,"), + "Node.leaf opted out via Box rule must use the boxed pointer: {content}" + ); + assert!( + content.contains("pub leaf: ::buffa::MessageField>"), + "Elsewhere.leaf (no rule) should be inline by default: {content}" + ); +} + +#[test] +fn unbox_oneof_and_inline_field_detect_cross_kind_cycle() { + // A cycle through ONE inlined oneof variant and ONE inlined singular field + // (A.body.b -> B, B.a -> A) must be caught by both resolvers: the variant + // stays boxed and the field stays on Box. Either alone is non-recursive, + // so the recursion DFS must follow both edge kinds. + let mut config = no_views(); + config.unboxed_oneof_fields.push(".".to_string()); + let content = generate_proto( + r#" + syntax = "proto3"; + package test; + message A { + oneof body { B b = 1; } + } + message B { A a = 1; } + "#, + &config, + ); + assert!( + content.contains("B(::buffa::alloc::boxed::Box)"), + "oneof variant in a cross-kind cycle must stay boxed: {content}" + ); + assert!( + content.contains("pub a: ::buffa::MessageField,"), + "singular field in a cross-kind cycle must stay on Box: {content}" + ); +} + +#[test] +fn boxed_oneof_variant_under_inline_default_uses_box() { + // The default PointerRepr::Inline also matches boxed oneof variant paths, + // but the resolved-set check (singular fields only) demotes them: the + // variant stays Box-wrapped so unbox_oneof remains the sole inlining knob + // for oneofs and its recursion guard cannot be bypassed. + let config = no_views(); + let content = generate_proto( + r#" + syntax = "proto3"; + package test; + message Node { + oneof kind { Node child = 1; } + } + "#, + &config, + ); + assert!( + content.contains("Child(::buffa::alloc::boxed::Box<"), + "boxed oneof variant must use Box even under a blanket Inline rule: {content}" + ); + assert!( + !content.contains("Child(::buffa::Inline<"), + "boxed oneof variant must not use the Inline pointer: {content}" + ); +} diff --git a/buffa-descriptor/src/generated/google.protobuf.compiler.plugin.__view.rs b/buffa-descriptor/src/generated/google.protobuf.compiler.plugin.__view.rs index 68cf31ff..5deedcf3 100644 --- a/buffa-descriptor/src/generated/google.protobuf.compiler.plugin.__view.rs +++ b/buffa-descriptor/src/generated/google.protobuf.compiler.plugin.__view.rs @@ -515,6 +515,7 @@ impl<'a> ::buffa::MessageView<'a> for CodeGeneratorRequestView<'a> { Some(v) => { ::buffa::MessageField::< super::super::Version, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -1448,6 +1449,9 @@ pub mod code_generator_response { Some(v) => { ::buffa::MessageField::< super::super::super::super::GeneratedCodeInfo, + ::buffa::Inline< + super::super::super::super::GeneratedCodeInfo, + >, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), diff --git a/buffa-descriptor/src/generated/google.protobuf.compiler.plugin.rs b/buffa-descriptor/src/generated/google.protobuf.compiler.plugin.rs index 83ce4b53..33339f24 100644 --- a/buffa-descriptor/src/generated/google.protobuf.compiler.plugin.rs +++ b/buffa-descriptor/src/generated/google.protobuf.compiler.plugin.rs @@ -393,7 +393,7 @@ pub struct CodeGeneratorRequest { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub compiler_version: ::buffa::MessageField, + pub compiler_version: ::buffa::MessageField>, #[cfg_attr(feature = "json", serde(skip))] #[doc(hidden)] pub __buffa_unknown_fields: ::buffa::UnknownFields, @@ -1337,7 +1337,10 @@ pub mod code_generator_response { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub generated_code_info: ::buffa::MessageField, + pub generated_code_info: ::buffa::MessageField< + super::super::GeneratedCodeInfo, + ::buffa::Inline, + >, #[cfg_attr(feature = "json", serde(skip))] #[doc(hidden)] pub __buffa_unknown_fields: ::buffa::UnknownFields, diff --git a/buffa-descriptor/src/generated/google.protobuf.descriptor.__view.rs b/buffa-descriptor/src/generated/google.protobuf.descriptor.__view.rs index 8fb07431..82f32ea9 100644 --- a/buffa-descriptor/src/generated/google.protobuf.descriptor.__view.rs +++ b/buffa-descriptor/src/generated/google.protobuf.descriptor.__view.rs @@ -642,6 +642,7 @@ impl<'a> ::buffa::MessageView<'a> for FileDescriptorProtoView<'a> { Some(v) => { ::buffa::MessageField::< super::super::FileOptions, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -650,6 +651,7 @@ impl<'a> ::buffa::MessageView<'a> for FileDescriptorProtoView<'a> { Some(v) => { ::buffa::MessageField::< super::super::SourceCodeInfo, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -1429,6 +1431,7 @@ impl<'a> ::buffa::MessageView<'a> for DescriptorProtoView<'a> { Some(v) => { ::buffa::MessageField::< super::super::MessageOptions, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -1980,6 +1983,7 @@ pub mod descriptor_proto { Some(v) => { ::buffa::MessageField::< super::super::super::ExtensionRangeOptions, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -2679,6 +2683,7 @@ impl<'a> ::buffa::MessageView<'a> for ExtensionRangeOptionsView<'a> { Some(v) => { ::buffa::MessageField::< super::super::FeatureSet, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -3610,6 +3615,7 @@ impl<'a> ::buffa::MessageView<'a> for FieldDescriptorProtoView<'a> { Some(v) => { ::buffa::MessageField::< super::super::FieldOptions, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -4103,6 +4109,7 @@ impl<'a> ::buffa::MessageView<'a> for OneofDescriptorProtoView<'a> { Some(v) => { ::buffa::MessageField::< super::super::OneofOptions, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -4497,6 +4504,7 @@ impl<'a> ::buffa::MessageView<'a> for EnumDescriptorProtoView<'a> { Some(v) => { ::buffa::MessageField::< super::super::EnumOptions, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -5218,6 +5226,7 @@ impl<'a> ::buffa::MessageView<'a> for EnumValueDescriptorProtoView<'a> { Some(v) => { ::buffa::MessageField::< super::super::EnumValueOptions, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -5572,6 +5581,7 @@ impl<'a> ::buffa::MessageView<'a> for ServiceDescriptorProtoView<'a> { Some(v) => { ::buffa::MessageField::< super::super::ServiceOptions, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -5957,6 +5967,7 @@ impl<'a> ::buffa::MessageView<'a> for MethodDescriptorProtoView<'a> { Some(v) => { ::buffa::MessageField::< super::super::MethodOptions, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -6665,6 +6676,7 @@ impl<'a> ::buffa::MessageView<'a> for FileOptionsView<'a> { Some(v) => { ::buffa::MessageField::< super::super::FeatureSet, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -7486,6 +7498,7 @@ impl<'a> ::buffa::MessageView<'a> for MessageOptionsView<'a> { Some(v) => { ::buffa::MessageField::< super::super::FeatureSet, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -8244,6 +8257,7 @@ impl<'a> ::buffa::MessageView<'a> for FieldOptionsView<'a> { Some(v) => { ::buffa::MessageField::< super::super::FeatureSet, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -8252,6 +8266,7 @@ impl<'a> ::buffa::MessageView<'a> for FieldOptionsView<'a> { Some(v) => { ::buffa::MessageField::< super::super::field_options::FeatureSupport, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -9572,6 +9587,7 @@ impl<'a> ::buffa::MessageView<'a> for OneofOptionsView<'a> { Some(v) => { ::buffa::MessageField::< super::super::FeatureSet, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -9963,6 +9979,7 @@ impl<'a> ::buffa::MessageView<'a> for EnumOptionsView<'a> { Some(v) => { ::buffa::MessageField::< super::super::FeatureSet, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -10422,6 +10439,7 @@ impl<'a> ::buffa::MessageView<'a> for EnumValueOptionsView<'a> { Some(v) => { ::buffa::MessageField::< super::super::FeatureSet, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -10431,6 +10449,7 @@ impl<'a> ::buffa::MessageView<'a> for EnumValueOptionsView<'a> { Some(v) => { ::buffa::MessageField::< super::super::field_options::FeatureSupport, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -10860,6 +10879,7 @@ impl<'a> ::buffa::MessageView<'a> for ServiceOptionsView<'a> { Some(v) => { ::buffa::MessageField::< super::super::FeatureSet, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -11271,6 +11291,7 @@ impl<'a> ::buffa::MessageView<'a> for MethodOptionsView<'a> { Some(v) => { ::buffa::MessageField::< super::super::FeatureSet, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -13608,6 +13629,7 @@ pub mod feature_set_defaults { Some(v) => { ::buffa::MessageField::< super::super::super::FeatureSet, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), @@ -13616,6 +13638,7 @@ pub mod feature_set_defaults { Some(v) => { ::buffa::MessageField::< super::super::super::FeatureSet, + ::buffa::Inline, >::some(v.to_owned_from_source(__buffa_src)?) } None => ::buffa::MessageField::none(), diff --git a/buffa-descriptor/src/generated/google.protobuf.descriptor.rs b/buffa-descriptor/src/generated/google.protobuf.descriptor.rs index c4e8e730..32f5d709 100644 --- a/buffa-descriptor/src/generated/google.protobuf.descriptor.rs +++ b/buffa-descriptor/src/generated/google.protobuf.descriptor.rs @@ -861,7 +861,7 @@ pub struct FileDescriptorProto { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub options: ::buffa::MessageField, + pub options: ::buffa::MessageField>, /// This field contains optional information about the original source code. /// You may safely remove this entire field without harming runtime /// functionality of the descriptors -- the information is needed only by @@ -876,7 +876,10 @@ pub struct FileDescriptorProto { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub source_code_info: ::buffa::MessageField, + pub source_code_info: ::buffa::MessageField< + SourceCodeInfo, + ::buffa::Inline, + >, /// The syntax of the proto file. /// The supported values are "proto2", "proto3", and "editions". /// @@ -1635,7 +1638,7 @@ pub struct DescriptorProto { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub options: ::buffa::MessageField, + pub options: ::buffa::MessageField>, /// Field 9: `reserved_range` #[cfg_attr( feature = "json", @@ -2240,7 +2243,10 @@ pub mod descriptor_proto { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub options: ::buffa::MessageField, + pub options: ::buffa::MessageField< + super::ExtensionRangeOptions, + ::buffa::Inline, + >, #[cfg_attr(feature = "json", serde(skip))] #[doc(hidden)] pub __buffa_unknown_fields: ::buffa::UnknownFields, @@ -2746,7 +2752,7 @@ pub struct ExtensionRangeOptions { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub features: ::buffa::MessageField, + pub features: ::buffa::MessageField>, /// The verification state of the range. /// TODO: flip the default to DECLARATION once all empty ranges /// are marked as UNVERIFIED. @@ -3056,7 +3062,7 @@ impl<'de> serde::Deserialize<'de> for ExtensionRangeOptions { ::buffa::alloc::vec::Vec, > = None; let mut __f_features: ::core::option::Option< - ::buffa::MessageField, + ::buffa::MessageField>, > = None; let mut __f_verification: ::core::option::Option< ::core::option::Option, @@ -3106,7 +3112,13 @@ impl<'de> serde::Deserialize<'de> for ExtensionRangeOptions { } "features" => { __f_features = Some( - map.next_value::<::buffa::MessageField>()?, + map + .next_value::< + ::buffa::MessageField< + FeatureSet, + ::buffa::Inline, + >, + >()?, ); } "verification" => { @@ -3909,7 +3921,7 @@ pub struct FieldDescriptorProto { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub options: ::buffa::MessageField, + pub options: ::buffa::MessageField>, /// If true, this is a proto3 "optional". When a proto3 field is optional, it /// tracks presence regardless of field type. /// @@ -4936,7 +4948,7 @@ pub struct OneofDescriptorProto { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub options: ::buffa::MessageField, + pub options: ::buffa::MessageField>, #[cfg_attr(feature = "json", serde(skip))] #[doc(hidden)] pub __buffa_unknown_fields: ::buffa::UnknownFields, @@ -5168,7 +5180,7 @@ pub struct EnumDescriptorProto { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub options: ::buffa::MessageField, + pub options: ::buffa::MessageField>, /// Range of reserved numeric values. Reserved numeric values may not be used /// by enum values in the same enum declaration. Reserved ranges may not /// overlap. @@ -5821,7 +5833,10 @@ pub struct EnumValueDescriptorProto { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub options: ::buffa::MessageField, + pub options: ::buffa::MessageField< + EnumValueOptions, + ::buffa::Inline, + >, #[cfg_attr(feature = "json", serde(skip))] #[doc(hidden)] pub __buffa_unknown_fields: ::buffa::UnknownFields, @@ -6082,7 +6097,7 @@ pub struct ServiceDescriptorProto { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub options: ::buffa::MessageField, + pub options: ::buffa::MessageField>, #[cfg_attr(feature = "json", serde(skip))] #[doc(hidden)] pub __buffa_unknown_fields: ::buffa::UnknownFields, @@ -6364,7 +6379,7 @@ pub struct MethodDescriptorProto { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub options: ::buffa::MessageField, + pub options: ::buffa::MessageField>, /// Identifies if client streams multiple client messages /// /// Field 5: `client_streaming` @@ -7054,7 +7069,7 @@ pub struct FileOptions { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub features: ::buffa::MessageField, + pub features: ::buffa::MessageField>, /// The parser stores options it doesn't recognize here. /// See the documentation for the "Options" section above. /// @@ -8027,7 +8042,7 @@ impl<'de> serde::Deserialize<'de> for FileOptions { ::core::option::Option<::buffa::alloc::string::String>, > = None; let mut __f_features: ::core::option::Option< - ::buffa::MessageField, + ::buffa::MessageField>, > = None; let mut __f_uninterpreted_option: ::core::option::Option< ::buffa::alloc::vec::Vec, @@ -8178,7 +8193,13 @@ impl<'de> serde::Deserialize<'de> for FileOptions { } "features" => { __f_features = Some( - map.next_value::<::buffa::MessageField>()?, + map + .next_value::< + ::buffa::MessageField< + FeatureSet, + ::buffa::Inline, + >, + >()?, ); } "uninterpretedOption" | "uninterpreted_option" => { @@ -8668,7 +8689,7 @@ pub struct MessageOptions { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub features: ::buffa::MessageField, + pub features: ::buffa::MessageField>, /// The parser stores options it doesn't recognize here. See above. /// /// Field 999: `uninterpreted_option` @@ -9072,7 +9093,7 @@ impl<'de> serde::Deserialize<'de> for MessageOptions { ::core::option::Option, > = None; let mut __f_features: ::core::option::Option< - ::buffa::MessageField, + ::buffa::MessageField>, > = None; let mut __f_uninterpreted_option: ::core::option::Option< ::buffa::alloc::vec::Vec, @@ -9109,7 +9130,13 @@ impl<'de> serde::Deserialize<'de> for MessageOptions { } "features" => { __f_features = Some( - map.next_value::<::buffa::MessageField>()?, + map + .next_value::< + ::buffa::MessageField< + FeatureSet, + ::buffa::Inline, + >, + >()?, ); } "uninterpretedOption" | "uninterpreted_option" => { @@ -9447,7 +9474,7 @@ pub struct FieldOptions { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub features: ::buffa::MessageField, + pub features: ::buffa::MessageField>, /// Field 22: `feature_support` #[cfg_attr( feature = "json", @@ -9457,7 +9484,10 @@ pub struct FieldOptions { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub feature_support: ::buffa::MessageField, + pub feature_support: ::buffa::MessageField< + field_options::FeatureSupport, + ::buffa::Inline, + >, /// The parser stores options it doesn't recognize here. See above. /// /// Field 999: `uninterpreted_option` @@ -10157,10 +10187,13 @@ impl<'de> serde::Deserialize<'de> for FieldOptions { ::buffa::alloc::vec::Vec, > = None; let mut __f_features: ::core::option::Option< - ::buffa::MessageField, + ::buffa::MessageField>, > = None; let mut __f_feature_support: ::core::option::Option< - ::buffa::MessageField, + ::buffa::MessageField< + field_options::FeatureSupport, + ::buffa::Inline, + >, > = None; let mut __f_uninterpreted_option: ::core::option::Option< ::buffa::alloc::vec::Vec, @@ -10296,14 +10329,23 @@ impl<'de> serde::Deserialize<'de> for FieldOptions { } "features" => { __f_features = Some( - map.next_value::<::buffa::MessageField>()?, + map + .next_value::< + ::buffa::MessageField< + FeatureSet, + ::buffa::Inline, + >, + >()?, ); } "featureSupport" | "feature_support" => { __f_feature_support = Some( map .next_value::< - ::buffa::MessageField, + ::buffa::MessageField< + field_options::FeatureSupport, + ::buffa::Inline, + >, >()?, ); } @@ -11816,7 +11858,7 @@ pub struct OneofOptions { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub features: ::buffa::MessageField, + pub features: ::buffa::MessageField>, /// The parser stores options it doesn't recognize here. See above. /// /// Field 999: `uninterpreted_option` @@ -12032,7 +12074,7 @@ impl<'de> serde::Deserialize<'de> for OneofOptions { mut map: A, ) -> ::core::result::Result { let mut __f_features: ::core::option::Option< - ::buffa::MessageField, + ::buffa::MessageField>, > = None; let mut __f_uninterpreted_option: ::core::option::Option< ::buffa::alloc::vec::Vec, @@ -12042,7 +12084,13 @@ impl<'de> serde::Deserialize<'de> for OneofOptions { match key.as_str() { "features" => { __f_features = Some( - map.next_value::<::buffa::MessageField>()?, + map + .next_value::< + ::buffa::MessageField< + FeatureSet, + ::buffa::Inline, + >, + >()?, ); } "uninterpretedOption" | "uninterpreted_option" => { @@ -12245,7 +12293,7 @@ pub struct EnumOptions { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub features: ::buffa::MessageField, + pub features: ::buffa::MessageField>, /// The parser stores options it doesn't recognize here. See above. /// /// Field 999: `uninterpreted_option` @@ -12571,7 +12619,7 @@ impl<'de> serde::Deserialize<'de> for EnumOptions { ::core::option::Option, > = None; let mut __f_features: ::core::option::Option< - ::buffa::MessageField, + ::buffa::MessageField>, > = None; let mut __f_uninterpreted_option: ::core::option::Option< ::buffa::alloc::vec::Vec, @@ -12597,7 +12645,13 @@ impl<'de> serde::Deserialize<'de> for EnumOptions { } "features" => { __f_features = Some( - map.next_value::<::buffa::MessageField>()?, + map + .next_value::< + ::buffa::MessageField< + FeatureSet, + ::buffa::Inline, + >, + >()?, ); } "uninterpretedOption" | "uninterpreted_option" => { @@ -12779,7 +12833,7 @@ pub struct EnumValueOptions { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub features: ::buffa::MessageField, + pub features: ::buffa::MessageField>, /// Indicate that fields annotated with this enum value should not be printed /// out when using debug formats, e.g. when the field contains sensitive /// credentials. @@ -12805,7 +12859,10 @@ pub struct EnumValueOptions { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub feature_support: ::buffa::MessageField, + pub feature_support: ::buffa::MessageField< + field_options::FeatureSupport, + ::buffa::Inline, + >, /// The parser stores options it doesn't recognize here. See above. /// /// Field 999: `uninterpreted_option` @@ -13120,13 +13177,16 @@ impl<'de> serde::Deserialize<'de> for EnumValueOptions { ::core::option::Option, > = None; let mut __f_features: ::core::option::Option< - ::buffa::MessageField, + ::buffa::MessageField>, > = None; let mut __f_debug_redact: ::core::option::Option< ::core::option::Option, > = None; let mut __f_feature_support: ::core::option::Option< - ::buffa::MessageField, + ::buffa::MessageField< + field_options::FeatureSupport, + ::buffa::Inline, + >, > = None; let mut __f_uninterpreted_option: ::core::option::Option< ::buffa::alloc::vec::Vec, @@ -13141,7 +13201,13 @@ impl<'de> serde::Deserialize<'de> for EnumValueOptions { } "features" => { __f_features = Some( - map.next_value::<::buffa::MessageField>()?, + map + .next_value::< + ::buffa::MessageField< + FeatureSet, + ::buffa::Inline, + >, + >()?, ); } "debugRedact" | "debug_redact" => { @@ -13153,7 +13219,10 @@ impl<'de> serde::Deserialize<'de> for EnumValueOptions { __f_feature_support = Some( map .next_value::< - ::buffa::MessageField, + ::buffa::MessageField< + field_options::FeatureSupport, + ::buffa::Inline, + >, >()?, ); } @@ -13322,7 +13391,7 @@ pub struct ServiceOptions { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub features: ::buffa::MessageField, + pub features: ::buffa::MessageField>, /// Note: Field numbers 1 through 32 are reserved for Google's internal RPC /// framework. We apologize for hoarding these numbers to ourselves, but /// we were already using them long before we decided to release Protocol @@ -13590,7 +13659,7 @@ impl<'de> serde::Deserialize<'de> for ServiceOptions { mut map: A, ) -> ::core::result::Result { let mut __f_features: ::core::option::Option< - ::buffa::MessageField, + ::buffa::MessageField>, > = None; let mut __f_deprecated: ::core::option::Option< ::core::option::Option, @@ -13603,7 +13672,13 @@ impl<'de> serde::Deserialize<'de> for ServiceOptions { match key.as_str() { "features" => { __f_features = Some( - map.next_value::<::buffa::MessageField>()?, + map + .next_value::< + ::buffa::MessageField< + FeatureSet, + ::buffa::Inline, + >, + >()?, ); } "deprecated" => { @@ -13800,7 +13875,7 @@ pub struct MethodOptions { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub features: ::buffa::MessageField, + pub features: ::buffa::MessageField>, /// The parser stores options it doesn't recognize here. See above. /// /// Field 999: `uninterpreted_option` @@ -14103,7 +14178,7 @@ impl<'de> serde::Deserialize<'de> for MethodOptions { ::core::option::Option, > = None; let mut __f_features: ::core::option::Option< - ::buffa::MessageField, + ::buffa::MessageField>, > = None; let mut __f_uninterpreted_option: ::core::option::Option< ::buffa::alloc::vec::Vec, @@ -14138,7 +14213,13 @@ impl<'de> serde::Deserialize<'de> for MethodOptions { } "features" => { __f_features = Some( - map.next_value::<::buffa::MessageField>()?, + map + .next_value::< + ::buffa::MessageField< + FeatureSet, + ::buffa::Inline, + >, + >()?, ); } "uninterpretedOption" | "uninterpreted_option" => { @@ -17800,7 +17881,10 @@ pub mod feature_set_defaults { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub overridable_features: ::buffa::MessageField, + pub overridable_features: ::buffa::MessageField< + super::FeatureSet, + ::buffa::Inline, + >, /// Defaults of features that can't be overridden in this edition. /// /// Field 5: `fixed_features` @@ -17812,7 +17896,10 @@ pub mod feature_set_defaults { skip_serializing_if = "::buffa::json_helpers::skip_if::is_unset_message_field" ) )] - pub fixed_features: ::buffa::MessageField, + pub fixed_features: ::buffa::MessageField< + super::FeatureSet, + ::buffa::Inline, + >, #[cfg_attr(feature = "json", serde(skip))] #[doc(hidden)] pub __buffa_unknown_fields: ::buffa::UnknownFields, diff --git a/buffa-descriptor/src/pool.rs b/buffa-descriptor/src/pool.rs index 205c5836..3d0b0d65 100644 --- a/buffa-descriptor/src/pool.rs +++ b/buffa-descriptor/src/pool.rs @@ -45,8 +45,12 @@ use buffa::MessageField; /// Clone a descriptor's raw `*Options` into a boxed `Option`, the form the /// linked descriptors store. `None` for the common no-options case; one -/// allocation only when options are present. -fn clone_options(opts: &MessageField) -> Option> { +/// allocation only when options are present. Generic over the source field's +/// pointer (`Inline` for non-recursive fields under the default, `Box` for the +/// recursive `FieldOptions.features` chain). +fn clone_options>( + opts: &MessageField, +) -> Option> { opts.as_option().cloned().map(Box::new) } diff --git a/buffa-test/build.rs b/buffa-test/build.rs index 80089bf2..d8785481 100644 --- a/buffa-test/build.rs +++ b/buffa-test/build.rs @@ -22,6 +22,17 @@ fn main() { .compile() .expect("buffa_build failed for box_type.proto"); + // PointerRepr::Inline default: every non-recursive singular message field + // is the built-in inline pointer (`::buffa::Inline`) with no config + // knob. The `self_ref` field is recursive and must be silently kept on + // `Box` — the crate compiling proves the recursion guard works on the + // default config (an inlined `self_ref` would E0072). + buffa_build::Config::new() + .files(&["protos/inline_field.proto"]) + .includes(&["protos/"]) + .compile() + .expect("buffa_build failed for inline_field.proto"); + // views(false) + vtable: owned-message vtable reflection is self-contained, // so it must compile without view generation (only owned impls emitted). buffa_build::Config::new() diff --git a/buffa-test/protos/inline_field.proto b/buffa-test/protos/inline_field.proto new file mode 100644 index 00000000..d0999d4a --- /dev/null +++ b/buffa-test/protos/inline_field.proto @@ -0,0 +1,21 @@ +syntax = "proto3"; + +package inline_field; + +// Compiled with the default `PointerRepr::Inline` (no pointer config) so every +// non-recursive singular message field uses the built-in `::buffa::Inline` +// pointer (laid out as `Option` in the parent struct, no heap). The +// `self_ref` field is recursive and must be silently kept on `Box`. Exercises +// the decode (`get_or_insert_default`), encode (deref), and view→owned (`some`) +// paths through the inline pointer, plus the recursion guard. +message Outer { + Inner inner = 1; + Inner maybe = 2; + int32 count = 3; + Outer self_ref = 4; +} + +message Inner { + int32 id = 1; + string name = 2; +} diff --git a/buffa-test/src/lib.rs b/buffa-test/src/lib.rs index 6b57247b..63aa11d9 100644 --- a/buffa-test/src/lib.rs +++ b/buffa-test/src/lib.rs @@ -54,6 +54,14 @@ pub mod box_type { buffa::include_proto!("box_type"); } +/// `PointerRepr::Inline` default: the built-in inline pointer +/// (`::buffa::Inline`) for every non-recursive singular message field. The +/// `self_ref` field is recursive and stays on `Box`. +#[allow(clippy::derivable_impls, clippy::match_single_binding)] +pub mod inline_field { + buffa::include_proto!("inline_field"); +} + /// `string_type` + vtable reflection with a crate-local newtype string used as /// a `repeated` element. Because the type is local, codegen may emit the /// `ReflectElement` and `ProtoElemJson` impls for it — the orphan rule forbids diff --git a/buffa-test/src/tests/inline_field.rs b/buffa-test/src/tests/inline_field.rs new file mode 100644 index 00000000..7f3e4454 --- /dev/null +++ b/buffa-test/src/tests/inline_field.rs @@ -0,0 +1,89 @@ +//! `PointerRepr::Inline` default: built-in inline storage for singular message +//! fields. +//! +//! `inline_field.proto` is compiled with **no pointer config**, so every +//! non-recursive singular message field is `MessageField>` +//! (laid out as `Option` in the parent struct — no heap). The recursive +//! `self_ref` field stays on `Box`. Compiling `crate::inline_field` is most of +//! the test: an inlined `self_ref` would E0072, so the recursion guard is proven +//! on the default config by `cargo build`. The runtime checks below pin the +//! field types, assert the inline layout, and verify binary + view→owned +//! round-trips. + +use crate::inline_field::{Inner, Outer}; +use buffa::{Inline, Message, MessageField}; + +fn inner(id: i32, name: &str) -> Inner { + Inner { + id, + name: name.into(), + ..Default::default() + } +} + +fn sample() -> Outer { + Outer { + inner: MessageField::some(inner(7, "alpha")), + maybe: MessageField::some(inner(9, "beta")), + count: 42, + ..Default::default() + } +} + +#[test] +fn field_types_are_inline_except_recursive() { + // Fails to compile if codegen emitted the wrong pointer. + let m = Outer::default(); + let _: &MessageField> = &m.inner; + let _: &MessageField> = &m.maybe; + let _: i32 = m.count; + // self_ref is recursive: must stay on Box (default pointer param). + let _: &MessageField = &m.self_ref; +} + +#[test] +fn inline_field_layout_is_option_t() { + // The whole point of #248: no per-field heap allocation. `Inline` is + // `repr(transparent)`, so `MessageField>` is `Option`. + use core::mem::size_of; + assert_eq!( + size_of::>>(), + size_of::>() + ); + // And the parent struct holds at least two `Inner` worth (the boxed default + // would be two pointers worth instead). + assert!(size_of::() >= 2 * size_of::()); +} + +#[test] +fn binary_round_trip() { + let msg = sample(); + let bytes = msg.encode_to_vec(); + let decoded = Outer::decode(&mut bytes.as_slice()).expect("decode"); + assert_eq!(decoded, msg); + assert_eq!(decoded.inner.id, 7); + assert_eq!(decoded.inner.name, "alpha"); + assert!(decoded.self_ref.is_unset()); +} + +#[test] +fn self_referential_nesting_round_trips() { + // The recursive field is heap-backed, so a chain works. + let mut msg = Outer::default(); + msg.self_ref = MessageField::some(sample()); + let bytes = msg.encode_to_vec(); + let decoded = Outer::decode(&mut bytes.as_slice()).expect("decode"); + assert!(decoded.self_ref.is_set()); + assert_eq!(decoded.self_ref.inner.id, 7); +} + +#[test] +fn view_to_owned_round_trip() { + // Exercises the view→owned `some_path` emitting the inline pointer. + let bytes = bytes::Bytes::from(sample().encode_to_vec()); + let owned: Outer = crate::inline_field::OuterOwnedView::decode(bytes) + .expect("decode view") + .to_owned_message() + .expect("to_owned"); + assert_eq!(owned, sample()); +} diff --git a/buffa-test/src/tests/mod.rs b/buffa-test/src/tests/mod.rs index 2337f6df..b0bbda45 100644 --- a/buffa-test/src/tests/mod.rs +++ b/buffa-test/src/tests/mod.rs @@ -39,6 +39,7 @@ mod editions_enum_json; mod extensions; mod extensions_json; mod idiomatic_imports; +mod inline_field; mod json; mod keyword; mod lazy_views; diff --git a/buffa-test/src/tests/proto3_semantics.rs b/buffa-test/src/tests/proto3_semantics.rs index e8ede073..669bc28f 100644 --- a/buffa-test/src/tests/proto3_semantics.rs +++ b/buffa-test/src/tests/proto3_semantics.rs @@ -553,8 +553,10 @@ fn synthetic_oneofs_not_user_visible() { let _: Option = OptionalAllTypes::default().i32; let _: Option = OptionalAllTypes::default().s; let _: Option> = OptionalAllTypes::default().e; - // The optional message field uses MessageField, not Option>. - let _: buffa::MessageField = OptionalAllTypes::default().nested; + // The optional message field uses MessageField (default-inline), not a + // synthetic oneof enum. + let _: buffa::MessageField> = + OptionalAllTypes::default().nested; } // ═══════════════════════════════════════════════════════════════════════════ diff --git a/buffa/src/json_helpers.rs b/buffa/src/json_helpers.rs index 756ea136..5103f96e 100644 --- a/buffa/src/json_helpers.rs +++ b/buffa/src/json_helpers.rs @@ -1773,9 +1773,12 @@ impl<'de, T: serde::Deserialize<'de>> serde::de::DeserializeSeed<'de> /// maps `null` → `None` (field absent). For types like `google.protobuf.Value` /// where `null` is a valid value (`NullValue`), this function ensures `null` /// reaches `T::deserialize` and the field is set rather than absent. -pub fn message_field_always_present<'de, T, D>(d: D) -> Result, D::Error> +pub fn message_field_always_present<'de, T, P, D>( + d: D, +) -> Result, D::Error> where T: Default + serde::Deserialize<'de>, + P: crate::ProtoBox, D: serde::Deserializer<'de>, { T::deserialize(d).map(crate::MessageField::some) diff --git a/buffa/src/lib.rs b/buffa/src/lib.rs index 44928683..b224d135 100644 --- a/buffa/src/lib.rs +++ b/buffa/src/lib.rs @@ -252,7 +252,7 @@ pub use message::{ DecodeContext, DecodeOptions, Message, MessageName, DEFAULT_UNKNOWN_FIELD_LIMIT, RECURSION_LIMIT, }; -pub use message_field::{DefaultInstance, MessageField, ProtoBox}; +pub use message_field::{DefaultInstance, Inline, MessageField, ProtoBox}; pub use oneof::Oneof; pub use size_cache::{SizeCache, SizeCachePool}; pub use types::{ProtoBytes, ProtoList, ProtoString, WirePayload}; diff --git a/buffa/src/message_field.rs b/buffa/src/message_field.rs index f006acce..afc83182 100644 --- a/buffa/src/message_field.rs +++ b/buffa/src/message_field.rs @@ -81,16 +81,23 @@ macro_rules! impl_default_instance { /// The owned smart pointer backing a singular message field inside /// [`MessageField`]. /// -/// The default is [`Box`]; `buffa_build`'s `box_type_custom` knob substitutes -/// any pointer that implements `ProtoBox` — for example a `smallbox`-style -/// pointer that stores small messages inline and avoids a heap allocation. The -/// wire format is unchanged; only the in-memory ownership of the boxed message -/// changes, and view types are unaffected. +/// The codegen default is [`Inline`] — the message is stored directly in the +/// parent struct (no heap), with recursive fields kept on `Box` +/// automatically. `buffa_build`'s [`box_type_in`][bti] selects [`Box`] (or a +/// custom pointer) for fields where reserving `size_of::()` in the parent is +/// wasteful. The `box_type_custom` knob substitutes any pointer that implements +/// `ProtoBox` — for example a `smallbox`-style pointer that stores small +/// messages inline but spills large ones to the heap. The wire format is +/// unchanged; only the in-memory ownership of the boxed message changes, and +/// view types are unaffected. /// -/// There is intentionally no blanket impl — the built-in `Box` impl below is -/// the only one buffa provides. Because `ProtoBox` is buffa-owned, a *foreign* -/// pointer cannot implement it directly (orphan rule); wrap it in a crate-local -/// newtype, like the `ProtoString` newtype pattern. +/// There is intentionally no blanket impl — the built-in [`Box`] and +/// [`Inline`] impls below are the only ones buffa provides. Because +/// `ProtoBox` is buffa-owned, a *foreign* pointer cannot implement it directly +/// (orphan rule); wrap it in a crate-local newtype, like the `ProtoString` +/// newtype pattern. +/// +/// [bti]: https://docs.rs/buffa-build/latest/buffa_build/struct.Config.html#method.box_type_in /// /// # Why `DerefMut` (and why `Rc` / `Arc` are excluded) /// @@ -115,9 +122,10 @@ macro_rules! impl_default_instance { /// /// # Caveat /// -/// An inline pointer (e.g. `SmallBox`) inflates the *parent* struct by its -/// inline-storage size for every such field, so it should be selected per field -/// or prefix, never as a blanket default. +/// A *custom* inline pointer (e.g. `SmallBox`) inflates the parent struct by +/// its inline-storage size for every such field. The built-in [`Inline`] +/// default is recursion-aware and is the intended blanket; reserve custom +/// inline pointers for per-field or per-prefix overrides. /// /// # Examples /// @@ -173,21 +181,77 @@ impl ProtoBox for Box { } } -// The default pointer must always satisfy the bound; freeze that invariant +/// A [`ProtoBox`] that stores the message inline (no heap allocation). +/// +/// `MessageField>` is laid out as `Option` — set values live +/// directly in the parent struct. This eliminates the per-field allocation of +/// the default `Box` pointer at the cost of inflating the parent's size by +/// `size_of::()` even when the field is unset. +/// +/// Codegen uses this by default via [`PointerRepr::Inline`][pri], which is +/// recursion-aware: a field that would form an infinite-size cycle is silently +/// kept on `Box`. Opt out per-field with `box_type_in(PointerRepr::Box, …)`. +/// +/// [pri]: https://docs.rs/buffa-codegen/latest/buffa_codegen/enum.PointerRepr.html#variant.Inline +#[derive(Debug, Clone, PartialEq, Eq, Hash, Default)] +#[repr(transparent)] +pub struct Inline(pub T); + +impl Deref for Inline { + type Target = T; + #[inline] + fn deref(&self) -> &T { + &self.0 + } +} + +impl DerefMut for Inline { + #[inline] + fn deref_mut(&mut self) -> &mut T { + &mut self.0 + } +} + +impl ProtoBox for Inline { + #[inline] + fn new(value: T) -> Self { + Inline(value) + } + + #[inline] + fn into_inner(self) -> T { + self.0 + } +} + +/// Convenience mirror of `Box`'s `From`; codegen and [`MessageField::some`] +/// use [`ProtoBox::new`] instead. +impl From for Inline { + #[inline] + fn from(value: T) -> Self { + Inline(value) + } +} + +// The built-in pointers must always satisfy the bound; freeze that invariant // against future changes to the trait's supertraits. const _: fn() = || { fn assert_proto_box, T>() {} assert_proto_box::, u32>(); + assert_proto_box::, u32>(); }; /// A wrapper for optional message fields that provides transparent access /// to a default instance when the field is not set. /// -/// This type is used for singular message fields in generated code. It avoids -/// the ergonomic pain of `Option>` while still being heap-allocated -/// only when set (no allocation for unset fields). -/// -/// The pointer type `P` is pluggable (default [`Box`]); see [`ProtoBox`]. +/// This type is used for singular message fields in generated code. The pointer +/// type `P` is pluggable; see [`ProtoBox`]. Codegen emits [`Inline`] by +/// default — the message is stored directly in the parent struct (laid out as +/// `Option`), so `size_of::()` is reserved whether set or not and there +/// is no per-field heap allocation. Recursive fields and explicit opt-outs use +/// `Box` instead, which heap-allocates only when set. The struct's own +/// `P = Box` type-parameter default applies only to a hand-written +/// `MessageField` (e.g. in tests), not to generated fields. /// Because `P` has a default, a *standalone* construction with no pinning /// context needs a type annotation — write `let f: MessageField = /// MessageField::some(x);` (or `MessageField::::some(x)`). In the common @@ -402,8 +466,10 @@ impl> MessageField { impl MessageField> { /// Create a `MessageField` from a boxed value. /// - /// Specific to the default `Box` pointer; for a custom [`ProtoBox`] - /// pointer construct with [`some`](Self::some). + /// Specific to `Box` (the struct's `P = Box` type-parameter default, + /// used for recursive fields and explicit opt-outs); inline-backed + /// generated fields use [`some`](Self::some) or + /// [`from_pointer`](Self::from_pointer) instead. #[inline] pub fn from_box(value: Box) -> Self { Self { @@ -543,6 +609,26 @@ mod tests { // `$crate` path resolution). crate::impl_default_instance!(Inner); + #[test] + fn inline_message_field_is_option_layout() { + // `Inline` is `repr(transparent)`, so `MessageField>` + // (which is `Option>`) is laid out as `Option`. + use core::mem::size_of; + assert_eq!( + size_of::>>(), + size_of::>() + ); + // And the `ProtoBox` surface round-trips. + let mut f: MessageField> = MessageField::some(Inner { + value: 7, + ..Default::default() + }); + assert_eq!(f.value, 7); + f.get_or_insert_default().value = 9; + assert_eq!(f.take().map(|i| i.value), Some(9)); + assert!(f.is_unset()); + } + #[test] fn impl_default_instance_macro_returns_singleton() { let a: &'static Inner = Inner::default_instance(); diff --git a/docs/guide.md b/docs/guide.md index d2e34acf..fa609f96 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -703,9 +703,9 @@ use my_crate::pkg::__buffa::{oneof, view}; // then: pkg::Foo, view::FooView, oneof::foo::Kind, view::oneof::foo::Kind ``` -### `MessageField` — ergonomic optional messages +### `MessageField` — ergonomic optional messages -`MessageField` wraps `Option>` internally but implements `Deref` to a static default instance when unset, eliminating unwrap ceremony: +`MessageField` stores the message inline by default (`P = Inline`, laid out as `Option` — no per-field heap allocation; recursive fields and explicit opt-outs use `P = Box`). It implements `Deref` to a static default instance when unset, eliminating unwrap ceremony: ```rust,ignore // Reading — no unwrap needed, derefs to default when unset