[TrimmableTypeMap] Add trimmable [Export] and [ExportField] callback support#11123
[TrimmableTypeMap] Add trimmable [Export] and [ExportField] callback support#11123simonrozsival wants to merge 34 commits intomainfrom
Conversation
There was a problem hiding this comment.
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](newExportMethodDispatchEmitter) and align typemap/manifest generation behavior. - Stabilize CI/test lanes: introduce CoreCLRTrimmable flavor + categories, defer registerNatives up base class chains, and ensure
Mono.Android.Exportisn’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. |
| _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); | ||
| })); | ||
|
|
There was a problem hiding this comment.
_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.
| _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); | |
| })); |
7c81e12 to
d429c27
Compare
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>
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>
2b68085 to
9007196
Compare
…ribute 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>
simonrozsival
left a comment
There was a problem hiding this comment.
🤖 Code Review — PR #11123
Verdict:
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,XmlResourceParsermarshalling in both directions (JNI→managed and managed→JNI return). - Array copy-back with null guards — the
Brfalse_sskip pattern correctly avoids null-array copy-back crashes. - Cross-assembly type resolution —
TypeRefSignatureTypeProvider+MetadataTypeNameResolverproperly chaseResolutionScopeto the correct assembly reference, and the testGenerate_ExportProxy_UsesExactCrossAssemblyTypeReferencesvalidates it end-to-end.
CI (Xamarin.Android-PR) is still pending — review is based on code analysis only.
- 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>
…mmable-typemap-export-attribute # Conflicts: # external/Java.Interop
…mmable-typemap-export-attribute 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>
Summary
Add UCO (UnmanagedCallersOnly) wrapper codegen for
[Export]methods and[ExportField]fields in the trimmable typemap pipeline, plus test pruning and stabilization for the CoreCLRTrimmable test lane.Part of #10788
Changes
Export support
[Export]and[ExportField]attributes, resolve JNI signatures for non-primitive Java-bound parameter types, collect Java access modifiers and throws clausesMarshalMethodInfo.IsExport/JavaAccess/ThrownNames/SuperArgumentsStringcarry all export metadatanativemethods with correct access modifiers and throws clauses for[Export]methods; generate Java field declarations for[ExportField][Register]methodsMono.Android.Exportfrom trimmable packages (its dynamicDynamicMethodcodegen is incompatible with AOT/trimming)Test stabilization
registerNativesto base classes