Skip to content

Commit bb845b3

Browse files
milyinclaude
andcommitted
Exception slot: Option<String> → Option<syn::Type>, exact path match
The closure's middle slot is ultimately the rust type that gets spliced into `Result<ty, X>` in the emitted converter signature — store it as a `syn::Type` from the start. Also tighten lookup: `find_exception` now matches by **exact** canonical-form equality against `ExceptionConfig.rust_path` — the short-name fallback (`"ZError"` matches `zenoh_flat::errors::ZError`) is gone. The closure must spell the same full path `.kotlin_exception_class("...")` was declared with, via `parse_quote!(zenoh_flat::errors::ZError)`. `ExceptionConfig.rust_short` stays — still drives the default Kotlin class name and the `throw_<short>` fn-name derivation — just out of the lookup path. `callback_input`'s `exc` arg follows: `Option<&str>` → `Option<syn::Type>`. Implementation note: `syn::Type` isn't `Send + Sync` (it holds `Rc<TokenStream>`), so the closure can't capture it directly — same trick as the dispatcher path: serialise to its canonical token form at the boundary, re-parse inside the closure. build.rs migrates mechanically: every `Some("ZError".into())` becomes `Some(parse_quote!(zenoh_flat::errors::ZError))`, and the two `callback_input(..., Some("ZError"), ...)` calls get the same path treatment. Generated `zenoh_flat_jni.rs` is byte-identical — the change is purely at the registration surface; the resolved `rust_path` spliced into `Result<_, zenoh_flat::errors::ZError>` is the same. 22 prebindgen-ext tests + 97 zenoh-java JVM tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 83bc864 commit bb845b3

3 files changed

Lines changed: 116 additions & 99 deletions

File tree

prebindgen-ext/src/jni/jni_ext.rs

