[generator] Phase 2: Kotlin @JvmInline value class projection#1440
[generator] Phase 2: Kotlin @JvmInline value class projection#1440jonathanpeppers wants to merge 6 commits into
Conversation
Phase 2 of #1431. Surface inline-class info on the api.xml so the generator can later project parameters/returns to strongly-typed wrapper structs while keeping JNI marshaling on the underlying primitive. * Stamp ClassFile.KotlinInlineClassUnderlyingJniType with the JNI descriptor of the single non-synthetic instance field on every Kotlin '@JvmInline value class'. * Stamp MethodInfo.KotlinInlineClassReturnJniType / ParameterInfo. KotlinInlineClassJniType when a method's Kotlin source-level return or parameter type was an inline class (the JVM-erased type is the inline class's backing primitive). * Emit kotlin-inline-class / kotlin-inline-class-underlying-jni-type on <class>, kotlin-inline-class-jni-type on <parameter>, and kotlin-inline-class-return-jni-type on <method>. * Bytecode tests cover the existing kotlin-gradle/ MyColor / MyAlpha / MyDp / Widgets fixture: ULong-backed and Float-backed detection, per-parameter stamping, return-type stamping, and round-trip through XmlClassDeclarationBuilder. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Phase 2 of #1431. Wires the inline-class XML attributes from the class-parse layer (commit 35ccc4f) through the generator so: * `<class kotlin-inline-class="true" .../>` is emitted as a `readonly partial struct` wrapper around the underlying primitive (e.g. `J` -> `long`, `F` -> `float`) instead of a peer-class binding. The struct exposes `Value`, implicit conversions, equality, and ToString. * `<parameter kotlin-inline-class-jni-type="L...;" .../>` makes the generator project the parameter's managed type to the wrapper struct while keeping JNI marshaling on the underlying primitive. Because the struct has implicit conversion operators, existing `JniArgumentValue` thunks compile unchanged. * `<method kotlin-inline-class-return-jni-type="L...;" .../>` does the same for return values via `ReturnValue.managed_type`. Plumbing: * `ClassGen.IsKotlinInlineClass`, `ClassGen.KotlinInlineClassUnderlyingJniType` * `Parameter.KotlinInlineClassJniType` * `Method.KotlinInlineClassReturnJniType` * `Parameter.Validate`/`ReturnValue.Validate` apply the projection by looking up the wrapper `ClassGen` via `SymbolTable`. * New `KotlinInlineClassStruct` TypeWriter emits the wrapper struct. * New `TypeNameUtilities.JniSignatureToJavaTypeName` helper. Out of scope (acknowledged limitations): boxed/nullable/generic inline-class positions still resolve to the (now-replaced) peer binding; reference-backed inline classes; generic inline classes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds Kotlin @JvmInline value class awareness end-to-end (bytecode → api.xml → generator) so inline-class parameters/returns can be projected as strongly-typed C# wrappers while preserving JNI marshaling on the JVM-erased underlying primitive.
Changes:
- Detect Kotlin inline/value classes in
class-parse, stamp underlying JNI primitive info, and emit new inline-class attributes intoapi.xml. - Import the new attributes in the generator and project method parameters/returns to inline-class wrapper types.
- Emit
readonly partial structwrapper types for inline classes and add unit tests covering import/projection/round-tripping.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/generator/Utilities/TypeNameUtilities.cs | Adds helper to map JNI reference signatures to Java type names for symbol lookup. |
| tools/generator/SourceWriters/KotlinInlineClassStruct.cs | New type writer emitting the C# readonly struct wrapper for inline classes. |
| tools/generator/Java.Interop.Tools.Generator.ObjectModel/ReturnValue.cs | Projects return types to inline-class wrapper structs when stamped. |
| tools/generator/Java.Interop.Tools.Generator.ObjectModel/Parameter.cs | Projects parameter types to inline-class wrapper structs when stamped; preserves clone data. |
| tools/generator/Java.Interop.Tools.Generator.ObjectModel/Method.cs | Adds storage for stamped inline-class return JNI type. |
| tools/generator/Java.Interop.Tools.Generator.ObjectModel/ClassGen.cs | Adds inline-class flags/underlying descriptor to the object model. |
| tools/generator/Java.Interop.Tools.Generator.Importers/XmlApiImporter.cs | Reads new inline-class XML attributes into the generator model. |
| tools/generator/Java.Interop.Tools.Generator.CodeGeneration/JavaInteropCodeGenerator.cs | Routes inline classes to the new struct writer. |
| tests/Xamarin.Android.Tools.Bytecode-Tests/Xamarin.Android.Tools.Bytecode-Tests.targets | Adjusts embedded Kotlin fixture class list. |
| tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinInlineClassCollisionTests.cs | Adds tests asserting fixup stamping and XML attribute emission. |
| tests/generator-Tests/Unit-Tests/KotlinInlineClassTests.cs | Adds tests covering XML import and the new helper. |
| src/Xamarin.Android.Tools.Bytecode/XmlClassDeclarationBuilder.cs | Emits new inline-class XML attributes for classes/methods/parameters. |
| src/Xamarin.Android.Tools.Bytecode/Methods.cs | Adds fields to carry stamped inline-class parameter/return JNI signatures. |
| src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs | Detects inline classes and stamps inline-class parameter/return metadata for projection. |
| src/Xamarin.Android.Tools.Bytecode/ClassFile.cs | Stores inline-class underlying JNI descriptor on the ClassFile. |
Wires the four Kotlin .class files compiled by the kotlin-gradle/
fixture under tests/Xamarin.Android.Tools.Bytecode-Tests/ into a
generator-level test that exercises the full Phase 2 pipeline:
bytecode (KotlinFixups + XmlClassDeclarationBuilder)
-> api.xml string
-> XmlApiImporter.Parse + Validate
-> JavaInteropCodeGenerator.WriteType
and asserts the projected C# output emits readonly partial structs
for MyColor/MyAlpha/MyDp, projects them in Widgets.tint/pad method
signatures, and never falls back to peer-class bindings for them.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When the Kotlin compiler mangles a JVM method name for inline-class
binary compatibility (e.g. `tint(MyColor)` -> `tint-Rn_QMJI`), recover
the unmangled Kotlin source name and surface it in api.xml as the
`managedName` attribute. The mangled JVM name still appears in
`name`/`jni-signature` so JNI invocation targets the actual method.
With this and the Phase 2 inline-class -> struct projection, methods
that erase to colliding JVM signatures emit as plain C# overloads
distinguished by struct type, e.g.:
void Tint (MyColor color);
void Tint (MyAlpha alpha);
void Tint (MyDp dp);
instead of the previous `Tint_Rn_QMJI` / `Tint_uzYZ1wI` mangled names.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1. KotlinInlineClassStruct.cs: drop unused `using System;` and unused `klass` field; gate `object?`/`string?` annotations on `opt.NullableOperator` so consumers without nullable enabled don't get CS8632 warnings. 2. TypeNameUtilities.JniSignatureToJavaTypeName: also translate `\$` (JNI nested-type separator) to `.` so SymbolTable.Lookup() resolves nested inline classes. 3. KotlinFixups.DetectInlineClasses: only stamp underlying-JNI type when there is exactly one non-synthetic instance field AND that field is a JVM primitive descriptor (Z/B/C/D/F/I/J/S). Reference- backed inline classes (e.g. `value class Tag(val s: String)`) are skipped — the wrapper struct currently emits a primitive `Value` field, so they would otherwise produce wrong bindings. 4. KotlinFixups.GetInlineClassJniType: now also takes the JVM-erased descriptor of the position being projected (param/return/property) and only returns a JNI-type when it equals the inline class's underlying primitive. Boxed / nullable / generic inline-class positions (where the JVM signature stays `L...;`) are no longer incorrectly stamped — they fall through to the legacy peer-class binding path so JNI marshaling stays consistent. Applied at all four stamping sites: function param, function return, property getter return, property setter param. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Java.Interop PR Reviewer completed successfully! |
There was a problem hiding this comment.
✅ LGTM — well-designed feature with minor issues
Summary: This PR adds Kotlin @JvmInline value class projection through the full pipeline (bytecode → api.xml → generator → C# wrapper structs). The architecture is clean — using the existing managedName attribute for name unmangling, projecting managed types while keeping JNI marshaling on primitives via the sym / managed_type split, and carefully filtering to only primitive-backed inline classes.
Issue counts:
⚠️ 2 warnings (silent fallback inJniPrimitiveToCSharpType; null-forgiving operator!usage ×4)- 💡 1 suggestion (
klass2naming)
Positive callouts:
- The two-pass approach in
DetectInlineClasses(pre-scan all classes before processing methods) correctly handles cross-class references regardless of class ordering. - Good defensive filtering: only primitive-backed inline classes are projected; reference-backed and boxed positions are explicitly excluded with clear documentation of why.
- The end-to-end test (
KotlinInlineClassEndToEndTests) exercises the real Kotlin.classfixtures through the full pipeline — much stronger than testing against hand-crafted XML alone. - Thorough comments linking each piece to the issue tracker (#1431) and explaining the "why" behind design decisions (e.g., why boxed positions are skipped).
Method.Clonecorrectly copiesKotlinInlineClassReturnJniType, andParameter.Clonecorrectly copiesKotlinInlineClassJniType.
CI: dotnet.java-interop is still queued/pending. license/cla passed.
Generated by Java.Interop PR Reviewer for issue #1440 · ● 10.6M
- XmlClassDeclarationBuilder: drop null-forgiving '!' (4 sites). The string.IsNullOrEmpty guard's [NotNullWhen(false)] annotation already narrows the type, so the operator was unnecessary and violated the repo convention banning '!'. - JavaInteropCodeGenerator.WriteType: collapse the 'klass'/'klass2' duplicate pattern variables into a nested if-else inside a single ClassGen match. - KotlinInlineClassStruct.JniPrimitiveToCSharpType: replace the silent '_ => \"long\"' fallback with ArgumentOutOfRangeException. DetectInlineClasses already filters to primitive descriptors, so this branch is unreachable for valid input - failing fast prevents a non-primitive from silently producing a wrong 'long' wrapper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the seven hand-rolled `[Flags]` enums in ComposeDefaults.cs
(`ButtonDefault`, `TextDefault`, `IconButtonDefault`,
`FloatingActionButtonDefault`, `SurfaceDefault`, `AlertDialogDefault`,
`TextFieldDefault`) with a new declarative form of
`ComposeDefaultsAttribute` that takes a positional Kotlin parameter
list:
[assembly: ComposeDefaults("ButtonDefault",
"!onClick", "modifier", "enabled", ..., "!content")]
Names prefixed with `!` consume a bit position but emit no enum member,
covering Compose params the caller always provides (onClick, content,
text, value, etc.). Optional slot params stay as members so call sites
can clear bits per-call (the AlertDialog pattern).
These seven were hand-rolled because the dotnet/android-libraries
binder strips the Compose overloads with mangled JVM names from inline
classes (Text--4IGK_g, Surface-T9BRK9s, AlertDialog-Oix01E0, ...), so
no IMethodSymbol exists for the existing generic
`[ComposeDefaults<T>]` form to read. The declarative form is a
near-term workaround; once dotnet/java-interop#1440 lands and exposes
those overloads, each declarative attribute can be swapped one-for-one
for the generic form.
Generator changes:
- New non-generic `ComposeDefaultsAttribute(string, params string[])`.
- `ComposeDefaultsGenerator` dispatches on attribute kind. Shared
`ComposeDefaultsEmitter` core driven by a `Slot` list, populated
either from `IMethodSymbol` (generic) or from the names array
(declarative).
- Two new tests covering the declarative path; existing eight unchanged.
Verified the generated enums match the deleted hand-rolled values
byte-for-byte (bit positions and `All` mask) by enabling
EmitCompilerGeneratedFiles and diffing AlertDialogDefault and
SurfaceDefault.
Also adds .github/copilot-instructions.md documenting the layout,
build/test commands, both attribute forms, the ComposeBridges JNI
recipe, the facade conventions (ComposableNode/Container,
ComposableLambdaN, named-slot AlertDialog pattern), the bindings
policy, and style.
### Fix CN1003 message to cover both [ComposeDefaults] forms
The diagnostic message previously said `[ComposeDefaults<T>(string)]`,
which was wrong for both forms: the generic overload takes two
strings, and the declarative overload isn't generic at all. Make
the message form-agnostic.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Phase 2 of #1431. Builds on #1432 (sibling-collision dedup) by detecting Kotlin
@JvmInline value classtypes inclass-parse, surfacing them inapi.xml, and projecting them into generator output asreadonly structwrappers — while JNI marshaling stays on the underlying primitive.What this does
Given Kotlin source like:
The generator now emits:
The bodies of
Tint/Padstill pass the JVM-erased primitive (long) across JNI — the implicit conversion operators on the struct make that compile without any thunk changes.Bytecode layer (commit 35ccc4f)
ClassFile.KotlinInlineClassUnderlyingJniType,MethodInfo.KotlinInlineClassReturnJniType,ParameterInfo.KotlinInlineClassJniType.KotlinFixups.Fixupdoes a pre-passDetectInlineClassesthat recognizes@JvmInline+KotlinClassFlags.IsInlineClass+ a single non-synthetic instance field, then stamps each method's parameter/return JNI types when the Kotlin source-level type was an inline class.XmlClassDeclarationBuilderemitskotlin-inline-class,kotlin-inline-class-underlying-jni-type,kotlin-inline-class-jni-type(parameter), andkotlin-inline-class-return-jni-type(method) attributes.KotlinInlineClassCollisionTests.Generator layer (this commit)
ClassGen.IsKotlinInlineClass/ClassGen.KotlinInlineClassUnderlyingJniType.Parameter.KotlinInlineClassJniType/Method.KotlinInlineClassReturnJniType.XmlApiImporterreads the new attributes.Parameter.Validate/ReturnValue.Validateapply the projection by looking up the wrapperClassGenviaSymbolTableand overridingmanaged_typesoType/FullNamereturn the struct whileSymbol(and therefore JNI marshaling) stays on the underlying primitive.KotlinInlineClassStructTypeWriteremits the wrapper struct.JavaInteropCodeGenerator.WriteTyperoutes inline-classClassGeninstances to the new writer instead ofBoundClass.TypeNameUtilities.JniSignatureToJavaTypeNamehelper.KotlinInlineClassTests.Out of scope / known limitations
MyColor?, generics likeList<MyColor>, supertype implementations) still emitLcom/example/MyColor;references that no longer have a peer class binding. These were rare on the targeted Compose surface; users can unblock them via Metadata.xml (<remove-node>) until a follow-up.value class Tag(val raw: String)) are detected as inline classes but the projection path expects a primitive backing field; non-primitive-backed inline classes are not yet projected.value class Wrap<T>(val v: T)).material3-androidAAR is not part of this PR.Tests
Xamarin.Android.Tools.Bytecode-Tests: 82 passed, 0 failed (2 pre-existing skips).generator-Tests: 7 new tests pass; total matches baseline (432 passed; 23 failures are pre-existing environment issues unrelated to this change, confirmed by stashing the changes and re-running).Closes part of #1431. Phase 1 (sibling-collision dedup) was #1432.