Skip to content

[TrimmableTypeMap] Add trimmable [Export] and [ExportField] callback support#11123

Open
simonrozsival wants to merge 39 commits intodev/simonrozsival/trimmable-test-plumbingfrom
dev/simonrozsival/trimmable-typemap-export-attribute
Open

[TrimmableTypeMap] Add trimmable [Export] and [ExportField] callback support#11123
simonrozsival wants to merge 39 commits intodev/simonrozsival/trimmable-test-plumbingfrom
dev/simonrozsival/trimmable-typemap-export-attribute

Conversation

@simonrozsival
Copy link
Copy Markdown
Member

@simonrozsival simonrozsival commented Apr 16, 2026

Summary

Add UCO (UnmanagedCallersOnly) wrapper codegen for [Export] methods and [ExportField] fields in the trimmable typemap pipeline.

Depends on #11091 (trimmable test plumbing + CoreCLRTrimmable CI lane).

Part of #10788

Changes

Export method dispatch support

  • Scanner: Detect [Export] and [ExportField] attributes on Java peer types; resolve JNI signatures for non-primitive Java-bound parameter types; collect Java access modifiers and throws clauses
  • Model: MarshalMethodInfo carries IsExport, JavaAccess, ThrownNames, and SuperArgumentsString for export metadata
  • JCW Java codegen: Generate Java native methods with correct access modifiers and throws clauses for [Export] methods; generate Java field declarations for [ExportField]
  • UCO wrappers + RegisterNatives: Export methods get the same UCO wrapper + JNI native registration as [Register] native callbacks
  • ExportMethodDispatchEmitter: New emitter class handling the PE metadata generation for export dispatch, separate from the UCO constructor emitter
  • Exclude Mono.Android.Export: Exclude the Mono.Android.Export assembly from trimmable packages — its DynamicMethod-based codegen is incompatible with AOT/trimming; uses [ExportAttribute]/[ExportFieldAttribute] from the user assembly's [Register]-scanned types instead

Bug fixes

  • Fix missing static keyword in Java codegen for static [Export] methods
  • Fix stack corruption in TryEmitExportParameterArgument (wrong local variable index)
  • Fix instrumentation targetPackage defaulting to use the passed-in package name parameter
  • Propagate deferred registerNatives to base classes so inherited exports are registered correctly

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the TrimmableTypeMap pipeline to support legacy [Export] / [ExportField] callbacks (including UCO wrapper + RegisterNatives generation and richer signature/metadata scanning), and adjusts test/build plumbing to stabilize the CoreCLRTrimmable test lane (including excluding Mono.Android.Export from app packaging on the trimmable path).

Changes:

  • Add scanner + model support for [Export] / [ExportField], including signature resolution for Java-bound types and [ExportParameter] legacy marshalling shapes.
  • Add generator support for direct-dispatch UCO wrappers for [Export] (new ExportMethodDispatchEmitter) and align typemap/manifest generation behavior.
  • Stabilize CI/test lanes: introduce CoreCLRTrimmable flavor + categories, defer registerNatives up base class chains, and ensure Mono.Android.Export isn’t packaged on the trimmable path.