Lines changed: 79 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -185,12 +185,15 @@ pub(crate) struct TypeConfig {
185185
/// a wire shape (or the self-converter case) ⇒ terminal converter
186186
/// with `destination = ty`; a rust type with its own converter ⇒
187187
/// composed as a value-inspecting stage onto that converter's chain.
188-
/// * `exc` — the bound JVM exception name (e.g. `"ZError"`, or the
189-
/// short / full Rust path; resolved via [`JniExt::find_exception`]).
190-
/// `Some(...)` ⇒ throwing: the body evaluates to `Result<ty, exc>`
191-
/// and is emitted as-is. `None` ⇒ non-throwing: the body evaluates
192-
/// to a bare `ty` and the framework wraps it `Ok(body)` with
193-
/// `Result<ty, __JniErr>` (= `JniBindingError`).
188+
/// * `exc` — the bound exception **as a Rust type**, matched by exact
189+
/// canonical-form equality against a [`JniExt::kotlin_exception_class`]
190+
/// registration's `rust_path` (use the same full path the
191+
/// registration was declared with, e.g.
192+
/// `parse_quote!(zenoh_flat::errors::ZError)` — no short-name
193+
/// matching). `Some(...)` ⇒ throwing: the body evaluates to
194+
/// `Result<ty, exc>` and is emitted as-is. `None` ⇒ non-throwing:
195+
/// the body evaluates to a bare `ty` and the framework wraps it
196+
/// `Ok(body)` with `Result<ty, __JniErr>` (= `JniBindingError`).
194197
/// * `body` — the closure body. The decision between Ok-wrap vs
195198
/// verbatim is keyed on `exc` (see [`JniExt::build_input_fn`] /
196199
/// [`JniExt::build_output_fn`]).
@@ -199,7 +202,7 @@ pub(crate) struct TypeConfig {
199202
/// inner-type entries (`registry.output_entry(t)`).
200203
pub(crate) type WrapperFn = Arc<
201204
dyn Fn(&[syn::Type], &Registry<KotlinMeta>)
202-
-> Option<(syn::Type, Option<String>, syn::Expr)>
205+
-> Option<(syn::Type, Option<syn::Type>, syn::Expr)>
203206
+ Send
204207
+ Sync,
205208
>;
@@ -225,7 +228,7 @@ pub struct Arity3;
225228

226229
impl<F> WrapperBuilder<Arity0> for F
227230
where
228-
F: Fn(&Registry<KotlinMeta>) -> Option<(syn::Type, Option<String>, syn::Expr)>
231+
F: Fn(&Registry<KotlinMeta>) -> Option<(syn::Type, Option<syn::Type>, syn::Expr)>
229232
+ Send
230233
+ Sync
231234
+ 'static,
@@ -238,7 +241,7 @@ where
238241

239242
impl<F> WrapperBuilder<Arity1> for F
240243
where
241-
F: Fn(&syn::Type, &Registry<KotlinMeta>) -> Option<(syn::Type, Option<String>, syn::Expr)>
244+
F: Fn(&syn::Type, &Registry<KotlinMeta>) -> Option<(syn::Type, Option<syn::Type>, syn::Expr)>
242245
+ Send
243246
+ Sync
244247
+ 'static,
@@ -253,7 +256,7 @@ where
253256

254257
impl<F> WrapperBuilder<Arity2> for F
255258
where
256-
F: Fn(&syn::Type, &syn::Type, &Registry<KotlinMeta>) -> Option<(syn::Type, Option<String>, syn::Expr)>
259+
F: Fn(&syn::Type, &syn::Type, &Registry<KotlinMeta>) -> Option<(syn::Type, Option<syn::Type>, syn::Expr)>
257260
+ Send
258261
+ Sync
259262
+ 'static,
@@ -269,7 +272,7 @@ where
269272
impl<F> WrapperBuilder<Arity3> for F
270273
where
271274
F: Fn(&syn::Type, &syn::Type, &syn::Type, &Registry<KotlinMeta>)
272-
-> Option<(syn::Type, Option<String>, syn::Expr)>
275+
-> Option<(syn::Type, Option<syn::Type>, syn::Expr)>
273276
+ Send
274277
+ Sync
275278
+ 'static,
@@ -746,8 +749,8 @@ impl JniExt {
746749
/// registered closure passes `None` for the exception slot. If a
747750
/// binding wants the failure to surface as a specific domain
748751
/// exception instead, register the converter directly via
749-
/// [`Self::input_wrapper`] with `Some("X")` in the closure's
750-
/// middle slot.
752+
/// [`Self::input_wrapper`] with `Some(parse_quote!(<full path>))` in
753+
/// the closure's middle slot.
751754
pub fn jint_enum(
752755
self,
753756
rust_key: impl AsRef<str>,
@@ -780,38 +783,45 @@ impl JniExt {
780783
/// * `exc = None` ⇒ non-throwing: emitted body is
781784
/// `<dispatcher_path>(env, &v)?` (framework `?`-propagation); only
782785
/// valid if the dispatcher returns the framework error.
783-
/// * `exc = Some("X")` ⇒ throwing: the dispatcher is expected to
784-
/// return `Result<impl Fn(...), X>` (e.g. `ZResult<_>`), and the
785-
/// emitted body is the dispatcher call directly — no `?`/`Ok`,
786-
/// per the body↔exception coupling.
786+
/// * `exc = Some(<Rust type>)` ⇒ throwing: the dispatcher is
787+
/// expected to return `Result<impl Fn(...), <Rust type>>` (e.g.
788+
/// `ZResult<_>`), and the emitted body is the dispatcher call
789+
/// directly — no `?`/`Ok`, per the body↔exception coupling. The
790+
/// type must match a [`Self::kotlin_exception_class`] declaration
791+
/// by exact canonical-form equality (see [`Self::find_exception`]).
787792
///
788793
/// The Kotlin FQN auto-derives via the callback-name template
789794
/// (`<callback_name_prefix><stem><callback_name_postfix>`); chain
790795
/// [`Self::kotlin_name`] immediately after to override.
791796
pub fn callback_input(
792797
mut self,
793798
impl_fn_key: impl AsRef<str>,
794-
exc: Option<&str>,
799+
exc: Option<syn::Type>,
795800
dispatcher_path: impl AsRef<str>,
796801
) -> Self {
797802
let key = TypeKey::parse(impl_fn_key.as_ref());
798803
let dispatcher_path_str = validate_path("callback_input", dispatcher_path.as_ref());
799804
let body_path = dispatcher_path_str.clone();
800-
// Capture the exception name (or absence) for the builder's triple.
801-
let exc_name = exc.map(str::to_string);
805+
// `syn::Type` holds `Rc<TokenStream>` internally and is neither
806+
// `Send` nor `Sync`, so we can't capture it directly in a builder
807+
// closure that satisfies `WrapperBuilder<Arity0>`'s `Send + Sync`
808+
// bounds. Serialise to its canonical token form here and re-parse
809+
// inside the closure — same dance the path captures use.
810+
let exc_str = exc.as_ref().map(|t| t.to_token_stream().to_string());
802811
let builder = move |_reg: &Registry<KotlinMeta>| {
803812
let path: syn::Path = syn::parse_str(&body_path).ok()?;
804813
// Throwing: dispatcher already returns `Result<_, exc>` — emit
805814
// the call verbatim. Non-throwing: framework `?`-propagation
806815
// unwraps, and the framework `Ok`-wraps later.
807-
let body: syn::Expr = if exc_name.is_some() {
816+
let body: syn::Expr = if exc_str.is_some() {
808817
syn::parse_quote!(#path(env, &v))
809818
} else {
810819
syn::parse_quote!(#path(env, &v)?)
811820
};
821+
let exc_ty = exc_str.as_deref().and_then(|s| syn::parse_str::<syn::Type>(s).ok());
812822
Some((
813823
syn::parse_quote!(jni::objects::JObject),
814-
exc_name.clone(),
824+
exc_ty,
815825
body,
816826
))
817827
};
@@ -892,10 +902,11 @@ impl JniExt {
892902
/// * `exc = None` ⇒ non-throwing: `body` evaluates to a bare `ty`;
893903
/// framework emits `-> Result<ty, __JniErr>` with an `Ok(...)`
894904
/// wrap, and `?` inside propagates the framework error.
895-
/// * `exc = Some("X")` ⇒ throwing: `body` evaluates to
896-
/// `Result<ty, X>`; framework emits it verbatim. `"X"` matches a
897-
/// [`Self::kotlin_exception_class`] by Rust path or short name
898-
/// (or `"JniBindingError"` for the framework default); the name
905+
/// * `exc = Some(<Rust type>)` ⇒ throwing: `body` evaluates to
906+
/// `Result<ty, <Rust type>>`; framework emits it verbatim. The
907+
/// type must match a [`Self::kotlin_exception_class`] declaration
908+
/// by **exact canonical-form equality** with its `rust_path` (see
909+
/// [`Self::find_exception`] — no short-name fallback). The match
899910
/// is validated at lookup time.
900911
///
901912
/// `ty` is auto-classified at resolve: a wire shape ⇒ terminal
@@ -915,9 +926,10 @@ impl JniExt {
915926
}
916927

917928
/// Output-direction counterpart of [`Self::input_wrapper`]. Same
918-
/// closure shape, same `exc = None` / `Some("X")` semantics, same
919-
/// terminal-vs-composed classification — see that method's docs.
920-
/// (`Some("X")` with a rust-typed `ty`, e.g. `("T", "ZError", v)` for
929+
/// closure shape, same `exc = None` / `Some(<Rust type>)` semantics,
930+
/// same terminal-vs-composed classification — see that method's docs.
931+
/// (`Some(parse_quote!(<full path>))` with a rust-typed `ty`, e.g.
932+
/// `(T, Some(parse_quote!(zenoh_flat::errors::ZError)), v)` for
921933
/// `ZResult<T>`, gives the auto-composed peel that the deleted
922934
/// `output_throw_stage` used to register.)
923935
pub fn output_wrapper<A, B>(self, pattern: impl AsRef<str>, builder: B) -> Self
@@ -951,32 +963,30 @@ impl JniExt {
951963

952964
/// [`Self::find_exception`] with a uniform fail-fast panic. `who` is
953965
/// the caller name for the message.
954-
fn find_exception_or_panic(&self, who: &str, exc_path: &str) -> usize {
955-
self.find_exception(exc_path).unwrap_or_else(|| {
966+
fn find_exception_or_panic(&self, who: &str, ty: &syn::Type) -> usize {
967+
self.find_exception(ty).unwrap_or_else(|| {
968+
let needle = ty.to_token_stream().to_string();
956969
panic!(
957-
"JniExt::{who}: no exception class registered matching `{exc_path}` — \
958-
declare it via `.kotlin_exception_class(\"<rust path>\")` first \
959-
(or use `\"JniBindingError\"` for the framework default)"
970+
"JniExt::{who}: no exception class registered matching `{needle}` — \
971+
declare it via `.kotlin_exception_class(\"<full rust path>\")` first, \
972+
and bind closures to it with `Some(parse_quote!(<the same path>))`. \
973+
The framework default is `::prebindgen_ext::jni::JniBindingError` (or \
974+
omit the closure's middle slot — pass `None` — for non-throwing)."
960975
)
961976
})
962977
}
963978

964-
/// Resolve an exception class name (Rust path or short name) against
965-
/// the registered [`Self::exceptions`]. Returns the index into the
966-
/// `exceptions` vec on match.
967-
fn find_exception(&self, name_or_path: &str) -> Option<usize> {
968-
// Match canonical rust_path first (most specific), then short.
969-
// `find_map` consumes either form; the framework slot at index 0
970-
// is addressable as both `"JniBindingError"` and the full path.
971-
if let Some(idx) = self.exceptions.iter().position(|e| {
972-
e.rust_path.to_token_stream().to_string().replace(' ', "")
973-
== name_or_path.replace(' ', "")
974-
}) {
975-
return Some(idx);
976-
}
977-
self.exceptions
978-
.iter()
979-
.position(|e| e.rust_short == name_or_path)
979+
/// Resolve an exception type against the registered
980+
/// [`Self::exceptions`] by **exact canonical-form equality** with the
981+
/// declaration's `rust_path`. No short-name fallback — the closure /
982+
/// caller must spell the same full path
983+
/// `.kotlin_exception_class(...)` was declared with. Returns the
984+
/// index into the `exceptions` vec on match.
985+
fn find_exception(&self, ty: &syn::Type) -> Option<usize> {
986+
let needle = ty.to_token_stream().to_string();
987+
self.exceptions.iter().position(|e| {
988+
e.rust_path.to_token_stream().to_string() == needle
989+
})
980990
}
981991

982992
/// The framework's pre-registered [`crate::jni::JniBindingError`]
@@ -1014,8 +1024,9 @@ impl JniExt {
10141024
/// Look up a registered input converter for `pat` with `args`
10151025
/// substituted into its `_` slots. The closure's middle slot (see
10161026
/// [`WrapperFn`]) carries the bound exception — `None` ⇒ framework
1017-
/// `__JniErr` with an `Ok`-wrap, `Some("X")` ⇒ `Result<ty, X>`
1018-
/// emitted verbatim, decided in [`Self::build_input_fn`].
1027+
/// `__JniErr` with an `Ok`-wrap, `Some(<Rust type>)` ⇒
1028+
/// `Result<ty, <Rust type>>` emitted verbatim, decided in
1029+
/// [`Self::build_input_fn`].
10191030
///
10201031
/// The closure's returned type is classified by [`is_wire_type`]:
10211032
/// * **wire** ⇒ terminal: a single converter `wire → outer`.
@@ -1036,14 +1047,14 @@ impl JniExt {
10361047
}
10371048
let key = TypeKey::from_type(pat);
10381049
let f = self.input_wrappers[rank].get(&key)?;
1039-
let (ty, exc_name, body) = f(args, registry)?;
1040-
// Resolve the exception name lazily: validated here, at lookup
1050+
let (ty, exc_ty, body) = f(args, registry)?;
1051+
// Resolve the exception type lazily: validated here, at lookup
10411052
// time, rather than at the `input_wrapper` call site — the
10421053
// closure is the single source of truth for both body shape and
10431054
// bound exception (see [`WrapperFn`]).
1044-
let exc = exc_name
1045-
.as_deref()
1046-
.map(|n| &self.exceptions[self.find_exception_or_panic("input_wrapper", n)]);
1055+
let exc = exc_ty
1056+
.as_ref()
1057+
.map(|t| &self.exceptions[self.find_exception_or_panic("input_wrapper", t)]);
10471058
let outer = substitute_wildcards(pat, args);
10481059
let throw_exc = exc.unwrap_or_else(|| self.framework_exception());
10491060
// Terminal vs composed: `ty` is composed iff it's a *distinct*
@@ -1150,11 +1161,11 @@ impl JniExt {
11501161
}
11511162
let key = TypeKey::from_type(pat);
11521163
let f = self.output_wrappers[rank].get(&key)?;
1153-
let (ty, exc_name, body) = f(args, registry)?;
1164+
let (ty, exc_ty, body) = f(args, registry)?;
11541165
// Resolve at lookup — see [`Self::lookup_input`] for the rationale.
1155-
let exc = exc_name
1156-
.as_deref()
1157-
.map(|n| &self.exceptions[self.find_exception_or_panic("output_wrapper", n)]);
1166+
let exc = exc_ty
1167+
.as_ref()
1168+
.map(|t| &self.exceptions[self.find_exception_or_panic("output_wrapper", t)]);
11581169
let outer = substitute_wildcards(pat, args);
11591170
let throw_exc = exc.unwrap_or_else(|| self.framework_exception());
11601171
// Terminal vs composed — see [`Self::lookup_input`] for the rule.
@@ -1835,7 +1846,7 @@ impl PrebindgenExt for JniExt {
18351846
// their `?` failures into this type via its `From<String>`
18361847
// impl, so a built-in decode failure surfaces as
18371848
// `JniBindingError` on the JVM. Throwing converters
1838-
// (closures returning `Some("X")` in the middle slot of
1849+
// (closures returning `Some(parse_quote!(<full path>))` in the middle slot of
18391850
// `input_wrapper` / `output_wrapper`) instead emit functions
18401851
// typed `Result<…, X>` — they bypass `__JniErr` entirely so no
18411852
// cross-type bridge between the framework error and a domain
@@ -2126,7 +2137,7 @@ impl PrebindgenExt for JniExt {
21262137
}
21272138
// No built-in Result/ZResult special-case: bindings register any
21282139
// throw-on-Err behaviour via
2129-
// an `output_wrapper("ZResult < _ >", |t,_| Some((t, Some("X"), v)))`
2140+
// an `output_wrapper("ZResult < _ >", |t,_| Some((t, Some(parse_quote!(...)), v)))`
21302141
// whose closure returns a rust type, which routes through
21312142
// `lookup_output` above (rust-typed continue ⇒ auto-composed peel).
21322143
if pat_match(pat, "Option < _ >") {
@@ -2195,7 +2206,7 @@ fn emit_jni_function_wrapper(ext: &JniExt, f: &syn::ItemFn, registry: &Registry<
21952206
// `match`-arms that dispatch to the input converter's own throw fn
21962207
// on `Err` and `return <sentinel>;` — so a malformed `Encoding`
21972208
// JObject raises `JniBindingError`, while a throwing input wrapper
2198-
// raises whatever exception it bound via `Some("X")` in the closure.
2209+
// raises whatever exception it bound via `Some(parse_quote!(...))` in the closure.
21992210
let mut prelude: Vec<TokenStream> = Vec::new();
22002211
let mut call_args: Vec<TokenStream> = Vec::new();
22012212

@@ -2209,7 +2220,8 @@ fn emit_jni_function_wrapper(ext: &JniExt, f: &syn::ItemFn, registry: &Registry<
22092220
panic!(
22102221
"JniExt::on_function: return type `{}` of `{}` has no registered output \
22112222
converter — register one via `JniExt::output_wrapper(pat, |…| Some((ty, exc, body)))` \
2212-
(exc = `None` for non-throwing, `Some(\"<exc>\")` to bind a domain exception)",
2223+
(exc = `None` for non-throwing, `Some(parse_quote!(<full path>))` \
2224+
to bind a domain exception)",
22132225
TypeKey::from_type(&return_ty),
22142226
original_ident,
22152227
)
@@ -2242,7 +2254,8 @@ fn emit_jni_function_wrapper(ext: &JniExt, f: &syn::ItemFn, registry: &Registry<
22422254
let conv = entry.function.sig.ident.clone();
22432255
// Each input converter carries the throw fn for its failures —
22442256
// framework `throw_JniBindingError` by default, or a custom one
2245-
// bound via `Some("<exc>")` in the input wrapper's closure.
2257+
// bound via `Some(parse_quote!(<full path>))` in the input
2258+
// wrapper's closure.
22462259
let input_throw = entry
22472260
.metadata
22482261
.throws_action

prebindgen-ext/src/jni/jni_kotlin_ext.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,11 +1271,12 @@ fn render_wrapper_fn(
12711271
// * each input parameter's wire-facing converter (its `?` failure
12721272
// raises the metadata `throws` exception — framework
12731273
// `JniBindingError` by default, or a custom one bound via
1274-
// `Some("<exc>")` in the input wrapper's closure);
1274+
// `Some(parse_quote!(<full path>))` in the input wrapper's
1275+
// closure);
12751276
// * each pre_stage on that input's chain (value-inspecting throw
12761277
// stages — an `input_wrapper` / `output_wrapper` whose closure
1277-
// returns a rust type with `Some("<exc>")` and gets composed
1278-
// onto that type's converter);
1278+
// returns a rust type with `Some(parse_quote!(<full path>))`
1279+
// and gets composed onto that type's converter);
12791280
// * the return type's output converter and its pre_stages
12801281
// (likewise).
12811282
// Collected into a `BTreeSet` so the emitted annotation is sorted and

0 commit comments

Comments
 (0)