Skip to content

F8: freeze the source-syntax classifier population in a committed ledger (#211) - #224

Merged
milyin merged 1 commit into
source-frontendfrom
frontend-boundary-ledger
Jul 28, 2026
Merged

F8: freeze the source-syntax classifier population in a committed ledger (#211)#224
milyin merged 1 commit into
source-frontendfrom
frontend-boundary-ledger

Conversation

@milyin

@milyin milyin commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Stage F8 of #215 (umbrella for #211), landed before F1 — on purpose.

#211's sixth completion criterion:

A mechanical check prevents new source-syntax classifiers from being added outside the frontend.

What a site is

A place that looks at captured Rust syntax and asks what shape is this. Real lines, all outside the frontend:

// core/types_util.rs:263            — "what kind of type is this?"
syn::Type::Reference(r) => vec![(*r.elem).clone()],

// lang/jnigen/jni/emit/wrapper.rs:516  — "is this a &T?"
InputKind::Handle { direct: true } if !matches!(arg_ty, syn::Type::Reference(_)) => {

// lang/cbindgen/builder.rs:598      — "is this a &[T]?"
let syn::Type::Reference(r) = ty else { .. };
let syn::Type::Slice(s) = &*r.elem else { .. };

Each is an independent decision about what the source Rust means — the decisions #211 says belong to core::frontend alone. #210 was two of them, in one file, disagreeing about [u8; <Holder>::N].

boundary.ledger records how many live in each file. boundary_ledger recomputes and fails if any count moved.

  • up — a new classifier landed outside the frontend. Move it in, or regenerate and justify it in review.
  • down — one was migrated. That is the goal, and it is still a ledger edit, so F3–F6's progress shows as a diff instead of passing silently.

Why before F1

As written, F8 was seeded from F1's classification of "the 189 sites". That ordering makes F1 a large document that rots the day after it is written, with nothing failing behind it.

Inverted, the ledger freezes the population now — growth stops immediately — and F1 becomes entries coming off the ledger during F3–F6. The allow-list is the classification, not a second copy of it. F1's checklist in docs/source-frontend.md is rewritten accordingly, and its first concrete task is now extending WATCHED one enum at a time, each addition a regenerated ledger whose diff is the decision.

Three load-bearing properties

The count is read off disk, not from the compiled crate. CI runs both cargo test and cargo test --all --all-features, and unstable-cbindgen gates the whole cbindgen suite — so a check that inspected the compiled AST would count a different population in each, i.e. give two answers in one CI run. That is #219's failure mode, and avoiding it is why this walks files.

It walks tokens, not lines and not an AST. A grep -c counts lines rather than occurrences (two on one line is common), counts the five inline #[cfg(test)] mod blocks — so adding a test would fail the check, which is how these ledgers die — and is defeated by one line: use syn::Type; then Type::Reference(_) greps as zero. A syn::visit::Visit walk fixes all three and opens a worse hole: it cannot see inside macro invocations, and a large share of the sites live in matches!(..). A token walk sees patterns, expressions, types and macro bodies alike, and is immune to rustfmt line-wrapping.

The header lists what the check cannot see. Classification that never writes a syn:: path is invisible to it: token-string classification (core/domain.rs matching to_token_stream().to_string() against "f32"), ident-name classification (seg.ident == "Option"), helper delegation (jnigen/jni/classify.rs is a whole classifier with zero watched sites), types_util::match_pattern unification against parse_quote!(_) patterns, and every syn enum outside WATCHED. A check that silently under-reports is worse than no check, so the gaps are named in the ledger header and are F1's queue.

Scope of WATCHED

&["Type", "Expr"] — exactly the population #211 and the branch map already track, and the low-noise one. Emitters construct syn::Item::Fn constantly but rarely construct a syn::Type, so adding Item today would churn the ledger on every emission change. Extending it is one line plus a regenerated ledger.

Honest caveat: a few counted occurrences build syntax rather than classify it — cbindgen/builder.rs:570 returns a syn::Type::Path, which is emission. Separating build from match mechanically costs real code, and those sites are few and stable. The number's job is to not move, not to be a precise census.

The seed number

196 sites across 28 files. The branch map's hand-counted 189 (67/97/25) was wrong in both directions — a recount by hand gives 69/96/26, a token count a third answer. Cross-checked per file against grep -o: types_util.rs 41 vs grep's 42, the difference being a syn::Type::Infer mention inside a doc comment, which is correctly not a classifier. A scoreboard nobody can reproduce is not a scoreboard; the ledger is now the authoritative number and the doc says so.

Verification

  • cargo test -p prebindgen boundary — the ledger test and a scanner test over fixture shapes: a match arm, a matches! body, a let .. else, two on one line, an aliased Type::Reference after use syn::Type, a use that must not count, a syn::Item::Fn outside WATCHED that must not count, and a #[cfg(test)] mod replace_ident_tests that must be skipped (that module is real — jnigen/jni/fold.rs:637 — so a name-based rule would miss it).
  • Proven to bite, not assumed: adding matches!(t, syn::Type::Never(_)) to jnigen/util.rs fails with api/lang/jnigen/util.rs: 1 -> 2; the same site written as use syn::Type; + Type::Never(_) fails identically, which is the hole the alias handling closes.
  • Feature-independent: cargo test and cargo test --all --all-features agree.
  • cargo test --all --all-features — green. clippy --deny warnings, fmt — clean.
  • ./examples/regen-check.sh — byte-identical. Per the branch's review protocol this stage is must not move; it adds no production code.

Test-only, no new dependency (proc-macro2 is already one), no new CI job — it rides both existing cargo test invocations.

🤖 Generated with Claude Code

#211's sixth completion criterion is a mechanical check that prevents new
source-syntax classifiers from appearing outside the frontend. This is it.

A site is a place that looks at captured Rust syntax and asks what shape it
is -- `syn::Type::Reference(r) => ..`, `matches!(ty, syn::Expr::Lit(_))`. Each
is an independent decision about what the source means, and #210 was two of
them, in one file, disagreeing. `boundary.ledger` records how many live in
each file outside `core::frontend`; `boundary_ledger` fails if any count moved.

Landed before F1, deliberately. As written F8 was seeded from F1's
classification, which makes F1 a document that rots with nothing failing
behind it. Inverted, the ledger freezes the population now and F1 becomes
entries coming off it during F3-F6 -- the allow-list is the classification
rather than a second copy of it. A count going down is also a ledger edit, so
a migration shows in the diff instead of passing silently.

Three properties are load-bearing:

* The count is read off disk, not from the compiled crate. CI runs both
  `cargo test` and `cargo test --all --all-features`, and `unstable-cbindgen`
  gates the whole cbindgen suite, so a compiled-AST check would give two
  answers in one CI run -- the failure mode of #219.
* It walks tokens. A grep counts lines not occurrences, counts the inline
  `#[cfg(test)] mod` blocks -- so adding a test would fail the check, which is
  how these ledgers die -- and is defeated by one line, `use syn::Type;` then
  `Type::Reference(_)`. A `syn::visit::Visit` fixes those and cannot see inside
  `matches!(..)`, where a large share of the sites live.
* The header lists what the check cannot see: token-string classification,
  ident-name classification, helper delegation, `match_pattern` unification,
  and the syn enums outside `WATCHED`. Those gaps are F1's queue.

The seed is 196 sites across 28 files. The hand-counted 189 in the branch map
was wrong in both directions; the ledger is now the authoritative number.

Test-only, no production code, no new dependency, no new CI job.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@milyin
milyin merged commit 7c56fc4 into source-frontend Jul 28, 2026
4 checks passed
milyin added a commit that referenced this pull request Jul 29, 2026
`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>
milyin added a commit that referenced this pull request Jul 29, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant