Handles: a live object, with state that lives across calls - #12
Open
zmaril wants to merge 6 commits into
Open
Conversation
The owner asked why not add streams. Checking jawohl first turned up a useful
correction: streams in jedem's sense -- an async iterable -- are not what it
needs. changes() is a DRAIN returning Vec<Event>, not an iterator. What jawohl
needs is HANDLES, and they unlock nearly the whole incremental API:
Stream::new, from_json_schema, push, finish, is_document_complete, status,
validation, is_irrecoverable. Only snapshot, changes, lowering_report and error
need more, and what they need is records and unions.
So this adds handles. An exported impl with a receiver or a Self-returning
constructor is now a handle, and each language gets a real class:
const c = new hello.Counter();
c.add(10); c.add(6);
c.total(); // 16 -- the state stayed
They are far simpler here than the same feature was in fluessig, and for a
structural reason: jedem's generated binding depends on the core crate, so a
handle can simply own a `core::Counter`. fluessig needed a core trait and an
Arc<Impl> only because its declared surface was separate from the code that
implemented it. Removing that separation removed the hard part.
Three things the implementation forced:
A method taking `self` by value is rejected with a spanned error. Consuming the
handle has no meaning once another language owns it.
Python has exactly one __new__, so only the op literally named `new` becomes
#[new]; any other Self-returning op becomes a #[staticmethod] factory, which is
how alternate constructors are spelled in Python anyway. napi has the same
shape with #[napi(factory)]. The first draft emitted two #[new] fns and would
not have compiled.
An infallible unit return now emits no return clause at all. `fn add(&mut self,
n: i64) -> ()` is noise no hand-written binding would carry.
Also checked fluessig for prior art, as asked. Its catalog! is surface!'s direct
ancestor -- same shape -- but it did NOT own generation; that went through
`cargo fluessig emit` to catalog.json. It also had the same marker-type wart:
"a marker table so catalog! has an entity root". Both are things jedem has since
removed, and the second confirms that was a real problem rather than a
cosmetic one.
43 tests. Both host round-trips exercise state persisting across calls,
independent instances, an alternate constructor, and a handle still being usable
after a method failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HsDxLrGdx6nPaXkVEWkNvS
An exported impl block is usually most of a type's API. The parts that cannot cross a boundary -- or that jedem cannot lower yet -- are the exception, and splitting the impl in two to express that scatters related methods. A marker on the method says it in place. Skipping is checked before the by-value-self rejection, so a Rust-only consuming method is a marker away rather than a hard error, and before handle classification, so a skipped constructor does not silently turn a namespace into a handle. No generated file changes: a skipped method is absent from the surface, so no backend ever sees it.
Generated crates are workspace members, so `cargo fmt --all` formats
them like any other source. Where the generator's layout differed from
rustfmt's, that command silently rewrote the committed bindings and the
next run of the drift guard failed pointing at the surface -- which
nobody had touched. It cost several confused debugging rounds.
CI sidestepped this by formatting only the hand-written crates, so the
breakage never reached a pull request. That makes it a worse trap, not a
smaller one: the repository failed under the obvious command and only a
non-obvious one kept it working.
Two constructs diverged, both in handles:
- a constructor's `Self { inner: some::Path::new() }` passes
rustfmt's `struct_lit_width` (18) and gets broken across four
lines. Wrapping now goes through a generated `From` impl, so the
only struct literal is `Self { inner }` -- short enough to stay put
-- and constructors read `Path::new().into()`.
- the method loop left a blank line before the impl's closing brace.
`#![rustfmt::skip]` would state the intent directly, but it is a custom
inner attribute and still unstable, so rustc rejects the file.
The guard is a test that runs rustfmt over the generated output of every
target and asserts it comes back unchanged, which catches the whole
class rather than these two cases. The sample surface gains a handle so
the test actually reaches the code that was wrong.
`#[diagnostic::on_unimplemented]` -- which turns a bare "trait not implemented" into a sentence naming the missing derive -- stabilised in 1.78, and nothing in the workspace declared that. jawohl found out when its own MSRV job failed against a jedem git dependency, which is the wrong place to learn it. A crate depending on jedem can now read the floor from the manifest.
`fn with_x(mut self, ...) -> Self` was a hard error, and the two such methods on jawohl's `Stream` had to be marked `#[jedem(skip)]`. That was the wrong call: the builder returns *the same logical object*. Rust's move guarantees the caller's old binding is dead, so nothing can observe the difference between "consumed and returned" and "mutated in place". Lowering it to an in-place mutation is an identity, not an approximation. So a builder now crosses with no annotation and no reshaping. Python returns `PyRefMut<Self>`, node returns `This`, and both hand back the same object -- `Counter().with_total(10).total()` chains in all three languages, and identity holds: `same is built` and `same === built`. The wrapper has to move its value out to call, so a handle with a builder stores `Option<Core>` and reaches it through accessors. Only such a handle pays that: every other one still holds the core value directly. The empty state is reachable only between the take and the put-back, so seeing it means the builder panicked, and the message says exactly that. `Result<Self, E>` is still rejected, now with its own message. On the error path Rust has already consumed the value and hands nothing back, so the handle would be left empty for real -- and every other method on the interface would have to answer for that, turning infallible signatures fallible across the whole interface. That is a much larger change than the builder itself, and not one to make silently. Two things found along the way: `This` must be written bare. napi-derive matches it by name against a fixed list, so a qualified path is taken for an ordinary argument and the class fails to register -- reported as "did not find struct parsed before expand", which points nowhere near the cause. The generated accessors go *after* the exported impl. napi-derive keeps a global registry of structs it has seen and depends on expansion order; an unrelated `impl` between the struct and its `#[napi] impl` breaks it.
`usize`, `u64` and the narrow ints all lowered to `Type::I64` and then lost which Rust type they came from, so the generated call passed an `i64` to a core expecting `usize` and the binding crate did not compile. Every op kind was affected; jawohl's `with_max_depth(usize)` was just the first to reach it, because it had been skipped until now. `Param::cast` and `Op::returns_cast` carry the Rust width when it is not the boundary width, and the call site casts on the way in and on the way out. `None` in the common case, emitting nothing. The demo grows `take_steps(usize) -> usize` to hold both directions, and both host tests call it.
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.
Stacked on #11. You asked why not add streams — checking jawohl first turned
up a correction worth having.
Streams aren't what jawohl needs; handles are
changes()is a drain returningVec<Event>, not an iterator. So jedem's"stream" (an async iterable) buys jawohl nothing. Handles buy it nearly
everything:
Stream::new,from_json_schemapush,finish,is_document_completestatus → Syntax,validation → Validationis_irrecoverablechanges(),snapshot()lowering_report,errorHandles alone unlock the entire incremental API. Only four accessors need more,
and they need records and unions — not streams.
Much simpler here than in fluessig, for a structural reason
jedem's generated binding depends on the core crate, so a handle simply owns a
core::Counter. fluessig needed a core trait and anArc<Impl>only becauseits declared surface was separate from the code implementing it. Removing that
separation removed the hard part.
Three things the implementation forced
selfby value is rejected, with a spanned error: consuming the handle hasno meaning once another language owns it.
__new__. Only the op namednewgets#[new]; otherSelf-returning ops become#[staticmethod]factories — how alternateconstructors are spelled in Python anyway, and
#[napi(factory)]in node. Myfirst draft emitted two
#[new]fns and would not have compiled.-> ()noise. An infallible unit return emits no return clause.On fluessig, as you asked
catalog!issurface!'s direct ancestor — same shape — but it did not owngeneration; that went through
cargo fluessig emit→catalog.json. It also hadthe same marker-type wart: "A marker table so
catalog!has an entity root".Both are things jedem has since removed, and the second confirms that was a real
problem rather than a cosmetic one.
43 tests. Both round-trips exercise state persisting across calls, independent
instances, an alternate constructor, and a handle still usable after a method
failed.
Next, in jawohl: annotate
Stream,complete_json,SyntaxandValidationdirectly and delete the surface crate — which is now possible.🤖 Generated with Claude Code
https://claude.ai/code/session_01HsDxLrGdx6nPaXkVEWkNvS