Cow<'_, T> is transparent, like Box<T> - #236
Merged
Merged
Conversation
`zenoh-flat`'s `zbytes_to_bytes(z: &ZBytes) -> Cow<'_, [u8]>` was refused by the closed flat API, so it would vanish when that crate migrates. The cause was an assumption in `lower_path`'s guard — "a builtin generic takes types only; a lifetime argument on one is not a shape this language has" — which skips the whole builtin match when any lifetime argument is present. `Cow` is the counterexample it did not anticipate: a builtin generic whose own signature includes a lifetime. So `Cow<'_, [u8]>` fell through to an undeclared nominal `Cow` and the item was refused. **A `Cow` carries nothing a destination language can see, and both adapters already say so in code.** cbindgen: "`Cow<'_, [T]>` → `T_wire* + size_t`. The C side receives an owned malloc'd copy, just like `Vec<T>` outputs", and `type_contains_vec` groups the two. jnigen: `env.byte_array_from_slice(&v)` — `&Cow<[u8]>` derefs to `&[u8]`, so there is no Cow-specific conversion at all — yielding Kotlin `ByteArray`, exactly what `Vec<u8>` yields. So `Cow<'_, T>` classifies as `T`'s own kind, the `Box<T>` treatment, and no `TypeKind` variant is added: the semantic surface says nothing about a fact no destination acts on. What codegen genuinely needs is the *spelling* — jnigen rewrites its generated fn's param type to `::std::borrow::Cow<'_, [u8]>` because "the param type must be resolvable without imports" — and spelling already travels in `origin`. Classify off `kind`, spell off `origin`, with both adapters' existing behaviour now predicted by the classification instead of special-cased. Transparent for any target, as `Box` is. Whether a `Cow` can actually cross stays the adapter's call, and both already restrict — cbindgen to scalar slices, jnigen to `[u8]` — refusing the rest with their own diagnostics. `std::borrow::Cow` joins the prelude, for the reason every entry is there: a name no source has to import. It also stops the frontend being *stricter* than the adapters, which tail-match the last path segment and so accept a qualified spelling — the cbindgen fixture `cow_u8_returns_scalar_array` writes exactly that, which is the proof the qualified form occurs. Verified the three new rows fail against the old guard before keeping them. Generation byte-identical, ledger unmoved, 48 JVM sections. zenoh-flat is a separate repo, so `zbytes_to_bytes` is covered by an acceptance row rather than by a build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
Reviewed The frontend change is appropriately narrow: normalization makes Validation on an isolated checkout: all 65 focused Looks good to merge from my review. |
This was referenced Jul 31, 2026
milyin
added a commit
that referenced
this pull request
Aug 4, 2026
…them (#211) (#229) * Map the Language integration program Umbrella document for making every prebindgen component consume `Element`s instead of parsing captured Rust itself: the design and the rule it turns on, the measured size of the problem (202 classification sites, 113 registry map reads), the stage order L0–L5, and the completion criteria restated from #211. This file is the authority on stage state; the umbrella PR body mirrors it. Refs #211. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Parse the record stream into elements that keep their syntax (#211) (#227) * jnigen: derive a return expansion from a value form (#213) (#221) * jnigen: derive a return expansion from a value form (#213 Gap A) `expand_return!(T).fields(fields!(t_to_struct))` takes T's output fields from its value form — the struct gathering its own accessors — instead of restating them. Two of zenoh-flat-jni's five hand-written lists had already drifted from the struct they mirror; a derived list cannot. `.fields()` is `.field()` applied to each struct field, so it keeps the same rule: a field crosses by ITS OWN type's default output boundary. A field type with an `expand_return!` splices it (a KeyExpr field still crosses as its string, not as a handle), a declared data class inlines, a field behind Option/Vec stays one leaf. Adopting it therefore preserves the boundary shape a hand-written list already had. Per-field adjustments live on the `FieldsDecl`, keyed on the Rust field ident like `FunctionDecl::expand_param`: `.field(name, expand_return!(..))` replaces one field's decomposition, `.name(name, "kt")` renames its leaf. Naming a field the struct lacks is a hard error — that is the drift this declarator exists to catch. Core changes: - `UnfoldLeaf.path` becomes `Vec<PathStep>` (`Call` / `Field`, each carrying its own optionality) so one path can mix accessor calls and field reads. Behaviour-preserving for every existing producer. - `DeconRecord::Fields` + `FieldRecord`; the adapter walks the struct (it knows which are declared classes), core decides per field whether to splice, and rides the existing visited/Cycle guard. - `UnfoldPlan.root_call` hoists the value-form call to one local, so the struct is built once per delivery rather than once per field. - `Prebindgen::deconstructors` now takes `&Registry`, matching `value_struct_decons` — a value form's fields come off the indexed struct. Sum-typed fields (ReplyStruct.result) are not covered yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * jnigen: a sum-typed field of a value form (#213 Gap A, sums) `ReplyStruct { result: ReplyResult, .. }` — a `sealed_class!` field of a value form now decomposes in place into its selector and one leaf group per alternative. A sum has no whole-value converter by construction, so this is the only shape in which it can cross at all. The user-facing callback still receives ONE typed `ZOutcome`: the tag and group slots collapse into a single parameter rebuilt by an inlined `when`, reusing the `GroupDesc` collapsing that a fixed-builder arg already uses. Handing the raw slots over would have defeated the `sealed_class!`. Generalizations, both behaviour-preserving for a sum in the whole-return position (its 20 existing tests are unchanged): - the selector leaf carries the sum's own type as its `out_ty`, so the emitter finds the enum to match on from the leaf rather than from `plan.source` — which names the CONTAINING value once a sum is a field; - `encode_sum_leaves` becomes `encode_sum_group`, taking one sum's leaf segment plus the expression to match on. `encode_plan_leaves` segments the leaf list and emits one match per sum instead of the whole plan being handed to the sum emitter; a whole-return sum is the degenerate case of one segment covering everything. `Vec<sum>` and `Option<sum>` fields are refused by name: the first has variable arity, the second would need a present flag beside its tag that an output leaf list cannot carry (the `fromParts` bridge's `PlanFieldKind::Sum` can, which is why a data-class field may be `Option<sum>`). Also restores examples/example-cbindgen goldens, which the previous commit picked up from an --all-features regeneration. The generator output is unchanged; only the committed artifact was wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * examples: restore example-cbindgen goldens to the plain-build variant An earlier `git add -A` in this branch swept in an --all-features regeneration, whose FEATURES guard reads "example-flat/internal example-flat/unstable" instead of "". `examples/regen-check.sh` builds with default features, so the committed artifact has to be the default-feature one — this is what CI checks. The generator output is unchanged either way; only the committed file was wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * covertest: exercise the derived value-form boundary on the JVM (#213) Library tests alone do not count as coverage in this repo, so `.fields()` gets a real round trip: `perftest_flat::ext::Report` is a handle whose output boundary is declared from its value form, with each field landing on a different rule of the expansion — summary a type with its own expand_return! ⇒ spliced into (count, total), NOT handed over as a handle taken Option<data class> ⇒ one leaf origin a non-optional data class ⇒ inlined into its fields outcome a sealed_class! ⇒ selector + one group per alternative, carrying a handle label a plain leaf `Test.kt`'s new section is itself the assertion: the callback signature would not compile if any field had been derived wrongly. It also pins the ownership contract for a handle reached through a value form and a sum group — live inside the callback, still live after, the receiver's to close. 47 sections pass on a real JVM. Adds the Gap B unit test the issue asked for: a handle-payload sum in DATA-CLASS FIELD position, the one position return/callback coverage did not reach. It works — and the test pins two consequences that were previously unstated: the container is NOT AutoCloseable (a sum payload is the receiver's to close, unlike a plain handle field, which cascades), and a sum field pushes its parent onto the whole-value fromParts bridge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * jnigen: address review on #221 — three value-form defects P1 — a single-leaf value form passed a borrow to an owned converter. One leaf makes core pick `Delivery::Return`, whose reach is composed separately in `emit/wrapper.rs`'s `is_convert` path. That path rendered a `Field` step as `&(expr).field` and returned it, so a plain field leaf — whose `out_ty` is the field type as written — got `&F` where its converter takes `F`, and a non-`Copy` field additionally borrowed out of the temporary the value-form call returned. It now clones the reached place, the same treatment `encode_plan_leaves` gives a `LeafSource::Field` leaf; an identity leaf stays borrowed, since its converter IS the borrowed-opaque clone. P2 — a per-field override did not validate its declared type. `.field("key_expr", expand_return!(ZBytes)...)` was accepted for a `ZKeyExpr` field whenever both were declared handles, and an override silently outlived an upstream field-type change — the exact drift `.fields()` exists to catch. The declared key is now compared against the peeled field type and names both, matching the target checks on the per-function expansion APIs. P2 — nested value forms were not hoisted. `root_call` only searched the declaration's top-level records, so a field splicing a child whose own boundary is also derived rebuilt that child once per child leaf, breaking the stated "called once per delivery" contract. Replaced by `UnfoldPlan.hoists: Vec<Vec<PathStep>>` — the path prefixes to bind once, recorded where `flatten` descends and therefore outermost-first. Each is composed from the longest already-bound prefix of itself, and each leaf reaches off the innermost hoist it sits under: let __vf0 = z_outer_to_struct(&arg); let __vf1 = z_inner_to_struct(&(&__vf0).inner); This also removes the single-value-form special case rather than adding a second one beside it. Three regression tests, one per finding. The only generated-output change is the `__vf` -> `__vf0` rename. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * jnigen: validate nested value-form field shapes * jnigen: consuming value forms — move the fields instead of cloning them `.fields(fields!(f))` now accepts a value form that takes its receiver BY VALUE. Such a form destroys the object into its parts, so the generated code moves the value in and moves each field OUT into its leaf — the clones the borrowing form pays disappear entirely. This is what the hot receive path wants and what zenoh itself recommends: `From<Sample> for SampleFields` exists, in zenoh's words, because it "allows deconstructing a sample to fields without cloning, which is more efficient than using getter methods". Every callback hands its value over owned (`impl Fn(Sample)`), so there is nothing to preserve — the borrowing form clones fields out of a value it is about to drop. Measured on covertest's `Report`: six clones removed from the callback body, `report_into_struct(__cb_arg0)` moved in, every field moved out. Consuming-ness is INFERRED from the accessor's signature, so it cannot drift from it, and both forms stay usable side by side. Because a consuming form moves the value, two shapes are refused at declaration time rather than emitted as Rust that cannot compile downstream: a sibling record (`.field_self()` or another `.field()` would read a moved value), and a form reached through another value form (it would move a field out from under the parent's other leaves). A `&T`-returning function clones once up front and consumes the clone, so one declaration still serves owned and borrowed returns alike. Two supporting changes: - The reach derivation is now SHARED (`reach_leaf_flat`) between the multi-leaf encoder and the single-leaf `Delivery::Return` shortcut in emit/wrapper.rs. Deriving it twice is what let them drift into the P1 defect; the shortcut also now refuses an optional intermediate step explicitly instead of composing code that cannot type-check. - Reaches project the leading run of plain field steps DIRECTLY (`&v.a.b`) instead of through a borrow of the base (`&(&v).a.b`). The two name the same value, but the second borrows the base as a whole, which the borrow checker rejects once a sibling leaf has moved another field out — so without this, field moves compiled only while the borrowing leaves happened to be declared first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * jnigen: `.fields_into()` — declare the consuming value form, and let it nest `6133f91` taught `.fields(fields!(f))` to accept a by-value accessor and INFERRED consuming-ness from its signature. That reads the decision off the wrong thing. Giving the value away is a boundary decision — the same one `.field_self()` makes, which is exactly why the two cannot coexist — not a property of which function happened to be named. So the collision surfaced as a resolve-time error phrased as a restriction on `.fields()`, when it is really two declarators to pick between. Now the decl says which it wants: .field_self() the value itself, whole .fields(fields!(to_struct)) a copy of its parts .fields_into(fields!(into_)) the value itself, as its parts `.fields_into(..)` must be the decl's only record — a `.field_self()` or a sibling `.field(..)` would read a value that is gone — and that is now a panic in the declarator, in BOTH orders, rather than an `UnfoldError` found a resolve later. The declared flag and the accessor's receiver are cross-checked when the records are flattened, so intent still cannot drift from the signature; naming the wrong one of a `to_struct`/`into_struct` pair is an error that says which declarator the accessor belongs to. The nesting refusal is GONE. Its stated reason — "it would move a field out from under the parent's other leaves" — does not hold: a hoisted value form is an owned struct, its fields are disjoint, and `project_leading_fields` (same commit) already stopped leaves from borrowing the base as a whole. So a nested consuming form is handed the parent's field BY MOVE: let __vf0 = z_outer_to_struct(&__cb_arg0); let __vf1 = z_inner_into_struct(__vf0.inner); // moved, not cloned … __vf0.tag … // sibling leaf, still fine `compose_step` borrows (`&(e).f`), so the field run to that field is projected in the hoist loop instead of going through it. A nested form reached through an accessor CALL holds a borrow with nothing to give up, so it clones once and consumes the clone — the same fallback a borrowed root already takes. That was the one place an available `_into_struct` went unused for no reason. Verified: 438 lib tests (three retargeted, five new — both collision orders, both signature-mismatch directions, and the nested move under a borrowing AND a consuming parent), covertest-kotlin's 47 JVM sections, regen-check byte-clean. The generated output for covertest is unchanged — same accessor, same moves; only the declaration that names it moved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * jnigen: address review on #221 — consuming ownership in two more places Two review findings, both cases where `.fields_into(..)` promised a move and the emitter did not deliver one. [P1] The single-leaf `Delivery::Return` shortcut never consulted the plan's hoists. It composed its reach straight off the raw value, so a one-field value form declared with `.fields_into(..)` emitted (&(myflat::z_one_into_struct(&__cvsrc)).label).clone() — `&ZOne` handed to a by-value receiver, ill-typed in the consumer's crate before you even reach the pointless clone. Every consuming test so far produced a MULTI-leaf callback plan and went through `encode_plan_leaves`, so nothing covered it. The hoist loop is now `bind_hoists`, shared by both paths, and `reach_leaf_flat` takes the rebased path plus its hoist's `consuming` flag. The shortcut binds the same `__vfN` locals as the multi-leaf encoder and reaches the leaf off the innermost one. That is the same fix that was applied to the reach itself in `6133f91` and for the same reason: two derivations of one question drift. [P2] The identity branch computed `consuming` and then returned before using it. Only a handle at the owned ROOT (empty path) moved; a handle FIELD always took the clone-via-converter arm: ZChild_to_jlong_...(&mut env, &__vf0.child) despite the parent form having given its value away — a preserved clone, and a `Clone` bound the handle type need not have. The branch now computes the owned PLACE (the root, or a plain-field run under a consuming hoist) and boxes it, `Box::into_raw(Box::new(__vf0.child))`. Both regressions reproduce the reviewer's exact shapes and both fail without the corresponding fix (verified by stashing each). Sum payloads, which the P2 comment also flagged, are NOT fixed here: filed as #228. `encode_sum_group` matches by reference and clones every payload kind through one chain, so moving means reworking that emitter's ownership model — the selector reads the same matched value, and an owned handle payload wants the identity branch's box rather than the borrowed-opaque converter. Not an addendum to this PR. Verified: 440 lib tests, covertest-kotlin's 47 JVM sections, regen-check byte-clean (neither shape occurs in covertest, which is why its goldens do not move — the unit regressions are what pin them). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * jnigen: decide leaf ownership in the plan, not in each emitter Two more review findings on #221, both the same defect wearing a different hat: an identity (handle) leaf under a consuming value form was still reached as a borrow, so the borrowed-opaque converter cloned it — and demanded a `Clone` the handle type need not have. * A value form whose SOLE field is a handle takes the single-leaf `Delivery::Return` shortcut. `bind_hoists` called the by-value accessor correctly, then the shortcut returned `&__vf0.child`, because its consuming case only covered `LeafSource::Field`. * An `Option<Handle>` field was excluded by the previous fix's plain-field test, leaving `match &(&__vf0).child { Some(__n0) => …clone… }` — an ordinary optional handle field, not the sum limitation of #228, and the commonest shape there is (`SampleStruct.attachment`). Patching each emitter would have been a third special case for one question. The question belongs to the PLAN: `place_is_owned` now decides, where an identity leaf's `out_ty` is chosen, whether the value at that path is the plan's to give away — the root of an owned plan, or a field of a form that CONSUMED its value, reached by a movable run of steps. An owned `out_ty` IS that statement, and it already selects the owning converter, so every emitter follows one decision instead of re-deriving it. `steps_are_movable` (plan.rs) is that run: field reads only, with an `Option` allowed on the LAST one — a `None` arm still hands the whole `Option` over by value, while an `Option` in the middle must be unwrapped and so can only be borrowed through. The resolver and both emitters read the same predicate; two readings would drift, and the disagreement is a borrow handed to an owning converter. Emitters then just project the place: * `reach_leaf_flat` moves whenever the leaf owns its `out_ty` — field and identity leaves alike. It keeps requiring a plain-field run, since return delivery has no `None` arm for a trailing `Option`. * The nullable identity branch matches the `Option` BY VALUE and boxes the `Some` payload, instead of matching a borrow of it. Both regressions reproduce the reviewer's shapes and fail without the fix (verified by stashing it). 442 lib tests, covertest's 47 JVM sections, regen-check byte-clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * jnigen: a nullable sole leaf is a callback delivery, not a return `single_return` chose `Delivery::Return` on leaf COUNT alone. A value form whose only field is an `Option<Handle>` therefore landed on the flat return path, which has no `None` arm and whose `convert_out_ty` names the leaf's own type rather than an optional of it — so it composed &(&__vf0).child into `ZChild_to_jlong(.., __out)`, typed for `ZChild`. The downstream crate does not compile. Making `out_ty` owned in 421531e addressed move-vs-clone; it says who frees the handle, not whether there is one. Absence is a DELIVERY question. Callback delivery already has the arm — the leaf crosses as a boxed `Long` or JVM null — so a nullable leaf goes there, which is one condition on `single_return` rather than teaching the shortcut to match and map a trailing option it has no way to represent in its return type. Nullability here only ever comes from an `Option` with something DECOMPOSED below it (a `.field_self()` handle, a nested value form); a plain leaf's own `Option` rides its converter and leaves the leaf non-nullable. So no shape that returns today stops returning — regen-check is byte-identical and covertest's 47 sections are unchanged. Regression reproduces the reviewer's shape and fails without the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * jnigen: an owned root identity moves on the flat return path too The flat return path asked the wrong question. It tied the move to the rebased hoist's `consuming` flag, but "a consuming form gave it to me" is only ONE of the two ways a leaf owns what it reaches. A plain `-> ZChild` return under the type-level `expand_return!(ZChild).field_self()` — the declaration that exists so the same boundary can be spliced as a value-form field — has no hoist at all, so `consuming` was false and the path emitted let __cvsrc = myflat::z_root_child_make(); { &__cvsrc } into the OWNING `ZChild_to_jlong`, whose argument is `ZChild`. Same mismatch inside the `map` closure of an `Option<ZChild>` return. For an identity leaf the plan already states ownership — that is what `place_is_owned` decides and what selected the owning converter — so the emitter reads it off `out_ty` instead of re-deriving it. A field leaf keeps asking the enclosing form, since its `out_ty` is the field type as written and owned either way. That predates this PR: the previous shape of this path composed `&base` for an empty path regardless. The callback emitter has always treated the owned root as an owned place; now both do. Regression covers the plain and the `Option` return and fails without the fix. 444 lib tests, covertest's 47 JVM sections, regen-check byte-clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * jnigen: rename `.fields_into()` to `.fields_self_into()` Puts the declarator squarely in the `field_self` family it belongs to, which is the whole point of it being its own declarator: `.field_self()` hands the value over whole, `.fields_self_into(..)` hands *the value itself* over as its parts, and `.fields(..)` hands over a copy of its parts. `self` is what the first two share and what makes them mutually exclusive. Mechanical: the method, the two panic messages, the doc links, the covertest declaration and its coverage-table row. Generated output is unchanged — regen-check byte-clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * Parse the record stream into elements that keep their syntax `Language` turns a captured `(syn::Item, SourceLocation)` stream into `Element`s: a closed, destination-neutral classification paired, at every level, with the exact syntax it was built from — the item, each parameter, field, variant and type. The pairing is the point. Issue #211 asks that adapters stop re-reading captured Rust, and the natural reading of that — a syn-free semantic model — makes the model responsible for reconstructing Rust too, because the generated glue is itself a destination artifact. That pressure is what turns a language-neutral IR back into a second `syn`: a delimiter, a lifetime and a literal's base all have to be modelled so they can be re-emitted. Keeping the original slice costs nothing and removes the pressure, so the classification stays small: Element::Enum → Variant { tag, discriminant: Option<i64>, fields, syntax } `B()` is a unit *group* and still spells `E::B()`, because `Variant::spell` reads the delimiters off `syntax`. `= 0x07` reaches a C header as `0x07` while Kotlin gets the number 7. Neither is a modelled fact. The rule for consumers is therefore: **classify off `kind`, spell off `syntax`.** #224's boundary ledger measures exactly that without adaptation — it counts variant mentions of `syn::Type` / `syn::Expr`, so `quote!(#slice)` is invisible to it and `matches!(ty, syn::Type::Reference(_))` is not. It is ported here and seeded at 202 sites, the population the adapter migrations pay down. Acceptance is preserved, not expanded. An item the language cannot express becomes `Element::Unsupported`, carrying its diagnosis: the pipeline has always scanned a signature only once an adapter declares it, and a source crate may mark items no binding uses. Only a duplicate name — which no declaration can disambiguate — fails the parse. Nothing consumes elements yet; `Registry::from_elements` is the next step. Ported from the #215 branch: the array-length subgrammar (#212), the type grammar and its acceptance tests, enum tag/discriminant numbering (#226), the ledger (#224). Refs #211. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Make the element model logical, not Rust-shaped `TypeKind` still named Rust type constructors where it should have named concepts, and the identity of a nominal type was a `syn::Path` sitting inside the classification — a position the boundary ledger cannot see. The test a variant has to pass is whether a *destination* language would act on the distinction; if only Rust can tell, it is spelling, and the syntax slice already carries it. Twelve variants become ten: * `Slice` folds into `Sequence`. `Vec<T>` and `[T]` are one concept — a run of `T` — and ownership is already the `Ref` layer's fact, so a second variant encoded it twice. This is what the pipeline does anyway: one `Shape::Iterable` covers both, and jnigen rewrites a `&[T]` input into the `Vec<_>` pattern. * `Boxed` goes. `Box<T>` **is** `T`: owned either way, and nothing outside Rust can tell. It classifies as what it wraps, and the `Box` survives where it matters — in the syntax generated Rust spells. * `Ptr` goes. No source crate writes a raw pointer, neither adapter has a selection arm for one, and accepting it *widened* acceptance, which this stage was not supposed to do. * `Str` covers `str`, so `&str` is a borrowed string rather than a reference to a nominal type nothing can resolve. It is the most common non-scalar parameter in the whole ecosystem, and both adapters already special-case it by name. * `Named` carries a `TypeId` — a name — instead of a `syn::Path`. The same test applied to the elements: a function's return is a `Type`, unit when elided, because no consumer distinguishes that from `-> ()` (eight of them normalize one to the other on the spot). A struct's fields are `Option<Vec<Field>>` — a product, or opaque — because named/unnamed/unit were three Rust shapes where `Variant` already modelled the same idea as a field list plus delimiters read off the syntax. `spell.rs` now holds everything that turns an element back into Rust tokens, so `element.rs` describes structure alone, and `Struct::spell` joins `Variant::spell` as the dual that makes the shapes unnecessary. Two things move to where they belong: `Language::parse` normalizes before lowering (`ty.rs` already assumed it had), and the callback grammar `extract_fn_trait_args` lives in the language rather than the registry — one ledger site paid down, 202 to 201. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Delete the passthrough element A `#[prebindgen]` crate marks the items that cross the boundary; the supporting code around them belongs to the consumer. The proc-macro already enforces that — marking a `use`, `mod`, `impl` or `macro_rules!` is a compile error at the mark site — so the variant's own doc listed items that could never reach it. What actually reached it was one thing: the `const _` feature guard, which is not a source item at all. `CfgFilter` synthesizes it and prepends it to the stream, so `Passthrough` existed to carry an item prebindgen itself wrote. It is a const, so it is modelled as one, and `Element::name` returns `None` for `_` — which is the real fact, and the one that lets several sources' guards coexist in the flat namespace. `write.rs` already had that rule for consts (`*ident == "_"` bypasses the declaration gate), dead until now because `const _` never reached the consts map. That leaves `union` and a type alias, the two kinds the macro accepts and the frontend does not model. Neither is written by any source crate in the ecosystem. They become `Unsupported` with a diagnosis naming the kind, rather than being copied verbatim into generated code that would reference source types by bare name — so the mark site and the frontend now disagree about exactly two kinds, and disagree loudly instead of silently. `Unsupported::name` becomes optional, since an item kind may have no identifier. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Give every node one Origin: its syntax, and where that syntax came from The classification is now logical, but its other half was still ad-hoc. `syntax` sat on nine node types as nine separate fields; `location` sat on the five item types only, because a captured record is per-item and a component has none of its own. That asymmetry had a cost. The one semantically load-bearing part of a location — the crate name — was reachable at item level only, so it got copied downward by hand, under a third field name, with drifting meaning: `ConstId.origin` is the crate a const was *declared* in, while `TypeId.origin` was the crate of the item *using* the type. The latter was also part of `TypeId`'s derived `Eq`, so `Sample` referenced from two source crates compared unequal — one type with two identities, three lines under a doc calling the name "the whole address". The two facts are orthogonal and neither derives from the other. `syn` tokens normally carry spans, but the proc-macro serializes each item as a string into JSONL and `build.rs` re-parses it, so every span in a slice points into an anonymous buffer; `SourceLocation::from_span` captures file/line/column while real rustc spans still exist, precisely because they cannot survive the trip. So every node now carries `Origin<S> { syntax: S, location: Rc<SourceLocation> }` — item, parameter, field, variant, type, and the array extent, which had no syntax at all and now spells its own length. Generic, so the typed slices survive; `Rc` because the model holds `syn` and is `!Send` regardless, the call `TypeKey` already made. One captured record is one item, so an item and every node lowered out of it share one allocation, which is both the honest answer to "where is this field" and the cheap one. With provenance arriving on its own, `item_crate: Option<&str>` stops being threaded through six lowering functions, `TypeId` is a name alone, and `ConstId.origin` becomes `ConstId.crate_name` — a crate that belongs to a *different* item, not this node's provenance. The rule, now stated where it can be read: a reference carries a name, the declaration carries the origin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * A variant's position is an index, not a tag `Variant.tag: i32` and `Field.index: usize` were one fact under two names: the ordinal of a child within its parent's ordered list. Sum versus product is already carried by *which* list it is — `Enum::variants` or `Struct::fields` — not by the number. The defence for keeping them apart was that a tag is transmitted while an index is only used to address a field. That defence was made of adapter behaviour: `i32` because cbindgen writes `c_int` and jnigen writes `jint`. Deciding a frontend field's shape from two generators' wire types is exactly the coupling this module exists to prevent, and it is the same test that stripped `Boxed` and `Slice` — a fact earns its shape from what the source means, not from what one adapter does with it. Transmitting the position to say which alternative is live is one destination's choice; another may send a name. The signedness had no defence at all: a declaration-order position is `0..N-1`. So `Variant.index: usize`, matching `Field.index`, and both documented as the same fact for the same reason — a node handed out on its own still knows where it sits. What remains genuinely distinct is `Variant::discriminant`: a position is where the source *put* a variant, a discriminant is the value Rust *assigns* it, and the two are independent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Address review: extent identity, callback returns, i64::MIN Three correctness fixes before this becomes the model later stages consume. **`ArrayExtent` had an equality that was neither identity it could have been.** It compared `value` and `source`, so `[u8; A]` differed from `[u8; 4]` when `A == 4` — one Rust type reported as two — while `[u8; 4]` equalled `[u8; 0x04]`, whose retained syntax differs. So it was not type identity and not spelling identity, and its own doc claimed the first while the code did neither. There is no single equality that could be right, because the extent answers three different questions, so it now provides none and each consumer projects what it needs: `value` for type and converter identity, `origin.syntax` for a C declaration's spelling at that occurrence, `const_id()` for which consts must reach the header. A regression pins all three apart — same value with different const dependency, same value with different spelling, same value with different const. The doc also records what a converter table will need: `value` being the identity means occurrences share one converter with differing spellings, so a canonical spelling must be chosen deliberately rather than inherited from whichever occurrence populated the entry. **The callback grammar silently dropped a return type.** `extract_fn_trait_args` read `ParenthesizedGenericArguments::inputs` and never `output`, so `impl Fn() -> u8 + Send + Sync + 'static` was accepted as `Callback { args: [] }`. `TypeKind::Callback` has no slot for a return and the grammar's own error text says a callback returns `()`, so the fact was lost — silently, which is worse than refusing. A non-unit return is now refused, a written `-> ()` still accepted, both with tests. No source crate in the ecosystem writes a returning callback, so nothing real narrows. The helper predates this PR, but making it the authoritative frontend classifier is what would have made the loss irreversible for every later consumer. **`i64::MIN` was not a discriminant.** `int_literal` parsed the magnitude as `i64` before applying the sign, so `-9223372036854775808` — valid Rust — failed at the digits. The magnitude is now parsed as `i128` and range-checked after negation, with a regression at the bottom of the range and one step past it. Along the way, `is_unit_type` becomes the language's one answer to "is this `()`", used by both the type lowering and the callback check. `types_util::is_unit` could not serve: it is gated behind `unstable-cbindgen`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Address review: async, variadic, generic binders, and a ledger hole Three more shapes the frontend accepted but could not represent, and one hole in the check that is supposed to catch exactly this class of thing. **`async fn` was the dangerous one.** `Function` has a direct return, so `pub async fn ping() {}` lowered as a function returning `()` — a generated wrapper would call it, drop the future, and export a function whose body never runs. A **C-variadic** tail was dropped from the signature just as quietly. Both are now `ItemError`s. **A type or const generic parameter is refused.** The elements have no generic binder, so a `T` in a field or parameter lowered as `TypeKind::Named` — an ordinary reference into the flat namespace, indistinguishable from a real item called `T`, which loses the scoping every downstream resolver needs. Modelling binders and substitution is the other option; refusing is the right one, because no destination language can express an uninstantiated parameter, and the source crates already write concrete types per instantiation. The diagnosis says so. Two things are deliberately *not* generic binders, both tested. A lifetime parameter: lifetimes are spelling and the spelling already travels, the same call `lower_type` makes for a lifetime argument. And `impl Trait` in argument position — Rust calls it an anonymous type parameter, but `syn` does not desugar it into the binder list, so the callback form every callback-taking source function uses is untouched. **The boundary ledger could be evaded.** `is_cfg_test` treated any predicate containing the ident `test` as test-only, so a classifier under `#[cfg(not(test))]` or `#[cfg(any(test, feature = "x"))]` was skipped — in a production build. It now matches the exact predicate `cfg(test)` and counts everything it cannot prove test-only, which is the safe direction for a check whose job is to stop a classifier hiding. `cfg(all(test, ..))` is genuinely test-only and is counted anyway; nothing in the tree writes one, and widening it later should be a deliberate edit with a ledger diff attached. The count does not move: every `cfg` on an item in the tree is either exactly `cfg(test)` or mentions no `test` at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Let Language read a source directory, not just a stream A build script's whole prebindgen preamble was two steps and a binding it did not otherwise want: let source = prebindgen::Source::new(zenoh_flat::PREBINDGEN_OUT_DIR); let registry = Registry::from_items(source.items_all())?; `Language` now folds the first step in, so naming the directory is enough: let elements = Language::new() .source(zenoh_flat::PREBINDGEN_OUT_DIR) .parse()?; That is five of the six consumer build scripts — zenoh-flat-jni, zenoh-flat-c, perftest-c, perftest-kotlin, example-cbindgen — which use nothing of `Source` but `new` and `items_all`. Reading a stream is kept, as the general case rather than the only one: `items()` takes any `(syn::Item, SourceLocation)` iterator, so everything a `Source` can express still composes — a group selection, a renamed dependency (covertest-kotlin's `crate_name` override, the sixth build script), several sources at once. `source()` is sugar over it. The other four knobs on `Source`'s builder — group selection and feature/target filtering — are reachable this way and were not mirrored, because no build script in the workspace calls them. The feeders accumulate and `parse` consumes, rather than each input being parsed as it arrives. That is forced, not stylistic: the rules that make a parse fail are whole-stream — one flat namespace, one const index an array length may reach into, one set of source modules to normalize against — so every input must be in hand before any of it is classified. A test now pins both directions of that: a length in one feeder resolving a const from another, and a duplicate name across feeders still failing. `Language` and `Element` join `Registry` in the `core` facade, since they are what a build script names; the rest of the element model stays in `core::language`, where an adapter reaches for it. The four doc examples on `Language` are now real doctests rather than `ignore` blocks — `Source::init_doctest_simulate` was already there to make that possible. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Split the two enum shapes: a Variant is not an Enum `Element::Enum` covered both a payload-carrying enum and a fieldless one, on the theory that the second is the degenerate first. They are two entities, and the evidence is in how they are numbered. A sum's alternatives are identified by **position**: cbindgen states it outright — "the mirror carries no explicit discriminants, so its tags are declaration order `0..N`" — and jnigen's sum emission mentions `discriminant` exactly zero times against eleven uses of the position. A fieldless enum's members are identified by the **value Rust assigns**: a C header re-states each `= expr`, and a Kotlin `enum class` entry is `NAME(7)`, with position only a fallback when the discriminant is not a literal. So one model covering both carried a field dead in each direction — and worse than dead on the sum side, because Rust *does* assign a discriminant to a payload alternative and using it would be wrong. The unified model invited exactly that mistake. Element::Variant(Variant { alternatives: Vec<Alternative> }) // a sum Element::Enum(Enum { values: Vec<EnumValue> }) // C-style `Alternative` carries `index` and `fields` and no discriminant; `EnumValue` carries `index` and `discriminant` and no fields. `discriminant_values` belongs to `Enum` alone now. `is_unit` and `first_payload_variant` are gone: the first was the classification, which `lower_enum` now makes once, and the second existed to name an offender to an adapter that only accepts fieldless enums — such an adapter matches `Element::Enum` and never sees the other shape. Both shapes still spell delimiters off their own syntax, because `A`, `B()` and `C {}` are fieldless alike and Rust demands the delimiters wherever the last two are named — so `spell` is on `Alternative` and `EnumValue`, over the one `spell::fields`. `enum E {}` and an all-empty-group enum are `Enum`; one field anywhere makes the item a `Variant`, and a sum may still mix empty and payload-carrying alternatives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * Flat: a resolved model with direct access, not a stream of elements (#232) * Rename the module to flat: these are the flat API's elements `core::language` modelled one thing and was named for another. What it parses is the **flat API** — the single flat namespace a `#[prebindgen]` crate exports — so `Language` becomes `Flat` and `api/core/language/` becomes `api/core/flat/`. Mechanical, and separated from the model changes that follow so those arrive as a readable diff. The boundary ledger's skipped-path constant and header move with the directory; the count does not change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Element is a function, a type, or a constant `Element` mixed two levels: `Function | Struct | Variant | Enum | Const` set type declarations beside functions and constants, when the kinds a binding distinguishes are a function, a type, and a constant. Types now group under `Element::Type`, and the type *reference* — which held the name `Type` — becomes `TypeRef`, so a declaration and a use site stop sharing a word. `Opaque` becomes the entity for a type whose contents do not cross, and it arrives two ways: * `#[prebindgen] pub type X = path;` — this **reverses** #227, where a marked alias was `Unsupported`. It is now how a handle enters the flat API deliberately: a foreign or crate-private type gets a name here without any claim about its contents. That is what makes the API closable, and it is the prerequisite for requiring references to resolve. * a marked tuple struct, whose fields no adapter has ever crossed — unchanged acceptance, now named for what it always meant. So `Struct::fields` drops its `Option`. `None` was the opaque case; an empty list now means the source wrote a struct with no fields, which is a different thing. `MaybeUninit<T>` joins the grammar as `TypeKind::Uninit`. It is a boundary concept — an out-parameter whose slot the caller supplies and the callee fills — and cbindgen already models it as exactly that, so this moves a classification out of the adapter and into the frontend, per #211. It is also the one foreign generic that no alias could name, a generic alias being a generic binder. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Flat resolves its references and answers by name Two changes that belong together, because the first is what makes the second decidable. **The model is addressed by name, not iterated.** `FlatBuilder` collects and `build` hands over a `Flat` — `function(name)`, `declared_type(name)`, `constant(name)`, `element(name)`, plus iterators over each kind. Names are unique across the whole model, so a name is a complete address, and that is what every later stage wants: an adapter asks what a declared name *is* rather than scanning a list. L1 carried this as a checklist bullet; it is really a property of the model. Two types rather than one, because a half-built model should not be the same type as a resolved one — `Source::builder()` sets the precedent. **References resolve at parse time.** A third pass walks every `TypeRef` — through `Option`, `Vec`, `&`, `Result`, arrays, callback arguments and generic arguments alike — and an item naming a type the flat API does not declare becomes `Element::Unsupported` with `ItemError::UnresolvedType`. Deferred, not fatal, like every other refusal: an item no binding declares stays harmless. This is what a marked type alias bought. A dangling name previously surfaced far downstream as an unresolved *converter*, from whichever adapter happened to look first — the "one fact, several authorities" #211 exists to end. Note the two remain distinct: resolution here says a name denotes something, while an adapter's resolver still decides whether it supplied a converter for it. A path-qualified name gets its own diagnosis, since `#[prebindgen] pub type foreign::Option = ..` is not a spelling that exists — marked items live in one flat namespace of bare names. Also: `Item::Type` no longer reaches the registry's passthrough. An opaque declaration states something about the API's surface and is not code to copy into the binding; its target is routinely crate-private, so re-emitting it would not compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Close the example flat APIs, and assert they stay closed Every type a marked signature named had to become a declaration for resolution to mean anything. Two idioms, chosen by what the type actually is rather than by its Rust shape: **A handle gets a marked alias.** `Storage`, the three callback handlers, `Token`, `TokenGc`, `Summary`, `Archive`, `Report`, `EscapeProbe`, `StorageError`, and example-flat's `Calculator` move into a private `handles` module, with `#[prebindgen] pub type X = handles::X;` at the top level. The alias is transparent, so every signature still says `Storage`. `Error` in both crates was already an alias and only needed the attribute — which is exactly the shape zenoh-flat's 26 zenoh re-exports will take. **A public newtype stays a marked struct.** `Millis`, `Celsius`, `Percent` and `Label` are not handles: they cross by `convert!`, and covertest-helpers both constructs them and reads `.0`. Hiding them behind an alias broke that downstream, which is the useful signal — a type alias names the type, not the tuple-struct constructor, and the constructor lives in the value namespace where the struct is defined. In-crate construction of the relocated handlers is qualified `handles::PayloadHandler(..)` for the same reason. Marking these as structs rather than aliases matters for a second reason: a marked struct enters `registry.structs`, and `write.rs` emits `on_struct` for any declared type there — so marking the *handles* as structs would have changed generated output. The alias route is invisible to the registry, which is why the goldens hold. **And the closure is asserted, not assumed.** covertest-kotlin's build script now runs `Flat` over both sources and fails if anything is unsupported. It is the right place: only there do the helper crate's references to perftest-flat's types resolve, since it cannot mark them itself. Verified by deliberately unmarking `Storage` — the build fails naming all twelve referencing functions and the fix. Generation is byte-identical (`examples/regen-check.sh`) and the JVM covertest passes all 47 sections. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Record L0.5 in the stage map The model is now indexed and resolved, which takes two bullets off L1 — elements indexed by name, and the entry point that shares one parser — and adds a prerequisite L0 did not have: the flat API has to be closed for resolution to mean anything. Also records what is left open: zenoh-flat and its two consumers are separate repos whose 28 unmarked types need the same treatment, and `Cow<'_, [u8]>` has no alias spelling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Take a slice, not a Vec reference, in the resolution pass `clippy::ptr_arg` under CI's no-default-features run: the pass only mutates elements in place, so a slice is the honest signature. My local checks used --all-features only; CI runs three clippy configurations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * An out-parameter is a mode of borrowing, not a type `TypeKind::Uninit` wrapped a type, but uninitialized-ness is a property of the **borrow**: my own doc said `MaybeUninit` is "only meaningful behind a `&mut`", which is the argument against modelling it as a type at all. So `Ref` carries the mode, and the `MaybeUninit` is absorbed into it: Ref { mode: RefMode, inner: Box<TypeRef> } enum RefMode { Shared, Exclusive, Out } `&T`, `&mut T`, `&mut MaybeUninit<T>` — one axis, three values, and `inner` is always the borrowed *value's* type. One variant fewer than the `mutable` flag plus a wrapper, and the combinations that mean nothing at a boundary can no longer be written down: uninitialized storage owned, returned or in a field promises nothing a destination language can use, and `&MaybeUninit<T>` promises a readable `T` that may not be one. Both are refused, each naming why. `Out` rather than `Uninit` because it names the boundary role every destination language has — C's `T *out` — which is the fact an adapter acts on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Address review: transitive closure, generic aliases, goldens, real index Four findings, all valid; two were mine in this PR. **Refusal was not transitive.** `resolve_references` snapshotted the initial declarations and validated everything against that fixed set, so refusing a type stranded its dependents: pub struct Broken { pub field: Missing } // refused pub fn use_broken(value: Broken) {} // survived anyway `Flat::resolve` then returned `None` for `use_broken`'s parameter, contradicting the one invariant the model promises. It now runs to a fixed point: each round drops the declarations it refused, and stops when a round refuses nothing. Chains of any length collapse, in either declaration order, because the declared set only ever shrinks — which is also why it terminates. Regressions cover the direct case both ways round, a four-link chain both ways round, a sound chain that must be left alone, and the invariant itself: every `Named` reachable from a surviving element resolves. **A generic type alias bypassed the binder refusal.** The `Item::Type` arm built an `Opaque` without calling `reject_generic_params`, so `pub type Handle<T> = hidden::Handle<T>;` was accepted as one declaration that `Handle<u8>` then resolved against — losing exactly the scoped-parameter distinction every other item kind refuses, and contradicting this PR's own argument that `MaybeUninit` needed grammar support *because* a generic alias is a binder. Type and const parameters are now refused; a lifetime binder stays accepted, as on every other kind. **The aarch64 goldens carried unrelated all-features output.** `git add -A examples` in the migration commit swept in pre-existing working-tree drift — `unstable_field`, `calculator_reset`, a non-empty feature guard — which is exactly the state 95fd753 had reverted, because committed aarch64 goldens represent a plain build. Restored from the base, and verified: a plain `cargo build --release -p example-cbindgen` on arm64 reproduces the base files byte-for-byte. CI is x86_64 and cannot see this pair, so it needed catching by hand. My "byte-identical" claim was wrong for that reason, not for the model changes. **`Flat` was not actually indexed.** It stored only a `Vec` and `element()` did `iter().find`, so every typed accessor and `resolve()` scanned — quadratic once later stages resolve in a loop, and not the "indexed by name" criterion L0.5 claims. Now a `HashMap<String, usize>` beside the elements: positions, so there is one copy of each element and source order stays available for iteration. Built after resolution, since refusing an item changes its kind but never its name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * A prelude, Extern instead of Opaque, and no args (#235) * A prelude, Extern instead of Opaque, and no args Three things the same question kept surfacing: what does the language know without being told, and what must it be told? **Path reduction had one rule and a std special case; now it has one rule.** `reduce_flat_path` already reduced `crate`/`self` and any source-module prefix — a path into the flat namespace collapses to its bare name. Bolted on was a five-entry whitelist of std paths. Naming what that whitelist is removes it: those are **aliases the language pre-declares**, a prelude in exactly Rust's sense. A crate need not write `use std::vec::Vec`, and need not write `#[prebindgen] pub type Vec = std::vec::Vec` either, for the same reason. So the mechanism is an alias map from path to name, seeded from `PRELUDE` and extended with every alias the ingested crates declared — because a prelude entry and a hand-written alias say the same kind of thing. That generalises past std: given `#[prebindgen] pub type Session = zenoh::Session;`, a signature may now spell `&zenoh::Session` and reach the declaration. `foreign::Option<u8>` is still not `Option<u8>`, because the key is the whole path, never a final segment. `Normalization` holds what to reduce against, replacing the module-gathering loop `FlatBuilder::build` and `Registry::from_items` each wrote separately — they cannot normalize differently now. Two traps found on the way. A marked alias must be excluded from the normalization it defines, or `pub type Duration = std::time::Duration` becomes `pub type Duration = Duration`. And the prelude's entries are *generic*, so an early "reduce only without type arguments" guard broke `std::vec::Vec<Foo>`; the guard was also unnecessary, since a full-path key cannot collide. `mem::MaybeUninit` joining the prelude is a bug fix. It was a grammar builtin that was **not** reducible, so it worked only because perftest-flat happens to `use` it; written `&mut std::mem::MaybeUninit<Payload>` it became an unresolvable nominal type and silently refused the item — and `maybe_uninit_inner`'s comment claimed normalization had already reduced it. One test row per prelude entry now pins both spellings to the same kind, which is how that class of drift gets caught. **`Opaque` becomes `Extern`, and carries what it points at.** It was never only handles: `pub type Duration = std::time::Duration` crosses by value through a `convert!`, erased to an integer. What the frontend knows is narrower and truer — this name is in the flat API and its contents are not modelled — and the adapter decides the rest. `target` is now a modelled fact, so an adapter can recognise `std::time::Duration` without taking syntax apart, and reduction uses it. Deliberately not classified as std-vs-foreign: `pub type Error = zenoh::Error` IS `Box<dyn std::error::Error + Send + Sync>`, so std-ness is a property of the spelling, not the type. A rule keyed on the path root would answer differently for one type depending on who aliased it. **`args` is gone from `Named`.** A reference is a name. Nothing could read retained arguments: a surviving reference resolves to a declared type, and no declaration takes type parameters, so `Foo<u8>` against a declared `Foo` would not compile in the source crate. They are still lowered, so a bad type inside one is diagnosed — the dropped test row asserted a shape real source cannot produce. The boundary ledger gains one site in `types_util` for reading an alias's target; L2 reclaims it when the frontend owns normalization outright. Generation is byte-identical and the JVM covertest passes all 48 sections. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Box the array extent, the size outlier among the kinds `clippy::large_enum_variant` under `-D warnings`: an `ArrayExtent` carries an `Origin` over its length expression, so `Array` towered over the second-largest variant once `Named` lost `args`. The lint compares those two, which is why shrinking one variant surfaced another's size. Boxed rather than allowed — an array is the rare kind, the same trade-off `Unsupported::error` already makes for the same reason. My local clippy runs missed it because they omitted `-- -D warnings`, so it was a warning my filter did not match. CI passes that flag in all three configurations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Address review: an alias key is a whole type, and never shadows the grammar `path_key` dropped **all** generic arguments, so `type Bytes = std::vec::Vec<u8>` keyed on `std::vec::Vec` — overwriting the prelude entry, since the alias pass runs after the seeding. Reduction then swapped the ident and kept the use site's arguments, so `Vec<String>` became `Bytes<String>`, `Named` discarded the argument, and an unrelated parameter stopped being a `Sequence`. Any concrete alias could do this to any prelude entry. The root cause is a constraint I had not stated: normalization decides which spellings denote **one type** (issue #95, "the canonical flat-namespace spelling"), so it may choose a canonical spelling but must never change what a type *means*. `zenoh::Session` → `Session` preserves the kind. `Vec<u8>` → `Bytes` turns a sequence into an extern — retyping, not canonicalizing. Naming what the two kinds of alias are makes the fix structural rather than a patch. They **partition** the targets, because a target either has a grammar meaning or it does not: * the prelude, over targets the grammar models. Each names a **constructor**, so arguments are ignored when matching and preserved when rewriting — `std::vec::Vec<Foo>` is `Vec<Foo>`. * a crate's aliases, over targets it does not. Each names one **complete type**, so the key keeps type arguments (lifetimes still dropped, since a lifetime is spelling) and a match replaces the whole type — an alias name carries no arguments of its own. So an alias to something the grammar already models is not a reduction rule: the prelude owns that path. `type Bytes = Vec<u8>` stays a perfectly good name for an `Extern` — a bare path is never reduced, so `Bytes` resolves — while `Vec<u8>` keeps meaning a sequence and `Vec<String>` is untouched. Duplicate targets now resolve deterministically: first declaration wins, rather than last-in-stream. Two regressions, both verified to fail against the old behaviour before being kept: the reported case verbatim, and two concrete aliases over one foreign constructor staying distinct. The partition is documented where each half lives — the equivalence rule list and `Extern`'s own doc, including the asymmetry that an alias is an `Extern` always but a reduction rule only sometimes (`type Error = Box<dyn Error>` has no rule at all). Generation byte-identical, ledger unmoved, JVM covertest 48 sections. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * An alias is a one-way road, not an equivalence The review found that `alias_key` kept only `GenericArgument::Type`, so `type Small = zenoh::Wrap<4>` and `type Big = zenoh::Wrap<8>` still collided on their const arguments. Retaining every non-lifetime argument would fix that instance, but the key shape was never the real problem. Normalization decides which spellings denote **one type**. An alias does not create such a spelling — it brings a foreign type *into* the flat API under a new name. That is a one-way road: the name is thereafter the only way to spell the type here, and `zenoh::Session` in a signature stays refused even when `type Session = zenoh::Session` is declared. The diagnosis already said exactly that — "Give the type a name here with `#[prebindgen] pub type <Name> = ..;` and refer to that" — so alias reduction was weakening a rule the language already had. Treating it as an equivalence is a category error, and the two reported bugs are symptoms of it: `Vec<u8>` ≡ `Bytes` turns a sequence into an extern, and once one path can stand for two types, key shape decides which — arguments, const arguments, associated bindings, each a new way to collide. Removing the equivalence makes that class unreachable rather than patched. So a crate's `pub type` is a declaration only, and the prelude alone reduces: `std::vec::Vec<Foo>` is `Vec<Foo>`, because those *are* one type. The prelude and a crate's aliases stop being "two kinds of alias" needing a partition — different mechanisms with different jobs, which is the simpler answer to how they relate. Net −133 lines: `alias_key`, `type_args`, the alias map, the alias-collection pass, the first-declaration-wins tie-break, and the circularity guard that stopped an alias rewriting its own target all go. The boundary ledger returns to 205 — the site the previous commit added was reading an alias target, and nothing does that now. Nothing real depended on it: no marked signature in zenoh-flat or the examples spells a qualified alias target. Verified byte-identical generation and 48 JVM sections. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * Cow<'_, T> is transparent, like Box<T> (#236) `zenoh-flat`'s `zbytes_to_bytes(z: &ZBytes) -> Cow<'_, [u8]>` was refused by the closed flat API, so it would vanish when that crate migrates. The cause was an assumption in `lower_path`'s guard — "a builtin generic takes types only; a lifetime argument on one is not a shape this language has" — which skips the whole builtin match when any lifetime argument is present. `Cow` is the counterexample it did not anticipate: a builtin generic whose own signature includes a lifetime. So `Cow<'_, [u8]>` fell through to an undeclared nominal `Cow` and the item was refused. **A `Cow` carries nothing a destination language can see, and both adapters already say so in code.** cbindgen: "`Cow<'_, [T]>` → `T_wire* + size_t`. The C side receives an owned malloc'd copy, just like `Vec<T>` outputs", and `type_contains_vec` groups the two. jnigen: `env.byte_array_from_slice(&v)` — `&Cow<[u8]>` derefs to `&[u8]`, so there is no Cow-specific conversion at all — yielding Kotlin `ByteArray`, exactly what `Vec<u8>` yields. So `Cow<'_, T>` classifies as `T`'s own kind, the `Box<T>` treatment, and no `TypeKind` variant is added: the semantic surface says nothing about a fact no destination acts on. What codegen genuinely needs is the *spelling* — jnigen rewrites its generated fn's param type to `::std::borrow::Cow<'_, [u8]>` because "the param type must be resolvable without imports" — and spelling already travels in `origin`. Classify off `kind`, spell off `origin`, with both adapters' existing behaviour now predicted by the classification instead of special-cased. Transparent for any target, as `Box` is. Whether a `Cow` can actually cross stays the adapter's call, and both already restrict — cbindgen to scalar slices, jnigen to `[u8]` — refusing the rest with their own diagnostics. `std::borrow::Cow` joins the prelude, for the reason every entry is there: a name no source has to import. It also stops the frontend being *stricter* than the adapters, which tail-match the last path segment and so accept a qualified spelling — the cbindgen fixture `cow_u8_returns_scalar_array` writes exactly that, which is the proof the qualified form occurs. Verified the three new rows fail against the old guard before keeping them. Generation byte-identical, ledger unmoved, 48 JVM sections. zenoh-flat is a separate repo, so `zbytes_to_bytes` is covered by an acceptance row rather than by a build. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * L1: Registry consumes Flat (#238) * Make every test fixture self-sufficient Preparation for L1, where `Registry` consumes `Flat` and an item naming a type the flat API does not declare stops being ingested. 167 of 524 tests held such an item; this makes them all declare what they name, verified against a temporary `#[cfg(test)]` check inside `from_items` that the next commit deletes. **`declare_referenced`** appends a marked alias for every nominal type a stream names but never declares, to a fixed point. Most fixtures are *about* a plan shape or a converter, and a handle declaration is noise in them — `reg_with(&["fn get(s: &Storage) -> Payload"])` is testing an unfold plan, not what `Storage` is. Declaring those as `Extern`s is what a real source crate does for a foreign handle, and it is inert either way: a type alias lands in no registry map. `reg_with` now parses `syn::Item`, so a fixture *can* declare its own types when that is the subject. Four things the helper cannot cover, each a real correction: **`std::time::Duration`** was spelled path-qualified in 15 places. A qualified name can never be a flat-API name, so those fixtures now declare `Duration` and spell it bare — the shape a real source crate uses. That moves the `TypeKey`, so the matching `convert!` and two generated-name assertions move with it. **Two array-length "qualification" tests** asserted that `Holder::N` and `array_len()` lengths get qualified. The subgrammar was narrowed to "an integer literal or the bare name of a marked const" in #212, so neither can reach an adapter any more; they survived only because `from_items` never validated lengths. Reduced to the form that can. (jnigen's qualifier still handle…
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes the
Cowitem from #235's "noted, not fixed" list, now that #235 has merged.The problem
zenoh-flat's byte accessor was refused by the closed flat API:
lower_path's guard says "a builtin generic takes types only; a lifetime argument onone is not a shape this language has" and skips the entire builtin match when any
lifetime argument is present.
Cowis the counterexample that assumption did notanticipate — a builtin generic whose own signature includes a lifetime — so
Cow<'_, [u8]>fell through to an undeclared nominalCowand the item becameUnsupported.zbytes_to_byteswould have vanished when zenoh-flat migrates.A
Cowcarries nothing a destination language can seeBoth adapters already say so, in code:
Cow<'_, [T]>→T_wire* + size_t. The C side receives an ownedmalloc'd copy, just like
Vec<T>outputs" (cbindgen/emit.rs:677), andtype_contains_vec = is_vec(ty) || cow_slice_elem(ty).is_some()(mod.rs:605)groups them outright.
env.byte_array_from_slice(&v)(jni/trait_impl.rs:159).&Cow<[u8]>auto-derefs to
&[u8], so there is no Cow-specific conversion at all, and theKotlin type is
ByteArray— exactly whatVec<u8>produces.So
Cow<'_, T>classifies asT's own kind, theBox<T>treatment, and noTypeKindvariant is added: the semantic surface stays silent about a fact nodestination acts on.
What codegen genuinely needs is the spelling. jnigen rewrites its generated fn's
param type to
::std::borrow::Cow<'_, [u8]>because "the param type must be resolvablewithout imports", and
Cow<'_, [u8]>is not interchangeable withVec<u8>in a Rustsignature. Spelling already travels in
origin— so this is exactly classify offkind, spell offorigin, and both adapters' existing behaviour becomes predictedby the classification rather than special-cased.
Transparent for any target, as
Boxis. Whether aCowcan actually cross staysthe adapter's call, and both already restrict — cbindgen to scalar slices, jnigen to
[u8]— refusing the rest with their own diagnostics. That is #211's division oflabour: the frontend says what a type is.
std::borrow::Cowjoins the preludeFor the reason every entry is there: a name no source has to import. It also stops the
frontend being stricter than the adapters — both tail-match the last path segment,
so they accept
std::borrow::Cow, and the cbindgen fixturecow_u8_returns_scalar_arraywrites exactly that spelling. That fixture is the proofthe qualified form occurs in practice, and it still passes unchanged.
Tests
Three rows, each verified to fail against the old guard before being kept:
a_cow_is_what_it_borrows— the property the treatment rests on:Cow<'_, [u8]>andVec<u8>classify identically;Cow<'_, str>is aStr; theCowsurvives inorigin.syntaxso a signature can still be spelled; transparent for a non-scalartarget too; and a lifetime argument on any other builtin is still refused
(
Vec<'a, u8>→ an undeclared nominalVec), so the exception is exactly one namewide.
a_cow_returning_accessor_resolves—zbytes_to_bytes's real signature, which nowsurvives with a
Sequencereturn still spelling itsCow.Cowrow, spelledCow<'_, [u8]>so it is validRust.
Verification
524 tests and 18 doctests; clippy clean in all three configurations with
-- -D warnings;examples/regen-check.shbyte-identical; boundary ledgerunmoved (the new arm matches a name, not a
synvariant); JVM covertest 48sections.
zenoh-flat is a separate repo and not yet migrated, so
zbytes_to_bytesis covered bythe acceptance row above rather than by a build.
Noted, not fixed
cow_bytes_outputand cbindgen'scow_slice_elemstay as adapter-side syntaxmatching, which L3/L4 will migrate to reading
origin.syntax. They are the model fora framework-level std type.
Cowas an input is unmodelled in practice — no source writes one — the sameposition
Box<String>is in.originextension; deliberately deferred.Refs #211. Umbrella: #229.
🤖 Generated with Claude Code