Skip to content

Commit 248f645

Browse files
committed
niche fix
1 parent d380b96 commit 248f645

5 files changed

Lines changed: 270 additions & 52 deletions

File tree

prebindgen-ext/src/jni/jni_ext.rs

Lines changed: 112 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,42 @@ use crate::util::snake_to_camel;
4747
/// [`FoldStrategy::Iterable`]. Drives both the typed Kotlin rendering of
4848
/// a handle-bearing field/return and the generated `close()` expression,
4949
/// uniformly across whatever wrappers compose.
50+
/// How a `Nullable` fold layer represents `None` over the JNI wire.
51+
///
52+
/// The choice is made at the point the `Option<_>` rank-1 handler folds the
53+
/// layer onto a projection's `FoldStrategy`, and only depends on whether
54+
/// `option_output` rode the inner converter's niche (wire stayed identical to
55+
/// the inner's wire) or boxed the primitive into `java.lang.<Box>` (wire
56+
/// widened to `JObject`). The renderer reads this to pick the matching
57+
/// Kotlin shape — without it, a primitive-wired `Option<Handle>` would be
58+
/// declared as nullable `Long?` even though the wire is a non-nullable
59+
/// `jlong` whose `0L` *is* the null.
60+
#[derive(Clone, Debug, PartialEq, Eq)]
61+
pub enum NullableKind {
62+
/// Wire kept the inner converter's encoding; `None` is the carved niche
63+
/// slot's `value` (e.g. `0L` for a handle's `jlong`). On Kotlin the
64+
/// declared wire is non-nullable; the wrapper body converts the sentinel
65+
/// to `null` explicitly via an `if (it == <sentinel>) null else W(it)`
66+
/// pattern.
67+
Niche,
68+
/// Wire widened to `JObject`; `None` is JVM `null`. On Kotlin the
69+
/// declared wire is nullable and `?.let { W(it) }` works directly. This
70+
/// is also the rendering object-shaped niches (`JByteArray::null` /
71+
/// `JString::null`) collapse onto — Kotlin's `T?` already maps to JVM
72+
/// reference-null at no extra cost.
73+
Boxed,
74+
}
75+
5076
#[derive(Clone, Debug)]
5177
pub enum FoldStrategy {
5278
/// The receiver *is* the handle.
5379
Direct,
54-
/// `T?` — receiver may be null.
55-
Nullable(Box<FoldStrategy>),
80+
/// `T?` — receiver may be null. `kind` records how null is represented
81+
/// over the wire (see [`NullableKind`]).
82+
Nullable {
83+
kind: NullableKind,
84+
inner: Box<FoldStrategy>,
85+
},
5686
/// `List<T>` — receiver is a collection. EXTENSION POINT: no
5787
/// `Vec<Handle>` shape exists today, so the emitters guard this arm
5888
/// loudly rather than silently mis-generating.
@@ -2859,7 +2889,13 @@ impl PrebindgenExt for JniExt {
28592889
);
28602890
let projection = inner.metadata.projection.clone().map(|h| Projection {
28612891
owned: false,
2862-
strategy: FoldStrategy::Nullable(Box::new(h.strategy)),
2892+
// `Option<&Handle>` always rides the inner's `*v == 0` niche
2893+
// (body is `if *v == 0 { None } else { ... }` above), so
2894+
// null is the `0i64` sentinel — never JVM boxed.
2895+
strategy: FoldStrategy::Nullable {
2896+
kind: NullableKind::Niche,
2897+
inner: Box::new(h.strategy),
2898+
},
28632899
..h
28642900
});
28652901
return Some(ConverterImpl {
@@ -2940,13 +2976,23 @@ impl PrebindgenExt for JniExt {
29402976
.input_entry(t1)
29412977
.and_then(|e| e.metadata.kotlin_name.clone());
29422978
let kotlin_name = self.override_kotlin_name(&outer_ty, inherited);
2943-
// Fold a Nullable layer over the inner handle (if any), so an
2944-
// `Option<Handle>` field/param carries the full close strategy.
2979+
// Fold a Nullable layer over the inner projection (if any). The
2980+
// kind mirrors which path `option_input` took: when it consumed
2981+
// an inner niche, the wire stays identical to the inner's
2982+
// destination (e.g. `jlong` for handles, `JByteArray` for
2983+
// ByteArray-shaped value classes) and `None` is the niche slot
2984+
// sentinel; the boxed fallback widens the wire to `JObject`. The
2985+
// renderer reads `kind` so the Kotlin declared wire and wrap
2986+
// shape match the runtime ABI.
2987+
let nullable_kind = nullable_kind_for(&wire, t1, registry);
29452988
let projection = registry
29462989
.input_entry(t1)
29472990
.and_then(|e| e.metadata.projection.clone())
29482991
.map(|h| Projection {
2949-
strategy: FoldStrategy::Nullable(Box::new(h.strategy)),
2992+
strategy: FoldStrategy::Nullable {
2993+
kind: nullable_kind,
2994+
inner: Box::new(h.strategy),
2995+
},
29502996
..h
29512997
});
29522998
return Some(ConverterImpl {
@@ -3159,13 +3205,20 @@ impl PrebindgenExt for JniExt {
31593205
.output_entry(t1)
31603206
.and_then(|e| e.metadata.kotlin_name.clone());
31613207
let kotlin_name = self.override_kotlin_name(&outer_ty, inherited);
3162-
// Fold a Nullable layer over the inner handle (if any), so an
3163-
// `Option<Handle>` output carries the full close strategy.
3208+
// Fold a Nullable layer over the inner projection (if any). The
3209+
// kind reflects which path `option_output` took (see
3210+
// [`nullable_kind_for`]): niche-fulfilled keeps the inner wire
3211+
// and treats the slot value as `None`; boxed widens to `JObject`
3212+
// and uses JVM null.
3213+
let nullable_kind = nullable_kind_for_output(&wire, t1, registry);
31643214
let projection = registry
31653215
.output_entry(t1)
31663216
.and_then(|e| e.metadata.projection.clone())
31673217
.map(|h| Projection {
3168-
strategy: FoldStrategy::Nullable(Box::new(h.strategy)),
3218+
strategy: FoldStrategy::Nullable {
3219+
kind: nullable_kind,
3220+
inner: Box::new(h.strategy),
3221+
},
31693222
..h
31703223
});
31713224
return Some(ConverterImpl {
@@ -4150,7 +4203,7 @@ pub(crate) fn handle_field_fqn(ext: &JniExt, h: &Projection) -> String {
41504203
fn assert_scalar(s: &FoldStrategy) {
41514204
match s {
41524205
FoldStrategy::Direct => {}
4153-
FoldStrategy::Nullable(inner) => assert_scalar(inner),
4206+
FoldStrategy::Nullable { inner, .. } => assert_scalar(inner),
41544207
FoldStrategy::Iterable(_) => panic!(
41554208
"struct handle field: collection (Vec<Handle>) layers are not yet \
41564209
supported by the struct encode/decode bridge — add array codegen \
@@ -4973,6 +5026,54 @@ fn value_class_inner_type(
49735026
Some(n.named.first()?.ty.clone())
49745027
}
49755028

5029+
/// Decide which [`NullableKind`] to fold for an `Option<_>` wrapper, given
5030+
/// the wrapper's destination wire and the registry-resolved inner. The
5031+
/// detection mirrors the two paths in [`option_input`] / [`option_output`]:
5032+
/// the niche path keeps the inner's wire untouched (e.g. `jlong` stays
5033+
/// `jlong`, `JByteArray` stays `JByteArray`), while the boxed-primitive
5034+
/// fallback widens the wire to `JObject`. So `outer_wire == inner.destination`
5035+
/// uniquely identifies the niche path.
5036+
///
5037+
/// Symmetric `_input` / `_output` flavors only differ in which registry side
5038+
/// they consult — the comparison is identical.
5039+
fn nullable_kind_for(
5040+
outer_wire: &syn::Type,
5041+
inner_ty: &syn::Type,
5042+
registry: &Registry<KotlinMeta>,
5043+
) -> NullableKind {
5044+
let inner_dest = registry
5045+
.input_entry(inner_ty)
5046+
.map(|e| e.destination.clone())
5047+
.expect(
5048+
"nullable_kind_for: Option<_> input handler reached here only after option_input \
5049+
returned Some, so the inner's input entry must exist",
5050+
);
5051+
if outer_wire == &inner_dest {
5052+
NullableKind::Niche
5053+
} else {
5054+
NullableKind::Boxed
5055+
}
5056+
}
5057+
5058+
fn nullable_kind_for_output(
5059+
outer_wire: &syn::Type,
5060+
inner_ty: &syn::Type,
5061+
registry: &Registry<KotlinMeta>,
5062+
) -> NullableKind {
5063+
let inner_dest = registry
5064+
.output_entry(inner_ty)
5065+
.map(|e| e.destination.clone())
5066+
.expect(
5067+
"nullable_kind_for_output: Option<_> output handler reached here only after \
5068+
option_output returned Some, so the inner's output entry must exist",
5069+
);
5070+
if outer_wire == &inner_dest {
5071+
NullableKind::Niche
5072+
} else {
5073+
NullableKind::Boxed
5074+
}
5075+
}
5076+
49765077
/// The single named field of a `value_class` struct — `(ident, type)`. Used by
49775078
/// the rank-0 value-class leaf to delegate to the inner field's converter.
49785079
fn value_class_inner_field(s: &syn::ItemStruct) -> Option<(syn::Ident, syn::Type)> {
@@ -4995,7 +5096,7 @@ fn value_class_descriptor(
49955096
fn is_iterable(s: &FoldStrategy) -> bool {
49965097
match s {
49975098
FoldStrategy::Iterable(_) => true,
4998-
FoldStrategy::Nullable(inner) => is_iterable(inner),
5099+
FoldStrategy::Nullable { inner, .. } => is_iterable(inner),
49995100
FoldStrategy::Direct => false,
50005101
}
50015102
}

0 commit comments

Comments
 (0)