Reflection: owned-message vtable, ReflectMode, vtable by default#149
Merged
Conversation
Add the runtime half of vtable-mode reflection: ReflectList/ReflectMap impls for the zero-copy view containers RepeatedView and MapView, so a generated view's reflective get() can return a borrowed list/map without materializing a Vec<Value>. Introduce two helper traits, ReflectElement and ReflectMapKey, with concrete impls for scalars, &str, &[u8], and EnumValue<E>. A trait-bound blanket over ReflectMessage is not possible — it would overlap the scalar impls under Rust's coherence rules — so codegen will emit a one-line ReflectElement impl per generated view and enum in a later change. Map reflection deduplicates duplicate wire entries (last-write-wins) via MapView::iter_unique to match the bridge path's distinct-key semantics, keeping len() and for_each() consistent across both reflection modes. Also record the landed ValueRef::List/Map refactor and the coherence constraint in the vtable design doc.
Generate `impl ReflectMessage for FooView<'a>` directly on view types when both reflection and the internal vtable flag are set. get()/has() read view struct fields through a per-field match — no encode/decode round-trip and no DynamicMessage — covering scalars, string/bytes, enums, nested messages, repeated, map, optional presence, and oneof members. for_each_set walks the descriptor; to_dynamic falls back to a bridge-style snapshot. Also emit `impl ReflectElement` for view types and generated enums so a RepeatedView/MapView of messages or closed enums reflects through the generic container impls. Per-message MessageIndex is memoized via an inherent associated fn (collision-free across sibling views in a shared module). Gated behind a new internal CodeGenConfig flag, exposed experimentally as buffa_build::Config::generate_reflection_vtable. Bridge mode and the reflect() call-site contract are unchanged.
Make the WKT view types (TimestampView, DurationView, AnyView, StructView, ValueView, wrappers, etc.) implement ReflectMessage, so a message that has a WKT field can reflect over it. Previously vtable reflection only worked for protos that did not reference WKTs, because the WKT views lived in buffa-types with no path to ReflectMessage. The reflection surface pulls a buffa-descriptor dependency and requires std (the embedded descriptor pool uses OnceLock), so it is gated behind a new buffa-types `reflect` feature; views and text stay unconditional. This is enabled by a targeted codegen flag, gate_reflect_on_crate_feature, which gates only the reflection impls — unlike gate_impls_on_crate_features, which gates json/views/text/reflect together and would have forced buffa-types consumers to opt into views/text. All seven WKT protos share the google.protobuf package, so one embedded descriptor pool backs every WKT view's reflection.
Add a sixth conformance run that exercises the generated `impl ReflectMessage for FooView`: decode a view, walk its vtable reflection surface (for_each_set/get) to rebuild a DynamicMessage, and serialize that to JSON. This reuses DynamicMessage's JSON serializer — which passes the corpus cleanly under BUFFA_VIA_REFLECT — so any failure isolates a bug in the vtable get/has surface rather than in JSON formatting. Binary and text output are skipped (the reflect rebuild drops unknown fields, which JSON omits anyway). The run passes 1246 binary->JSON conformance tests with zero failures across proto2/proto3/editions, so known_failures_view_vtable.txt is empty. Vtable reflection is enabled on the four view-bearing conformance protos and gated behind the conformance `reflect` feature (via the new buffa_build::Config::gate_reflect_on_crate_feature), so the no_std binary omits it. Also fix a latent needless-return in process_editions surfaced by clippy once the editions protos are present, and correct the stale conformance-run count in CONTRIBUTING (four runs -> six, including the previously undocumented via-reflect run).
Extend the reflection benchmark with the zero-copy view path so the vtable
ReflectMessage work can be measured against the alternatives:
- decode/view — decode_view alone (no reflection), the zero-copy floor.
- reflect/{vtable,bridge,dynamic}_read_{one,all} — from wire bytes, obtain a
reflective handle and read one field / all set fields, comparing the view
vtable path, the bridge round-trip, and pure DynamicMessage reflection.
Enables generate_reflection_vtable on the bench types so the view types
implement ReflectMessage. Measurements: vtable view reflection runs ~6-10x
faster than the bridge round-trip and ~4x faster than pure DynamicMessage
reflection, because it is dominated by decode_view, which is itself cheaper
than owned decode (borrowed strings/bytes, no per-field allocation).
Add ReflectElement for the owned element types (String, Vec<u8>, Bytes) and Value, ReflectMapKey for String, and generic ReflectList for Vec<T> plus ReflectMap for std::collections::HashMap<K, V> (std-gated — vtable reflection needs std). This lets owned-message vtable reflection return ValueRef::List(&self.field) / ValueRef::Map(&self.field) over owned Vec/HashMap storage, mirroring the view-container impls. The generic Vec<T> impl subsumes the bridge DynamicMessage's Vec<Value> storage via ReflectElement for Value, so the bespoke ReflectList for Vec<Value> is removed — a single list impl now covers both. MapValue keeps its own ReflectMap impl (distinct sorted-vec type). Bridge conformance/e2e unaffected.
Emit `impl ReflectMessage` / `impl ReflectElement` on the owned message struct in vtable mode (parallel to the view impls), reading owned fields directly — String/Vec<u8>/Bytes, MessageField, Vec/HashMap containers, owned oneof enums, closed-enum has(). The owned Reflectable::reflect() body then becomes ReflectCow::Borrowed(self): reflecting an in-memory message costs no encode/decode round-trip (the interceptor use case), while bridge mode keeps the round-trip. The reflective handle still borrows self, so the call site is unchanged between modes. owned ReflectMessage overrides unknown_fields() to return the message's preserved unknowns (gated on preserve_unknown_fields), matching the bridge path — without it a recursive reflective walk would silently drop them. Tests: reflectable.rs now compares owned-vtable vs bridge reflection field by field (scalars, string, bytes, enum, nested message, repeated, map, optional, oneof) and asserts reflect() borrows; reflect_vtable.rs adds an OwnedView entry-point test. WKT views/owned types regenerated with the owned impls (reflect feature now pulls buffa-descriptor/std for the HashMap ReflectMap). Renamed reflect/view.rs -> reflect/containers.rs (it now holds owned containers too) and refreshed the now-inaccurate "vtable is deferred" docs.
Add a `ReflectMode` enum (buffa-codegen) and expose it as the canonical reflection selector: `buffa_build::Config::reflect_mode(ReflectMode)` and `protoc-gen-buffa`'s `reflect_mode=off|bridge|vtable` option. It maps to the underlying generate_reflection / generate_reflection_vtable config flags. `generate_reflection(bool)` stays as the bridge-or-off shorthand. The experimental, doc-hidden `buffa_build::Config::generate_reflection_vtable` method is removed in favor of `reflect_mode(VTable)`; the buffa-test, conformance, and benchmark build scripts migrate to it. Generated output is unchanged (same flags), so no regen.
Implement ReflectElement for SmolStr / EcoString / CompactString, gated behind new buffa-descriptor features (smol_str / ecow / compact_str) that forward to buffa's. This closes the gap where a `repeated <repr>` string field generated with string_type(SmolStr) + vtable reflection had no ReflectElement impl for its Vec<SmolStr> elements. A trait-bound blanket (e.g. impl<S: ProtoString> ReflectElement for S) is not possible: ProtoString is itself a structural blanket marker, and a trait-bound blanket collides with the scalar/Value impls under coherence (the same E0119 wall the message/enum element cases hit). So these are concrete per-type impls, one line each (AsRef<str> -> ValueRef::String). Only the repeated case needs them — singular string fields reflect via deref regardless of repr, and map string keys/values stay String. Covered by a new vtable_string_repr test (Labels with string_type(SmolStr) + reflect_mode(VTable)).
generate_reflection(true) (and protoc-gen-buffa reflection=true) now select ReflectMode::VTable — the fast path where reflect() borrows self — rather than Bridge. Bridge is opt-in via reflect_mode(ReflectMode::Bridge). Relax the codegen precondition: vtable mode required generate_views, but the owned impl ReflectMessage is self-contained, so vtable now requires only generate_reflection (for the descriptor pool). Views-off builds get owned-only vtable reflection (view impls skipped with the views) instead of an error. A new vtable_no_views test proves the owned-only path compiles and reflects.
|
All contributors have signed the CLA ✍️ ✅ |
This was referenced May 23, 2026
asacamano
previously approved these changes
May 24, 2026
Resolves conflicts from the idiomatic enum aliases (#152) and module collision fix (#145) landing on main while the vtable reflection stack was open: - enumeration.rs: keep both the vtable `ReflectElement` impl for closed enums and the idiomatic-alias doc-note wrapping of `enum_doc`. - codegen lib.rs: keep all three new config fields (generate_reflection_vtable, gate_reflect_on_crate_feature, idiomatic_enum_aliases); move the vtable-config validation into the new generate_with_diagnostics so both entry points enforce it. - buffa-build: keep both new builder methods. - buffa-test/build.rs: keep the new modcollide/modrace build blocks and the vtable-aware proto2 comment. Generated WKT/bootstrap code regenerated; matches the textual merge.
Propagates the origin/main merge (idiomatic enum aliases #152, module collision fix #145) up the stack. Sole conflict in codegen lib.rs: keep the two-function generate / generate_with_diagnostics split from view-vtable, but retain owned-vtable's relaxed vtable check (vtable reflection requires generate_reflection only, not views) and move it into generate_with_diagnostics. Doc # Errors updated to match.
iainmcgin
added a commit
that referenced
this pull request
May 26, 2026
First of three stacked PRs adding **vtable-mode reflection** — generated types implement `ReflectMessage` directly, so reflective field access reads struct fields in place with no `DynamicMessage` round-trip. This PR lands the **view-type** path and the shared plumbing. ## What's here - **Container reflection traits** in `buffa-descriptor`: `ReflectElement` (element → `ValueRef`) and `ReflectMapKey` (key → `MapKeyRef`), with generic `ReflectList` / `ReflectMap` impls over `RepeatedView` / `MapView`. `ValueRef::List` / `Map` now carry `&dyn ReflectList` / `&dyn ReflectMap`. - **View vtable codegen**: `impl ReflectMessage for FooView<'a>` (`get` / `has` / `for_each_set` / `to_dynamic`) plus a memoized per-message `MessageIndex`, behind an internal mode flag. - **WKT reflection**: a `reflect` feature on `buffa-types` so well-known-type views implement `ReflectMessage` (needed by any message that embeds a WKT). - **Conformance**: a new `BUFFA_VIA_VTABLE` run that decodes a view, walks its `ReflectMessage` surface to rebuild a `DynamicMessage`, and serializes that to JSON. - **Benchmarks**: vtable read cases added to the reflect bench. ## Validation - The `via-vtable` conformance suite passes all 1246 binary→JSON cases across proto2/proto3/editions with zero failures (12 `CONFORMANCE SUITE PASSED` lines total). - `cargo test --workspace --all-features`, clippy `-D warnings`, and markdownlint all green. ## Stack 1. **this PR** — view-type vtable + dynamic containers 2. owned-message vtable + `ReflectMode` public API + vtable-by-default (#149) 3. benchmarks, changelog, design docs, tooling (#150) Owned-type reflection still uses the bridge round-trip after this PR; that flips in PR 2.
main now carries the squash-merged #148 (view-type vtable) and #152 (enum aliases), both of which this branch already incorporates. All conflicts resolve to the branch side: this branch renames buffa-descriptor/src/reflect/view.rs to containers.rs (generalised for owned containers) and replaces the doc-hidden generate_reflection_vtable knob with the public ReflectMode selector, so main's copies of those are superseded. The merge introduces no content change relative to the previous branch head.
asacamano
approved these changes
May 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second of three stacked PRs on vtable reflection. Builds on #148. Adds owned-message vtable reflection and the public mode selector, and makes vtable the default.
What's here
impl ReflectMessageon the owned struct (parallel to the view impls), reading owned fields directly —String/Vec<u8>/Bytes,MessageField,Vec/HashMapcontainers, and owned oneof enums.Reflectable::reflect()now returnsReflectCow::Borrowed(self)in vtable mode, so reflecting an in-memory message costs no encode/decode round-trip.unknown_fields()is preserved (bridge parity).ReflectModeselector: a publicOff/Bridge/VTableenum, exposed asbuffa_build::Config::reflect_mode(...)andprotoc-gen-buffa'sreflect_mode=option.generate_reflection(true)(andreflection=true) now selectVTable;Bridgeis opt-in viareflect_mode(Bridge). The reflective call site is unchanged. Vtable no longer requires view generation — the owned impl is self-contained, so views-off builds get owned-only reflection.ReflectElementforSmolStr/EcoString/CompactString(feature-gated), so arepeated <repr>field reflects in vtable mode.Validation
OwnedView → &dyn ReflectMessageentry-point test plus views-off and string-repr tests cover the new paths.cargo test --workspace --all-features, clippy-D warnings, and the no_std builds are all green.Stack
ReflectMode+ vtable-by-default