Reviewed changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs Detect/export [Export] metadata, compute JNI signatures with legacy marshalling shapes, and record precise managed type/assembly info.
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ExportMethodDispatchEmitter.cs New emitter that generates UCO wrappers which dispatch directly to managed [Export] targets (avoids legacy dynamic callback generation).
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs Integrate export dispatch emitter, refactor RegisterNatives emission, and adjust proxy/UCO emission flow.
src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs Manifest rooting rewrite + propagation of deferred registration flags to base classes; pass prepared manifest through generation.
src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets Mark Mono.Android.Export references with AndroidSkipAddToPackage=true for trimmable typemap builds.
src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeApplicationConfigSources.cs Skip assemblies marked AndroidSkipAddToPackage when generating native app config sources.
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/* New/updated fixtures and assertions covering export scanning, export dispatch generation, manifest rewriting, and instrumentation defaults.
tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj Add CoreCLRTrimmable configuration defaults (runtime selection, categories, constants).
build-tools/automation/yaml-templates/stage-package-tests.yaml Add CoreCLRTrimmable instrumentation lane and adjust CoreCLR lane args.

Comment on lines +210 to +217
_baseCtorRef = _pe.AddMemberRef (_javaPeerProxyRef, ".ctor",
sig => sig.MethodSignature (isInstanceMethod: true).Parameters (2,
rt => rt.Void (),
p => {
p.AddParameter ().Type ().Type (_systemTypeRef, false);
p.AddParameter ().Type ().Type (_systemTypeRef, false);
}));

Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_baseCtorRef is assigned but never used, and the encoded .ctor signature here doesn’t match JavaPeerProxy<T> (it encodes two System.Type parameters instead of string + Type). This should be removed to avoid dead/incorrect metadata and to prevent future accidental use of a wrong member reference.

Suggested change
_baseCtorRef = _pe.AddMemberRef (_javaPeerProxyRef, ".ctor",
sig => sig.MethodSignature (isInstanceMethod: true).Parameters (2,
rt => rt.Void (),
p => {
p.AddParameter ().Type ().Type (_systemTypeRef, false);
p.AddParameter ().Type ().Type (_systemTypeRef, false);
}));

Copilot uses AI. Check for mistakes.
@simonrozsival simonrozsival marked this pull request as draft April 16, 2026 15:07
@simonrozsival simonrozsival added the copilot `copilot-cli` or other AIs were used to author this label Apr 16, 2026
@simonrozsival simonrozsival force-pushed the dev/simonrozsival/trimmable-typemap-export-attribute branch 2 times, most recently from 2b68085 to 9007196 Compare April 18, 2026 20:29
Copy link
Copy Markdown
Member Author

@simonrozsival simonrozsival left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Code Review — PR #11123

Verdict: ⚠️ Needs Changes (1 warning, 2 suggestions; CI still pending)

Summary

Solid implementation of [Export]/[ExportField] for the trimmable typemap pipeline. The static UCO dispatch via [UnmanagedCallersOnly] + RegisterNatives is correct, the IL generation handles all primitive/object/array/stream/XML marshalling shapes, and the test coverage is thorough (162 new test assertions across scanner, model builder, assembly generator, JCW codegen, and build integration). The Mono.Android.Export.dll exclusion from the packaged APK is properly wired.

Positive callouts

  • ExportMethodDispatchEmitterContext factory — single-allocation, reused for the entire emit pass. Clean separation from the parent emitter.
  • ExportParameterKind support — properly resolves InputStream, OutputStream, XmlPullParser, XmlResourceParser marshalling in both directions (JNI→managed and managed→JNI return).
  • Array copy-back with null guards — the Brfalse_s skip pattern correctly avoids null-array copy-back crashes.
  • Cross-assembly type resolutionTypeRefSignatureTypeProvider + MetadataTypeNameResolver properly chase ResolutionScope to the correct assembly reference, and the test Generate_ExportProxy_UsesExactCrossAssemblyTypeReferences validates it end-to-end.

CI (Xamarin.Android-PR) is still pending — review is based on code analysis only.

Comment thread src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerInfo.cs
@simonrozsival simonrozsival marked this pull request as ready for review April 22, 2026 16:13
@simonrozsival
Copy link
Copy Markdown
Member Author

Blocked by #11091

simonrozsival and others added 18 commits April 26, 2026 14:24
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix RootManifestReferencedTypes to resolve relative android:name
  values (.MyActivity, MyActivity) using manifest package attribute
- Keep $ separator in peer lookup keys so nested types (Outer$Inner)
  match correctly against manifest class names
- Guard Path.GetDirectoryName against null return for acw-map path
- Fix pre-existing compilation error: load XDocument from template
  path before passing to ManifestGenerator.Generate
- Add tests for relative name resolution and nested type matching

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival and others added 14 commits April 26, 2026 15:24
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Propagate CannotRegisterInStaticConstructor through the base class chain
  so that base types like TestInstrumentation_1 also use the deferred
  __md_registerNatives() pattern instead of static { registerNatives(...); }
  which crashes before the managed runtime registers the JNI native.

- Revert C++ host-jni.cc/hh registerNatives bridge — the managed
  [UnmanagedCallersOnly] registration in TrimmableTypeMap.RegisterNatives()
  handles this without needing a C++ bridge.

- Add targetPackage default for instrumentation in ComponentElementBuilder.

- Switch proxy base type to generic JavaPeerProxy<T> in TypeMapAssemblyEmitter.

- Add CannotRegisterInStaticConstructor to JavaPeerProxyData model.

- Normalize manifest android:name to actual JNI names.

- Add test exclusions for TrimmableIgnore and SSL categories.

- Add TRIMMABLE_TYPEMAP define constant for conditional compilation.

- Add unit tests for base class propagation and manifest normalization.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… refactoring

Revert files that are not about [Export] support:
- CI lane (stage-package-tests.yaml)
- Test exclusions/categories (TrimmableIgnore, DoNotGenerateAcw)
- NUnitInstrumentation test plumbing
- Mono.Android.NET-Tests.csproj trimmable setup
- TrimmableTypeMapGenerator manifest refactoring (from #11105)
- TrimmableTypeMapGeneratorTests manifest/propagation tests

Keep only Export-related changes:
- CoreCLRIgnore removal from Export tests in JnienvTest.cs

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Revert cast spacing changes in AssemblyIndex.cs, MetadataTypeNameResolver.cs
- Revert indentation changes in JavaPeerScanner.cs, GenerateNativeApplicationConfigSources.cs
- Revert whitespace in PackagingTest.cs, GeneratePackageManagerJavaTests.cs, TypeMapAssemblyGeneratorTests.cs
- Move EmitRegisterNatives back after EmitUcoConstructor to match main's method ordering

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
LoadArgument + LoadConstantI4(0) were emitted unconditionally before the
switch statement. When exportKind is Unspecified (the default for
parameters without [ExportParameter] attributes), the method returned
false without consuming those two stack values, corrupting the IL
evaluation stack.

Move the LoadArgument + LoadConstantI4(0) into each case block so they
are only emitted when the method will also emit the consuming Call.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The _AndroidTypeMapImplementation=trimmable forcing and Assert.Ignore
removal for NativeAOT belong in the separate CI setup PR, not here.
This PR should only add the Export code generation support without
modifying CI configuration or device test behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The JcwJavaSourceGenerator was not emitting the 'static' keyword for
static [Export] methods, which would cause a runtime crash. Add the
keyword to both the wrapper method and the native declaration when
method.IsStatic is true. Add a regression test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Guard TypeRefSignatureTypeProvider.DecodeSignature behind isExport
  to avoid unnecessary allocation for every [Register] method
- Extract ExportParameterKindInfo enum to its own file
- Extract ExportMethodDispatchEmitterContext to its own file

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The attribute was changed from TypeMapAssociationAttribute`1 (generic) to
TypeMapAssociationAttribute (non-generic), but the test assertion wasn't updated.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…s and fix stale comment

These properties were superseded by the TypeRefData-based ManagedParameterTypes
and ManagedReturnType properties and were no longer read anywhere.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ttribute`1)

The attribute emitter uses the generic TypeRef name 'TypeMapAssociationAttribute`1',
so the assertion must check for the generic name.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival simonrozsival force-pushed the dev/simonrozsival/trimmable-typemap-export-attribute branch from cc98948 to 68aafbe Compare April 26, 2026 13:53
@simonrozsival simonrozsival changed the base branch from main to dev/simonrozsival/trimmable-test-plumbing April 26, 2026 13:55
simonrozsival and others added 7 commits April 26, 2026 17:02
Restore the full ExcludedTestNames list from the base branch
(dev/simonrozsival/trimmable-test-plumbing) that was lost during
the rebase, and add two new entries for failures introduced by the
trimmable Export support:

- JnienvTest.ActivatedDirectObjectSubclassesShouldBeRegistered: direct
  Object subclass registration is not supported in the trimmable typemap.
- JavaObjectTest.Dispose_Finalized: finalization behavior differs
  under the trimmable typemap.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…pers

Mirror TypeManager.Activate so [Export]-using classes' instance
initialization runs when the peer is created from the Java side.

For the parameterless `()V` UCO constructor wrapper, the emitter
previously called the inherited activation ctor
`(IntPtr, JniHandleOwnership)` after `GetUninitializedObject`. This
matched what was needed to set up the peer reference but skipped the
user-visible ctor body — so any field initialization there (e.g.
`Constructed = true` in `ContainsExportedMethods`) never ran when
the peer was created from Java.

The new IL pattern matches `TypeManager.Activate`:

    var obj = (T) RuntimeHelpers.GetUninitializedObject(typeof(T));
    ((IJavaPeerable) obj).SetPeerReference(new JniObjectReference(self, Invalid));
    obj..ctor();   // user-visible parameterless ctor

The user-visible ctor's chain into Java.Lang.Object()/IJavaPeerable is
a no-op when the peer reference is already set, so this does not create
a second Java peer.

Parameterized Java ctors (`(Lfoo;)V` etc.) still take the legacy
activation-ctor path — JNI args are not forwarded. Forwarding args to a
matching user-visible ctor is left as a TODO follow-up.

Removes the `Java.InteropTests.JnienvTest.ActivatedDirectObjectSubclassesShouldBeRegistered`
test from the trimmable exclusion list.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The earlier 'Remove unrelated changes' commit was authored before the
test-plumbing PR (#11091) was merged into the base branch. After
rebasing, that commit ended up reverting changes that now legitimately
belong to the base branch, deflating the diff in the wrong direction.

Restore from origin/dev/simonrozsival/trimmable-test-plumbing:

- build-tools/automation/yaml-templates/stage-package-tests.yaml:
  re-add the Mono.Android.NET_Tests-CoreCLRTrimmable instrumentation
  stage (10 lines) that was inadvertently removed.

- external/Java.Interop: reset submodule pointer to 26c7948a6 (the
  version on main / base), undoing an unintentional submodule update.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Revert pure cosmetic/tangential changes from the rebase:

* TypeMapAssemblyEmitter.cs: restore EmitRegisterNatives to its original
  location with its descriptive comments intact (it had been moved
  earlier in the file and stripped of comments). Also revert a few
  unrelated whitespace/brace-style changes (extra blank line before a
  closing brace, gratuitous `for { ... }` brace insertions, indentation
  damage in the generic-proxy ctor signature lambda).
* TrimmableTypeMapGenerator.cs: revert a cosmetic brace-style change on
  PropagateDeferredRegistrationToBaseClasses; the surrounding
  refactoring stays because it's required by the new manifest-rewriting
  feature on this PR.
* GenerateNativeApplicationConfigSources.cs: revert `using` reordering
  and `ITaskItem[]?` -> `ITaskItem []?` whitespace; only the new
  ShouldSkipAssembly helper + its two call sites are kept.
* Mono.Android.NET-Tests.csproj: drop the unused
  `;TRIMMABLE_TYPEMAP` define constant. Nothing in the codebase
  references it; the runtime feature switch covers the build-time
  selection instead.

All 453 generator unit tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The trimmable typemap path uses managed JniEnvironment.Types.RegisterNatives
directly from the generated proxy types' RegisterNatives method, so the
native JNI shim added during the rebase was not needed. Runtime registration
of natives is already solved via the managed code path on this branch.

Reverts the native bits to match base verbatim:
* src/native/clr/host/host-jni.cc
* src/native/clr/include/host/host-jni.hh
* src/native/clr/libnet-android.map.txt

Verified end-to-end: full Release build + on-device test run with
_AndroidTypeMapImplementation=trimmable, UseMonoRuntime=false:
917 tests / 0 errors / 0 failures / 57 ignored.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…riginal positions

These methods were unintentionally moved earlier in the file during the
rebase, which made the diff against base look like a big delete + big add
of identical content (no clear signal of actual changes).

Restore them to their pre-PR positions (after EncodeUcoConstructorLocals_JavaInterop)
so the diff against base shows only genuine additions: the new
[Export] dispatch wiring, member refs, comments, and the parameterless
UCO ctor branch.

No behavior change. 453/453 unit tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…fety guard

The previous comment leaked a test-fixture detail (`Constructed = true`) into
the implementation explanation. Rephrase to describe the general invariant
("user-visible ctor body runs when the peer is created from the Java side")
and explicitly point at the safety guard that makes the approach safe:
`if (PeerReference.IsValid) return;` in Java.Lang.Object's chain, which
prevents the user-visible ctor's :base() from creating a second Java peer.

No code change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

copilot `copilot-cli` or other AIs were used to author this trimmable-type-map

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants