Umbrella: one Rust frontend and a common source IR for all generators (#211) - #215
Draft
milyin wants to merge 6 commits into
Draft
Umbrella: one Rust frontend and a common source IR for all generators (#211)#215milyin wants to merge 6 commits into
milyin wants to merge 6 commits into
Conversation
The integration branch for #211. One document: what has landed, what has not, and the order — #211 stays the authority on the invariants and completion criteria, so this does not restate them. Records the two things the issue text leaves implicit. First, the scale: 189 `syn::Type::`/`syn::Expr::` match sites outside tests and the frontend, split 67 core / 97 jnigen / 25 cbindgen. Not all are source-syntax classifiers — separating those from legitimate inspection of adapter-synthesized types is F1's first job, so the number is a scale marker, not a target. Second, that #211's steps are independently landable, unlike #187's. The branch is therefore about keeping one reviewable diff for a change whose value is structural, not about hiding half-migrated states — and any stage can be re-pointed at `main` if its own fix becomes urgent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#212 began `SourceModel` ahead of its place in the order. The reason belongs in the map, because it generalizes: a C header must be able to emit `uint8_t tag[MARKER_TAG_LEN]`, and the only honest way to carry "this extent named that const" to an adapter is a node in the IR. Stashing the original `syn::Type` for the adapter to re-read would have violated invariants 5 and 6 in one move. So whenever an adapter turns out to need a source fact, the answer is to model the fact — never to widen what syntax reaches the adapter. F5 loses its "cbindgen has neither array support nor an explicit refusal" item, which #212 settled, and gains a sharper one: the struct-field path reads the model but still asks `is_scalar`/`is_string`/`is_vec` of the `to_syn()` projection. That re-derivation is precisely the duplicate F5 exists to delete. The site table is now per stage, and shows the count did NOT go down: 67/97/26 against 67/97/25. #212 added the model without removing what it will replace, so until F5/F6 the table measures duplication that exists rather than progress. Reporting one shrinking number would have implied otherwise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Jul 28, 2026
… extents to C (#210, first step of #211) (#212) * Lower array lengths in one frontend walk, not two adapter walks Array-length handling had two independent walks over the same expression: a whitelist in jnigen deciding what was ACCEPTED, and a rewriter deciding what it could QUALIFY. Nothing tied them together, so they drifted eight times (#210). The eighth was still open — the whitelist accepted every `Expr::Path`, the rewriter skipped paths with a `qself`, so pub qualified_assoc: [u8; <Holder>::N], was accepted and emitted verbatim into a crate where `Holder` is not in scope. Fixing that shape alone leaves the mechanism that produced #1-#8 intact. Both walks are replaced by one fallible lowering in a new `core::frontend`: lower_array_len(&Expr, &NameIndex) -> Result<ArrayLen, UnsupportedArrayLen> with the contract that `Ok` means the length was fully understood AND fully resolved. "Accepted" is now a consequence of lowering rather than a parallel judgement, so accepted-but-unqualified is not representable. `ArrayLen` is the one closed representation every consumer reads. It runs at INGEST, as pass 3 of `Registry::from_items`, where the name index is complete. So the decision is made before any adapter exists — both generators refuse the same input with the same message — and no emit-time length pass remains: `reject_unsupported_array_length`, `QualifyLengthPaths` and the `length_names` map are deleted, along with the comment block recording which defect each guard came from. The grammar narrows to a literal or a plain path. Const arithmetic (`[u8; A + 1]`, `[u8; A as usize]`) and `const fn` calls (`[u8; array_len()]`) leave the language; hoist the value into a named `const`. This is an intentional breaking change with a small blast radius — `[T; N]` crossing arrived in #209 and no released binding depends on it. `zenoh-flat`'s `[u8; ZENOH_ID_MAX_SIZE]`, the one real use, is a free-const path and is unaffected; regenerating zenoh-flat-jni changes converter names and one diagnostic string, nothing on the Kotlin surface. Three variants rather than the two #211 sketches: a path naming nothing the registry indexes (`usize::MAX`) is legal and must be emitted verbatim. That was a silent fallthrough in the old rewriter; `ExternalConst` makes it explicit, so lowering stays total instead of reintroducing a guess. Two shapes change behavior beyond the narrowing. A `qself` is now an explicit rejection naming the offending sub-expression — #210 asked for the decision to be made either way, and silent acceptance was the one option it ruled out. And `crate::MAX` now resolves like the bare `MAX`; previously its leading segment missed the name set and it was emitted as `crate::MAX`, which names the CONSUMER's crate. Stripping the source head is deliberately not the type-path rule, which reduces to the final segment and would collapse `myflat::Holder::N` to `N`, losing the owner of the associated const. Tests replace the eight characterization tests with a table: accepted spelling -> lowered value -> emitted spelling, and refused spelling -> reason. A new shape is a row. The two jnigen tests that only an end-to-end run can prove survive — that qualification reaches emitted code and does NOT touch a converter body's identically-named locals. The covertest fixture sizes `Arrays::bytes` by a `#[prebindgen]` const instead of a literal. That the covertest crate COMPILES is the check: a literal would prove nothing about resolving a path a different crate can name. `docs/source-language.md` writes down the accepted subset — #211 step 1 — and is explicit that only this row has moved into the frontend so far. Closes #210. First step of #211. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Resolve a length path by its item, not by its first segment Review finding on #212. `strip_source_head` dropped a `crate`/`self`/source-crate head and then assumed the next segment was the semantic anchor. That assumption holds only if accepted paths contain no intermediate modules, which was never stated and is not true: mod limits { #[prebindgen] pub const MAX: usize = 4; } pub bytes: [u8; crate::limits::MAX], `limits` is not indexed, so this became `ExternalConst { limits::MAX }` and the generated crate emitted an unqualified `limits::MAX` — unresolvable there, or worse, bound to a consumer-side module. It also contradicted the `ExternalConst` contract outright: the head was stripped BEFORE the path was classified, so a path documented as verbatim was not. The fix is the flat namespace. Prebindgen items are uniquely named and reachable as `<origin crate>::<bare name>`, so a module prefix inside the source crate carries no information — the bare name already identifies the item. Lowering becomes two ordered decisions: 1. Is the path source-relative? Its head is `crate`, `self`, a source module, or an indexed name. Everything else is external and is returned untouched. Classifying FIRST is what makes the verbatim guarantee true; it also keeps `other_crate::Holder::N` alone even though `Holder` names an indexed item, the same rule `normalize_type` applies to foreign type paths. 2. Which segment is the item? The leftmost that names one. Everything before it is module path and is replaced by the origin module; everything after is relative to the item and kept. So `MAX`, `crate::MAX`, `myflat::MAX`, `crate::limits::MAX` all lower to one value — which is the property that was missing, not just a missing branch. Leftmost, not rightmost: in `Holder::N` with a free const `N` also indexed, `Holder` is the anchor and `N` is its associated const. A `Leaf` item — a const or fn, through which nothing is reachable — is skipped when segments follow it, so a const sharing a module's name cannot capture the path. That is the only collision Rust permits, modules and types sharing one namespace but consts sitting in another, and it is a matrix row. A source-relative path naming no indexed item is now a hard error rather than silently verbatim: it claims a source item, so emitting it into a different crate is exactly the misinterpretation this pass exists to prevent. The bare spelling stays `ExternalConst` — indistinguishable from an external namespace — and the docs now say so instead of implying one rule. This makes explicit an invariant that was already load-bearing for TYPES: `normalize_type` reduces `crate::a::Foo` to `Foo` and emission qualifies it to `myflat::Foo`, so a nested item has always had to be re-exported at the source crate root. zenoh-flat already does — `ZENOH_ID_MAX_SIZE` lives in `base::config::zenoh_id` and `lib.rs` re-exports it. Verified the anchor scan is load-bearing by restoring the first-segment rule: the matrix fails. Generated output byte-identical; covertest PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * An array length is a number, not a path Rereview of #212 found a two-source provenance hole: one registry-wide name index, no origin for the item being lowered, so `crate::limits::MAX` in source A — with `MAX` unmarked there and marked in source B — anchored to B and silently changed the length from 4 to 8. It also found relative module paths to be irreducibly ambiguous without indexing modules, and the constructor unusable through the public facade. All three dissolve rather than get patched, once the requirement is stated properly: a length must be a KNOWN NUMBER. A generator runs in build.rs and cannot evaluate Rust, and a destination language that groups a small array into scalars needs the count literally — a Kotlin surface cannot reference a Rust const at all. So the grammar is an integer literal, or the BARE NAME of a `#[prebindgen]` const whose own initializer is an integer literal. The frontend reads the value and emits the number. That is a real narrowing, and it is the point: * module paths go, because a marked item is uniquely named and the bare name is its whole address — `crate::limits::MAX` only restated it, and `limits::MAX` was never distinguishable from a foreign crate path; * associated consts go, because prebindgen never captures `impl` blocks, so `Holder::N` could only ever have produced a path, never a number; * external paths go, and `usize::MAX` shows why — it is not even a fixed value; * an UNMARKED const is now a hard error rather than emitted verbatim. The generated crate sees only what the macro exposed, so this was never a qualification problem: the item does not exist downstream. Provenance is then a one-line rule instead of a lowering context: a bare name must be a const marked in the item's own source crate. Uniqueness holds across the marked namespace only, so without it the review's example still binds to the other crate's value; with it, that is unrepresentable. Origin-less streams are one anonymous crate and match trivially. Emitting the number is what removes the defect class rather than fixing it. There is no path left in generated code, so there is nothing to qualify, no namespace to get wrong, and no re-export invariant to rely on. A const length and the same number written literally become one type and one converter — which they always were in Rust. And a changed const now shows in the diff of a committed generated artifact, where echoing the name showed nothing at all. API surface (finding 3, my call): the lowering machinery is crate-private and the public frontend is the decided model plus its diagnostics. `NameIndex`, `ItemRole` and `ArrayLenResolver` are gone from the facade rather than completed; the entry point is `Registry::from_items`, and offering a second route to a partly-built model is what #211 exists to prevent. `Registry::named_item_idents`/`named_items` go with them — they existed only for the machinery this replaces. The restriction is on LENGTHS, not on consts: a `#[prebindgen]` const may still be computed however you like. Generated output is byte-identical to main apart from the new fixture const: `[u8; ARRAY_BYTES]` evaluates to the `[u8; 4]` that was there before. covertest PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * A type-keyed length table stores the number, not the spelling Review finding on 4880ffe. `Registry::array_lens` was keyed by `TypeKey` but stored `ArrayLen`, which records WHICH SPELLING produced it — and `TypeKey` has already collapsed the spellings. Given pub a: [u8; A], // A = 4 pub b: [u8; B], // B = 4 pub literal: [u8; 4], all three are one key, so the stored `Const { name }` was whichever occurrence the iteration reached last: order dependent, and false for the other two either way. No consumer reads the name today, so nothing drifted — it was a latent nondeterminism and a claim the table could not honour. Three identities were being conflated, and the fix is to keep them apart rather than to drop the resolution: * const identity — `A` is `4` in crate X — the const index; * type semantics — `[u8; 4]` — the type table; * source-use provenance — field `S::a` was written `[u8; A]` — per occurrence, belonging to a per-use model that does not exist yet (#211's `SourceModel`). So the type-keyed table holds `usize` and `Registry::array_len` returns one. `ArrayLen` keeps the spelling, because at the occurrence level that is true, and becomes crate-private: there is no type-keyed table it could be handed out through honestly, and nothing public produced it any more. `equal_lengths_collapse_to_one_typed_entry` is the regression: three fields, two const-spelled with equal values, one type between them. It asserts both halves — the occurrences stay distinguishable, the type does not — so the layering is pinned rather than the current storage choice. Generated output byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * The spelling of a length is discarded, and says so Review finding on 961be86: the previous test claimed occurrences "stay distinguishable", but that was only true inside the lowering call. After `Registry::from_items` the item types are rewritten to the literal and the occurrence vector is dropped, so no adapter can tell that `S::a` wrote `A`, `S::b` wrote `B`, and `S::literal` wrote `4`. The model was advertising provenance that did not survive the boundary it was documented at. The review offered two coherent ends: build the per-use record now, or declare the spelling semantically irrelevant. Taking the second, explicitly. `ArrayLen` is deleted; lowering returns `usize`. It carried a name that nothing read — a successful lowering raises no diagnostic, and `UnsupportedArrayLen` already renders the offending expression — so the enum existed only to make the overclaim expressible. Why the policy rather than the record: `[u8; A]` and `[u8; B]` with `A == B == 4` are one Rust type, one `TypeKey` and one converter. An adapter rendering them differently would need two destination representations for a single Rust type, which the type-keyed converter table cannot express — so the capability is incoherent at this layer even if the provenance were carried. Building a per-use record now would also mean inventing the "stable use identity" that is precisely `SourceModel`'s design work, one construct at a time, which is the parallel structure #211 exists to remove. The cost is stated rather than hidden: a C header cannot echo `uint8_t x[MAX_SIZE]`, only `uint8_t x[16]`. If that is wanted, provenance arrives on the use site in `SourceModel` — never on a type-keyed table, where three occupants of one key cannot be told apart by the key. The regression now inspects the state AFTER `Registry::from_items`, as asked, and pins both halves: all three fields are `[u8; 4]` sharing one type-keyed row with value 4, and the const names appear nowhere in the model. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * A C header can spell an array extent by name `uint8_t tag[MARKER_TAG_LEN]`, not `uint8_t tag[4]`. A symbolic extent is part of the C API's meaning: it makes changing the size one edit instead of a hunt through literals. 32370af had discarded the spelling, so that fact could not reach cbindgen at all. The obvious patch — stash the original `syn::Type` and let cbindgen re-read it — is what #211 forbids twice over: adapters do not parse captured source, and emitters do not recover semantic facts by re-reading it. So the fact goes in the IR, and this begins `SourceModel` (#211 step 2) rather than building a side channel that would have to be deleted again. `core::frontend::model` adds a closed, language-neutral `SourceType`. Lowering is TOTAL over the grammar in docs/source-language.md — a form it cannot lower is a frontend error, the same acceptance-is-lowering contract array lengths already had. An array node carries `ArrayExtent { value, source }`: the value is the semantic length that keeps `[u8; A]` and `[u8; 4]` one type and one converter, the source is which const the use site named. Both halves have consumers, and they are different ones. The model is the source of truth and `syn` is a projection: pass 3 lowers every struct field and writes `to_syn()` back into the `syn::ItemStruct`, so there are never two representations to disagree. `TypeKey` stays numeric, which is why all of jnigen and most of cbindgen are untouched — covertest PASS, generated Kotlin and Rust byte-identical. Scope is bounded on purpose. Types: complete. Items: structs only, the one surface an adapter consumes as source; functions and enums keep their `syn` items, which #211 step 4 sanctions while adapters migrate. Adapters: cbindgen's struct-field path only. Its per-field CLASSIFICATION still runs on `to_syn()` — `SourceType` is already the answer to `is_scalar`/`is_string`/`is_vec`, but deleting those duplicates is F5/F6, not this change. Two things blocked the header besides the missing fact, both verified rather than assumed: * cbindgen had no array support at all, so `[T; N]` in a data-struct field panicked. `c_field_wire` now accepts an array of non-`bool` scalars: `[u8; N]` is already `#[repr(C)]`-compatible, so it is its own wire and the field copies with no conversion. `[bool; N]` stays refused — its domain is `0`/`1` and a mirror is reinterpreted wholesale with no per-element hook, the same hazard `restricted_validity_field` documents. * consts never reached the header. `on_const` aliased them to `= perftest_flat::ARRAY_BYTES`, a path cbindgen cannot evaluate, so it emitted no `#define`. A probe pinned this down exactly: with a literal it emits `#define MARKER_TAG_LEN 4` and `uint8_t tag[MARKER_TAG_LEN]`; with the alias it emits the struct referencing an UNDEFINED symbol. So the literal is not cosmetic — without it the feature produces a header that does not compile. Only extent consts change; the rest keep the alias. The exercise is in example-cbindgen because `smoke-asan.sh` is the only place CI compiles a generated header. `Marker { tag: [u8; MARKER_TAG_LEN], weight }` round-trips by value in smoke.c, which uses the macro as a bound — so the test COMPILING is most of the proof, since a missing `#define` is a compile error. The x86_64 goldens are derived: the substitution from the aarch64 pair was first verified to reproduce the committed x86_64 pair exactly at HEAD, then applied. CI on x86_64 is the oracle. `equal_lengths_collapse_to_one_type_after_ingest` now pins both halves for real. The second half — that the model still reports `A`, `B` and a literal per field — was the assertion the previous review correctly called untrue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * The projection preserves type identity Review finding on 21ab04a. Pass 3 writes `field.ty = ty.to_syn()`, which made the projection a semantic rewrite in the live pipeline — while it could not reconstruct five forms the grammar accepts: &'static Foo -> &Foo Foo<'static> -> Foo (Foo,) -> (Foo) <T as Trait>::Assoc -> Trait::Assoc foreign::Option<u8> -> Option<u8> Each changes `TypeKey`, so each changes converter selection and the emitted field type. The lifetime losses contradict this repo's own rule that `Foo<'static>` is not `Foo`, which `ptr_class!(ZKeyExpr<'static>)` relies on. Three of the five turn out to be LANGUAGE questions, so the fix narrows the grammar rather than growing the model: * Tuples are not in the language. Verified: every `Type::Tuple` site in both adapters is the unit case or a generic walk — no adapter has ever lowered a non-empty tuple. Refusing deletes the 1-tuple bug instead of fixing it, and turns a late "unresolved type" into a precise frontend error naming the type. * Associated types are refused. `#[prebindgen]` never captures `impl` blocks, so what `<T as Trait>::Assoc` resolves to is unknowable here; carrying the spelling would only move the failure downstream. `Named` then needs no `qself`, which removes that reconstruction path entirely. * `foreign::Option` was a DETECTION bug, not a fidelity one: builtins matched on the last segment alone. They now require a bare single-segment path, which is exactly what `normalize_type` already guarantees — it reduces the genuine std spellings at ingest and deliberately leaves unknown crate paths alone. A foreign type that merely shares a name stays foreign. The remaining two are fidelity, and lifetimes are kept the way they actually behave: part of a type's NAME, never structure, because they mean nothing to a destination language. `Named` keeps its path as identity with the last segment's arguments split out in SOURCE ORDER, lifetimes included, so `Foo<'a, T>` comes back exactly; nested types stay canonical, so an extent inside a generic still projects numerically and `TypeKey` stays right. `Ref` gains a lifetime — no struct field needs one today, but `&'static Encoding` is a real zenoh-flat return, and the variant has to be right before signatures are modeled. The old `to_syn_round_trips` could not catch any of this: it projected an already-projected type, so it only proved a lossy function idempotent. It is replaced by comparing `TypeKey` of the NORMALIZED ORIGINAL against `TypeKey` of its projection, over every accepted form including all five above. Verified it discriminates by restoring two of the bugs: each fails naming its own symptom. Generated output is byte-identical in-repo, and the sibling zenoh-flat-jni regenerates identically to 21ab04a — none of the five occurs in a real struct field, so a correct fix moves nothing. covertest PASS, smoke.c PASS. Also fixes the contract text the review flagged: `frontend.rs`'s "Scope today" and `docs/source-language.md`'s "Read this first" both still claimed only array lengths had moved. The Types table now states the tuple and qself refusals, and records that they bind only in MODELED positions — struct fields today — which is the honest shape of a partial migration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Turbofish and a trailing generic comma are not identity Review finding on 13978e7: `Wrapper::<u8>` and `Wrapper<u8,>` lost their punctuation through `to_syn()`, and `TypeKey` — whose identity is the normalized token string — treated the result as a different type. The damage is worse than a mismatched key. Modeled struct fields are rewritten through `to_syn()` and function signatures are not, so during the migration one Rust type would hold the old key in a signature and the new key in a field, splitting converter lookup by POSITION. Fixed where the reviewer said it belongs: at the declared canonicalization boundary. `normalize_type` now drops the turbofish and a trailing comma, so every position agrees and no consumer has to. Carrying the punctuation in `SourceType` was the alternative, and it is the wrong one — the spellings mean the same type, so preserving the difference only spreads it. That also keeps reconstruction from silently defining canonicalization, which was the reviewer's real point: `to_syn()` already emitted the bare form, so it was deciding by accident what `normalize_type` should decide on purpose. Verified both tests are load-bearing by removing the normalization: the types_util test fails `"Wrapper :: < u8 >" != "Wrapper < u8 >"` and the identity matrix fails `Wrapper::<u8> lost identity`, which is exactly what the review reproduced. Lifetimes stay identity — `Foo::<'static,>` collapses to `Foo<'static>`, never to `Foo`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Close the callback gate, and say which refusals are temporary Review finding on a3cf2af, and this one was LIVE rather than latent. `extract_fn_trait_args` is the acceptance gate for every callback — 14 call sites across core resolution and both adapters — and it read `p.inputs` while never looking at `p.output` or the bound's `lifetimes`. So impl Fn(u8) -> u16 + Send + Sync + 'static was accepted, planned from its arguments alone, and its result dropped: every callback wire is void-shaped, C's `call` having no return and jnigen's `run` returning void. The test that now refuses it reports, without the fix, exactly what was happening — `accepted as Callback { args: [Scalar(U8)] }`. Refusals now come in two kinds, because a bare "no" leaves the author guessing whether to redesign or to wait, and those are opposite responses: * RESERVED — the language intends it, the machinery does not exist yet. A callback returning a value is reserved, and the diagnostic names issue #216, which carries the design that actually blocks it: both adapters have a case where the foreign side yields no value (a null `call` pointer, a throwing Kotlin callback), and with a return type there is nothing to swallow to. A callback returning a callback is reserved too. * UNSUPPORTED — it cannot work here. A higher-ranked binder is the case: no FFI boundary can be generic over a lifetime, so no adapter could ever carry one. The gate becomes `extract_fn_trait_sig -> Result<_, CallbackReject>` with `extract_fn_trait_args` a thin `.ok()` wrapper, so the 13 shape-query callers are untouched and ONE function still decides. A separate "why did it fail" helper beside an Option-returning gate would have been the two-authority drift this PR exists to remove. `ScanError::DisallowedImplTrait` and the model's `UnsupportedTypeReason` both carry that one reason rather than restating it. Three further spellings were one type with different keys — the split-by-position bug fixed for turbofish in a3cf2af, reappearing in `Fn`'s parenthesized argument list and its bound list. They canonicalize at the same boundary, by the rule settled last round: a trailing input comma, an explicitly-spelled `-> ()`, and bound order (traits sorted, lifetime bounds last so the result stays valid Rust). Since every accepted callback is therefore unit-returning with a fixed bound order, `SourceType::Callback` needs no output field and its projection stays exact. Verified both new rules are load-bearing by reverting each: the refusal matrix accepts the non-unit callback again, and the normalization test reports `Sync + Send` against `Send + Sync`. Generated output byte-identical in-repo, and the sibling zenoh-flat-jni regenerates identically to a3cf2af. covertest PASS, smoke.c PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Fix normalization idempotence, a false HRTB claim, and a layering inversion Three findings on ca21e16, all correct. **Idempotence.** `visit_path_arguments_mut` tested whether the `Fn` return was `()` BEFORE calling the recursion that unwraps `Paren`, so `impl Fn(u8) -> (())` needed two passes to reach `impl Fn(u8)`. Items are normalized at ingest and `TypeKey::from_type` normalizes again, while a directly built key normalizes once — so a key could depend on how many passes its input had had. The recursion moves to the top, matching `visit_type_impl_trait_mut`, which already had the right order. The regression is the general property, not the instance: `normalize_is_idempotent` asserts `canon(canon(x)) == canon(x)` over every spelling the suite exercises, because the way this breaks is a rule reading a node the recursion has not reached yet, and that mistake belongs to no particular rule. **The HRTB claim was wrong.** `ca21e16` refused an explicit `for<'a>` binder as definitively unsupported, reasoning that no FFI boundary can be generic over a lifetime. The counter-example is decisive: `impl Fn(&u8)` IS higher-ranked — it desugars to `impl for<'a> Fn(&'a u8)` — and the two are mutually substitutable. The binder constrains the Rust closure; it is not carried on any wire. So the accepted spelling and the refused one are the same type, which makes this the last two-spellings-one-type case rather than an impossibility. Reclassified to RESERVED with the real reason: the canonicalization is not written, and it has to be exact, since elision gives each elided input lifetime its own fresh binder — `for<'a> Fn(&'a u8, &'a u8)` is NOT `Fn(&u8, &u8)`. Issue #222 carries that rule. The diagnostic now points at the elided form, which is accepted and means the same thing. **Core depended on an adapter.** `extract_fn_trait_sig` called `jnigen::util::is_unit` — a language-neutral source rule reaching into one adapter, the inversion #211 exists to prevent. Core's own `is_unit` is ungated and jnigen's is DELETED rather than re-exported: it had exactly one other caller, and two names for one predicate is what let the inversion happen unnoticed. Verified the idempotence fix is load-bearing by moving the recursion back — both the general assertion and the `-> (())` row fail, reproducing the review exactly. Generated output byte-identical in-repo and in the sibling zenoh-flat-jni. Builds clean with and without default features. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * The HRTB fix-it is only exact when elision preserves the binding Follow-up on 2b84e02. That commit corrected the HRTB *classification* but left the advice unconditional: "write the elided form instead (`Fn(&T)`, which means exactly the same thing)". True for `for<'a> Fn(&'a T)`, false for the rest of the refused class — the enum's own doc gives the counterexample two lines above. Elision binds each input separately, so `Fn(&T, &T)` is `for<'a, 'b> Fn(&'a T, &'b T)`; an author whose signature ties two inputs to one lifetime was being told to make a semantic change and told it was exact. The message now states the condition: exact when every bound lifetime is used once, and no accepted spelling yet when one is used twice. `source-language.md` carried the same unconditional sentence and gets the same treatment. The acceptance matrix still labelled the row UNSUPPORTED and repeated the retracted "no FFI boundary can be generic over a lifetime" claim, so the executable document disagreed with both the enum and the guide. Relabelled RESERVED with the actual reason. Checked that no copy of the retracted claim survives anywhere in the tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ger (#224) #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>
…rojection (#211) (#225) * F5: cbindgen's struct-field classification reads the model, not the projection #212 made the struct-field path read `SourceType` from `Registry::source_struct`, but then threw the model away: every field was projected back through `to_syn()` and re-classified with `is_string` / `is_bool` / `is_scalar` / `is_scalar_array`. The frontend had already decided all of that, so the projection was a second authority on one fact -- the drift #211 exists to stop. `c_field_wire`, `is_scalar_array`, `data_field_wire`, `data_field_owns`, `restricted_validity_field`, and the `in_data_struct` / `out_data_struct` field loops now take a `SourceType` and match it. The rule this establishes, for the stages after it: SHAPE FROM THE MODEL, IDENTITY FROM THE REGISTRY. "Is this field a `String`, a `bool`, an array of scalars?" is a source fact the frontend decided; asking syntax again is the duplicate. "Is this the type the binding declared as a `tagged_union`?" is a registry lookup that happens to be keyed by `TypeKey`, i.e. by a `syn::Type` -- rekeying that is F4's job, not this one, so those sites project and say why. `mirror_field_wire` deliberately stays on syntax. It is ONE policy serving both the `repr_c_struct` mirror path, which has a model, and the tagged-union payload path, whose variant fields the frontend does not model yet. Splitting it in two would duplicate the policy, which is the thing being deleted here. It waits on F2 modeling enums. The ledger caught the win on its first real use: `api/lang/cbindgen/mod.rs` 6 -> 5, total 196 -> 195, because `is_scalar_array` no longer matches `syn::Type::Array`. Note what that number does NOT capture -- `is_string`, `is_bool` and `is_scalar` were never in the ledger at all, since they classify by ident name, one of the blind spots its header lists. The count moved by one; the duplication deleted was larger. That is the ledger working as designed, not a measure of the change. Generated C artifacts byte-identical (`regen-check.sh` clean); 428 lib tests and the full workspace suite green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Update struct_fields' contract comment to the split this PR established The paragraph still said per-field classification runs on `to_syn()` and that migrating it was future work -- the opposite of what this branch does, and of docs/source-frontend.md. In a boundary-migration series a stale contract comment is worse than none: the next change reads it and heads back toward the duplicate authority this PR removed. Replaced with the rule itself -- shape from the model, identity may still project through TypeKey until F4 -- plus the one deliberate exception, `mirror_field_wire`, and why splitting it would duplicate a policy rather than delete one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…211) (#226) * F2: model enums, and move cbindgen's whole enum path onto the model Everything still open in #211 was blocked on one fact: the model covered struct fields and nothing else. This lands enums, which is what unblocks F5's `mirror_field_wire` and is a precondition for F6. Not a new model. `types_util` already described an enum language-neutrally -- `SumSpec` / `SumVariant` / `SumField`, tag plus one field group per variant, with jnigen already consuming it. Writing a second enum model beside it would have been exactly the duplicate authority #211 exists to remove, so this MOVES them into the frontend and completes them: * payload types are lowered through `lower_type` instead of being carried as raw `syn::Type` -- the change that lets cbindgen ask the model; * `EnumShape` / `enum_shape` / `first_payload_variant` fold in as `SourceEnum::is_unit` / `first_payload_variant` / `SourceVariant::shape`, so the shape question stops being a match on `syn::Fields` at each use site; * the discriminant is modeled. THE DISCRIMINANT IS AN `ArrayExtent` PROBLEM AGAIN. The two backends need different halves of one fact: cbindgen re-emits the SPELLING verbatim into its `#[repr(C)]` mirror, deliberately, so a `const`- or `cfg`-driven value keeps working and `= 0x07` stays `0x07`; jnigen needs the NUMBER, for a Kotlin `NAME(n)` entry and the `jint -> variant` decode. So `Discriminant { value: Option<i64>, source: DiscriminantSource }`, the same split `ArrayExtent { value, source }` already makes for an extent. `value: None` turns a panic reachable from `enum_class!` into a refusal naming the variant. `DiscriminantSource::Explicit(syn::Expr)` is the model's one carrier of open syntax -- deliberate, since narrowing it would delete C support that exists today, and now listed under F7. With variant payloads modeled, cbindgen's whole enum path follows the rule #225 set -- shape from the model, identity from the registry: `mirror_field_wire`, `payload_field_wire`, `payload_wire_owns`, `payload_needs_converter`, `enum_variants`, `variant_pattern`, `variant_ctor`, `payload_wire_of`. `mirror_field_wire` is the bullet #225 had to defer, because it is ONE policy serving both the modeled mirror path and the then-unmodeled payload path; splitting it would have duplicated the policy rather than deleted it. Modeling enums let it move as one piece. Ledger: `api/core/types_util.rs` 41 -> 39, total 195 -> 193. Generated C, Rust and Kotlin byte-identical (`regen-check.sh` clean); 426 lib tests and the full workspace suite green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * F2 review: keep the written variant shape, and stop overflowing discriminants Two blockers from the review of the new enum model. P1: `fields.is_empty()` is not `syn::Fields::Unit`. `B()` and `C {}` carry no payload and still must be spelled `E::B()` / `E::C {}` wherever Rust names them, so deriving the shape from the payload group emitted `E::B` — a constructor function, not a value. `VariantShape` is now taken from `syn::Fields` at lowering and stored on `SourceVariant`; `is_unit()` keeps answering the separate group question. `VariantShape::spell` is the one place the delimiters are chosen, replacing five hand-rolled derivations across cbindgen and jnigen (two of which read `syn::Fields` directly). P2: `next = value.map(|n| n + 1)` panicked during ingest for valid Rust such as `#[repr(u64)] enum E { A = i64::MAX, B }`, taking down every adapter including the C one that only re-emits the spelling. `checked_add` ends the numeric chain the way an unevaluable spelling does, leaving `DiscriminantSource` intact. Covered in the model (both empty forms, shape vs group, top-of-range discriminant) and in the generated Rust for both adapters and both directions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This was referenced Jul 29, 2026
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>
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.
This body mirrors
docs/source-frontend.mdon this branch, which is the one place stage state is edited. Change the doc, then re-sync this body — never the other way round.One Rust frontend and a common source IR — branch map
Integration branch for the frontend work tracked by
#211.
Draft, and long-lived — do not merge until every completion criterion in
#211 holds.
#211 remains the authority on the invariants, the frontend/adapter boundary, and
the completion criteria. This document does not restate them; it records what has
landed, what has not, and the order.
Why a branch
Unlike #187, whose stages are
not independently landable, #211's steps mostly are — the issue says so
explicitly: "Each step should leave tests green and have a concrete consumer."
So this branch is not here to hide half-migrated states. It is here to keep one
reviewable diff against
mainfor a change whose value is structural: "noadapter independently parses source Rust" is a property of the whole, not of any
one step, and it is easy to lose one PR at a time.
The consequence is worth stating plainly: because the steps are independently
landable, any of them could go straight to
main, exactly as Stage 0 (#200) didfor #187. Routing them through this branch is a choice about reviewability, not a
technical necessity — so if a stage's own fix becomes urgent, re-pointing it at
maincosts nothing but a base change.Child PRs target
source-frontend.Size of the problem
syn::Type::/syn::Expr::match sites outside tests and outside the frontend,i.e. places that classify source shape today:
api/coreapi/lang/jnigenapi/lang/cbindgenThe first two columns were counted by hand and are wrong in both directions — a
recount gives 69/96/26, a token-level count a third answer. The
After F8column is the authoritative one, and it is not a column in this table: it is
prebindgen/src/api/core/frontend/boundary.ledger, regenerated by a test.A scoreboard nobody can reproduce is not a scoreboard, which is the reason F8
exists at all; the buckets above are a rollup of the per-file ledger, kept here
only as a scale marker.
Not all of these are source-syntax classifiers — many inspect types the adapter
itself synthesized (wire types, converter signatures), which is legitimately the
adapter's business. Separating the two populations is F1's job.
The count did not go down at F2, and that is expected. #212 added
SourceType(insidefrontend/, so outside this count) without deleting theclassifiers it will eventually replace: cbindgen's struct-field path now reads
the model, but still asks
is_scalar/is_string/is_vecof theto_syn()projection. Removal is F5/F6. Until then this table measures duplication that
exists, not progress — which is why it is reported per stage rather than as one
number.
Stages
core::frontendseedSourceModel— the language-neutral item modelRegistryconsumesSourceModelCbindgenJniGen(the long pole — 97 sites)PrebindgentraitChecklist
F0 — array lengths and the frontend seed — done (#212)
core::frontendmodule exists, with module docs stating the one-walk ruleArrayLen— one closed representation (Literal/SourceConst/ExternalConst)lower_array_len— a single fallible walk; acceptance is a consequence of loweringRegistry::from_itemspass 3), before any adapter existsScanError::UnsupportedArrayLength, naming the offending sub-expressionreject_unsupported_array_length,QualifyLengthPathsandlength_namesdeleted<Holder>::Nregressiondocs/source-language.md— the written contract#[prebindgen]const used as a length, compiled from another crateF0 is self-contained: CI-green, and it closes #210 on its own. It targets
this branch so the chain has one diff against
main, but it is the one stagethat could be re-pointed at
mainat any time — as #187 did with its Stage 0 —if #210 should be fixed before the rest of the chain lands. Note the cost of
keeping it here:
Closes #210does not fire until this branch merges.F1 — specify the source language
docs/source-language.mdcurrently inventories item kinds, function forms, andtype forms, but only the array-length row has actually moved into the frontend;
every other row records where the decision is made today, often inside an
adapter.
F8 landing first changes what this stage is. Classification is no longer a
table to write; it is entries coming off
boundary.ledger, one migration at atime, with a failing test behind every one that stays.
WATCHEDinfrontend/boundary.rsbeyondType/Expr—Item,Fields,FnArg,ReturnType,GenericArgument,Pat— one enum at a time, each addition a regenerated ledger whose diff is the decisioncore/domain.rs), ident-name classification (seg.ident == "Option"), helper delegation (jnigen/jni/classify.rs),match_patternunificationunion,typealias, generic parameters, raw pointersF2 —
SourceModel— partially landed in #212Started earlier than planned, and for a concrete reason worth recording: a C
header must be able to emit
uint8_t tag[MARKER_TAG_LEN], and the only honestway to carry "this extent named that const" to an adapter is a node in the
IR. The tempting shortcut — stash the original
syn::Typeand let cbindgenre-read it — would have violated invariants 5 and 6 in one move.
That is the general lesson, not a detail of this construct: whenever an adapter
turns out to need a source fact, the answer is to model the fact, never to widen
what syntax reaches the adapter.
SourceType— a closed, language-neutral type model; lowering is total over the documented grammar, so acceptance is a consequence of loweringArrayExtent { value, source }carried per use site, never keyed byTypeKeySourceStructfor indexed structs; thesynfield types are a projection (to_syn), never a second source of truthUnsupportedType, surfaced asScanError::UnsupportedFieldTypenaming item and fieldsynitemsSourceModel(completion criterion 1)F3 — move the type-shape classifiers into the frontend
registry::immediate_subtype_positionsandtypes_util::immediate_pattern_childrenare near-duplicates that already diverge in theirType::Pathhandlingtypes_util::normalize_type/normalize_item_types— ingest normalization belongs to the frontendextract_fn_trait_args— theimpl Fn(..) + Send + Sync + 'staticclassifierRegistry::scan_fn_signature's receiver and parameter-pattern guardstypes_util::match_patternand theType::InferunificationF4 —
RegistryconsumesSourceModelSourceModelitems rather than rawsyn::Itemmapsfunctions/structs/enums/consts/passthroughfields stop being the adapter-facing contractRegistryinto phase-specific types) — same seam, different cutF5 — migrate
CbindgenCbindgen gains array support —
[T; N]of non-boolscalars is its own C wire;[bool; N]is refused for the restricted-validity reason (One frontend for array lengths, and a typed source model that carries extents to C (#210, first step of #211) #212)The struct-field path reads
SourceTypefromRegistry::source_struct(One frontend for array lengths, and a typed source model that carries extents to C (#210, first step of #211) #212)The per-field classification now matches the model:
c_field_wire,is_scalar_array,data_field_wire,data_field_owns,restricted_validity_field, and thein_data_struct/out_data_structfield loops take aSourceTypeand match it, instead of re-deriving the shape fromto_syn()The rule this established, and the one later stages should follow: shape from the model, identity from the registry. "Is this field a
String, abool, an array of scalars?" is a source fact the frontend already decided, so asking it again of syntax is the duplicate authority Architecture: one Rust frontend and a common source IR for all generators #211 forbids. "Is this the type the binding declared as atagged_union?" is a registry lookup that happens to be keyed byTypeKey, i.e. by asyn::Type— that key is F4's to change, not F5's, so those call sites project and say why.Effect on the ledger:
api/lang/cbindgen/mod.rs6 → 5, the first entry to come off it. The other predicates in that file (is_string,is_bool,is_scalar) were never in it — they classify by ident name, one of the blind spots the header lists — which is precisely why a count alone is not the measure.mirror_field_wirestill classifies syntax (is_scalar/box_inner/is_option). It is one policy shared by therepr_c_structmirror path, which has a model, and the tagged-union payload path, whose variant fields the frontend does not model yet. Splitting it in two would duplicate the policy — the exact thing Architecture: one Rust frontend and a common source IR for all generators #211 forbids — so it waits on F2 modeling enumslang/cbindgen/{trait_impl,convert,emit,builder,mod}.rsconsumeSourceModelfor parameters and returns too, not only struct fields — also blocked on F2 (functions are stillsynitems)Generated C artifacts byte-identical, except where previously-accepted ambiguous syntax becomes an intentional frontend error —
regen-check.shclean across the field-classification migrationF6 — migrate
JniGenprim_array_ofreadsArrayLeninstead of re-matchingType::Arraybuilder.rs,emit/*,iface.rs,selector.rs,render.rs,fold.rs,overloads.rsconsumeSourceModelemit/names.rs's remaining source probes (pat_match,pat_match_top,option_inner_ref_mutability,rust_short_name_opt) classified per F1 and migrated or justifiedF7 — close the open-syntax boundary
#211: "Keeping
syninternally in the frontend is fine. Letting unclassifiedsyn::Expror equivalent open syntax become an adapter-facing semantic contractis not." These are the places where it currently does:
Niches { value: syn::Expr, matches: syn::Expr }— a niche is a semantic fact carried as raw expression syntaxDomainScalar::rust_expr()/portable_expr()returningsyn::ExprConverterImpl::function/TypeEntry::functionassyn::ItemFnPrebindgen::post_process_item(&mut syn::Item)— the hook that let qualification live in an adapter in the first placePrebindgen::prerequisites/local_functionsreturning raw itemsNote this one overlaps #187's emission work; F7 is about the contract, not
about how emission is planned.
F8 — mechanical boundary check — done, and moved to the front
As written, F8 was seeded from F1. That ordering makes F1 a large document that
rots the day after it is written, with nothing failing behind it. Inverting the
two costs one stage and is strictly better: the ledger freezes the population
now, so growth stops immediately, and F1's classification then happens
incrementally as entries come off the ledger during F3–F6. The allow-list becomes
the classification instead of a second copy of it.
frontend/boundary.rs, riding the existingcargo test; no new CI jobfrontend/boundary.ledger, per file, exact-match, regenerated byUPDATE_BOUNDARY_LEDGER=1 cargo test -p prebindgen boundary_ledgerThree properties are load-bearing and easy to lose in a later edit:
cargo testandcargo test --all --all-features, andunstable-cbindgengates the whole cbindgen suite, so a compiled-AST check would give two answers
in one CI run — the failure mode of regen-check.sh and the --all-features test job disagree about the committed example-cbindgen goldens #219.
grep -ccounts lines ratherthan occurrences, counts the inline
#[cfg(test)] modblocks (so adding a testwould fail the check — how these ledgers die), and is defeated by one line:
use syn::Type;thenType::Reference(_). Asyn::visit::Visitfixes thoseand cannot see inside
matches!(..), where a large share of the sites live.under-reports is worse than no check; those gaps are F1's queue.
Relationship to #187
Deliberately separate, and neither blocks the other.
If #187 proceeds first, its Tier 0 and later plans should consume the frontend
model rather than introduce another source classifier. If this branch proceeds
first, F3–F6 reduce the surface #187's stages have to move.
The one concrete coupling: #187's Stage T (#190, merged) introduced a semantic
shape tier. F1 must classify whether Tier 0's shape reading is a source fact
(migrates here) or a boundary fact (stays there).
Review protocol
Each stage PR states its own exit:
examples/regen-check.sh. A diff is a bug.CI runs on every PR whatever it targets, so a stacked PR is not exempt from
clippy, fmt, the golden-diff check, the JVM harness, or the sanitizer gate.
🤖 Generated with Claude Code