[TrimmableTypeMap] Add AOT-safe array and collection factories using MakeArrayType and MakeGenericType

[simonrozsival]

Summary

This PR adds runtime-only safe factory helpers for the trimmable type map path:

• SafeArrayFactory centralizes managed array Type lookup and array instance creation.
• SafeJavaCollectionFactory centralizes generic Java collection wrapper Type lookup and wrapper creation for JavaList<T>, JavaCollection<T>, and JavaDictionary<TKey,TValue>.
• ValueTypeFactory<T> is the shared primitive/nullable value-type rooting table used by both factories.
• JNIEnv, JavaConvert, and TrimmableTypeMapTypeManager now call these helpers instead of carrying their own trimmable-specific construction policy inline.

This also adds boxed nullable primitive array support (int?[], etc.) and runtime coverage for nullable array/list/dictionary marshaling.

Why using MakeArrayType() / MakeGenericType() is OK here

These APIs are annotated with RequiresDynamicCode / RequiresUnreferencedCode because the general case is unsafe in NativeAOT/trimming:

• arbitrary array element types can lack an array EEType/template;
• arbitrary constructed generic instantiations can lack a runtime template or constructor metadata;
• value-type generic instantiations are not canonically shared the same way reference-type instantiations are.

This PR does not suppress those warnings broadly. The suppressions are isolated in the safe factories and only after constraining the shapes to cases we know NativeAOT can support.

The runtime source path this is based on is:

• RuntimeType.NativeAot.cs -> RuntimeTypeInfo.MakeArrayType() / MakeGenericType()
• ExecutionEnvironmentImplementation.MappingTables.cs
• TypeLoaderEnvironment.cs
• TypeBuilder.TryBuildArrayType() / TypeBuilder.TryBuildGenericType()
• TemplateLocator.cs
• StandardCanonicalizationAlgorithm.cs

The key NativeAOT behavior is:

• Reference types canonicalize to __Canon, and reference arrays/generic instantiations can use canonical templates.
• Value types remain value-specific, so exact value-type arrays and exact value-type generic instantiations must be rooted.
• Array.CreateInstanceFromArrayType() immediately needs the array TypeHandle and allocates through RuntimeAugments.NewArray(), so it must only see array types that are actually runtime-buildable/rooted.

The factories enforce that model:

• Reference arrays use elementType.MakeArrayType() and then Array.CreateInstanceFromArrayType() only for NativeAOT-buildable reference-array canonical shapes.
• First-rank primitive/nullable value arrays are allocated directly through ValueTypeFactory<T>.CreateArray() (new T[length]), which roots the exact T[] vector.
• Additional array ranks are jagged SZArrays. Once the first vector exists, each outer wrapper has an array reference type as its element, so it follows the reference-array canonical path.
• Reference Java collection wrappers use typeof(JavaList<>).MakeGenericType(...), etc., relying on canonical generic templates such as JavaList<__Canon>.
• Primitive/nullable value Java collection wrappers use ValueTypeFactory<T> direct generic references and constructors, rooting exact instantiations such as JavaList<int> and JavaList<int?>.
• Mixed reference/value dictionaries root the canonical shapes with dedicated type tokens (JavaDictionary<object,T> and JavaDictionary<T,object>).

Unsupported value-type cases are rejected instead of falling through to unrooted MakeGenericType() / MakeArrayType() usage.

Scope

Included:

• JavaList<T> / IList<T>
• JavaCollection<T> / ICollection<T>
• JavaDictionary<TKey,TValue> / IDictionary<TKey,TValue>
• Java boxed primitive set and nullable forms: bool, sbyte, char, short, int, long, float, double, plus nullable variants
• reference-type array and wrapper shapes

Out of scope for this PR:

• JavaSet<T>
• byte and unsigned aliases
• removing generated array proxy code
• removing JavaPeerContainerFactory

Follow-up

This PR intentionally leaves generated array proxy code, JavaPeerContainerFactory, and related obsolete support code in place. A follow-up PR will remove that generator/runtime cleanup once this runtime-only factory path is validated.

Validation

• dotnet build src/Mono.Android/Mono.Android.csproj -v:minimal /p:Configuration=Debug --no-restore /p:JavaCPath=/Library/Java/JavaVirtualMachines/microsoft-21.jdk/Contents/Home/bin/javac /p:JarPath=/Library/Java/JavaVirtualMachines/microsoft-21.jdk/Contents/Home/bin/jar /p:JavaPath=/Library/Java/JavaVirtualMachines/microsoft-21.jdk/Contents/Home/bin/java
• dotnet test tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj -v:minimal --no-restore

[simonrozsival]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

[Copilot]

Pull request overview

This PR refactors the trimmable typemap marshaling path to centralize AOT-safe construction of array and generic Java collection wrapper types, and extends runtime/test coverage to support nullable primitive arrays and nullable list/dictionary marshaling.

Changes:

• Introduces SafeArrayFactory, SafeJavaCollectionFactory, and ValueTypeFactory<T> to constrain and root AOT-safe array/generic instantiations.
• Updates JNIEnv, JavaConvert, and TrimmableTypeMapTypeManager to use the new factories and adds nullable primitive array support.
• Adds new tests and updates APK descriptor baselines to reflect output changes.

Show a summary per file

File	Description
tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaConvertTest.cs	Adds runtime tests for nullable IList<T> and IDictionary<TKey,TValue> conversions via JavaConvert.FromJniHandle.
tests/Mono.Android-Tests/Mono.Android-Tests/Android.Runtime/JnienvArrayMarshaling.cs	Adds test coverage for JNIEnv nullable primitive array round-tripping and item set/get.
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.NativeAOT.apkdesc	Updates expected APK file/package sizes after runtime/type-map changes.
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc	Updates expected APK file/package sizes after runtime/type-map changes.
src/Mono.Android/Mono.Android.csproj	Adds the new factory helper source files to the build.
src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMapTypeManager.cs	Extends NativeAOT array-type discovery to use SafeArrayFactory when a generated proxy isn’t present.
src/Mono.Android/Java.Interop/ValueTypeFactory.cs	Adds the shared primitive/nullable rooting table for AOT-safe arrays and collection wrappers.
src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs	Adds constrained AOT-safe generic wrapper type creation and handle-to-wrapper creation for lists/collections/dictionaries.
src/Mono.Android/Java.Interop/SafeArrayFactory.cs	Adds constrained AOT-safe array Type lookup and instance creation (including jagged arrays).
src/Mono.Android/Java.Interop/JavaConvert.cs	Switches trimmable-path generic collection conversion to the new factory-based approach.
src/Mono.Android/Android.Runtime/JNIEnv.cs	Routes trimmable array allocation through SafeArrayFactory and adds nullable primitive array marshaling support.

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 4

[github-actions]

🤖 Android PR Review — ⚠️ Needs Changes (minor)

Runtime-only AOT-safe factories for managed arrays and Java collections on the trimmable-type-map path. The core design is well-reasoned: value types are rooted explicitly (typeof(T[]) / new T[] / direct generic constructor references), reference types ride NativeAOT's __Canon canonical templates, and unsupported value-type shapes are rejected (return null) rather than silently reaching an unrooted MakeGenericType/MakeArrayType. The [UnconditionalSuppressMessage] attributes are narrowly scoped with sound justifications after the shapes have been constrained. 👍

I verified the removed CoreCLR Array.CreateInstance catch-all is not a regression: ArrayCreateInstance is only ever reached with nullable primitives (via CreateManagedArrayFromObjectArray) or reference types (IJavaObject/Array converters). Enums are mapped to their underlying primitive by GetConverter, and GetArray<T> uses new T[cnt] directly — so no non-primitive value type flows into SafeArrayFactory.CreateInstance.

Findings (all inline)

Sev	Location	Issue
⚠️	SafeJavaCollectionFactory.cs:139	Value/value IDictionary goes through the virtual-generic-method path (CreateDictionaryWithKey<TKey> → new JavaDictionary<TKey,T>) — the AOT-riskiest construct here — and has no test. Add an IDictionary<int,long> runtime test under the NativeAOT trimmable config.
💡	SafeJavaCollectionFactory.cs:77	TryCreateInstance(Type, object?, out ...) (construct-from-items) has zero callers.
💡	SafeArrayFactory.cs:19	Throwing GetArrayType(Type,int) overload is unused.
💡	JNIEnv.cs:954	Per-element closure/delegate allocation in CopyManagedObjectArray.

Positives

• Boxed nullable-primitive array / list / dictionary support with round-trip tests (GetArray_NullableInt32, FromJniHandle_IListNullableInt32, FromJniHandle_IDictionaryNullableInt32String).
• Clear inline rationale for why the trimmable path avoids the reflection fallback and why each rooting trick exists.
• Correct JNI local/global ref lifetime handling in the new NewObjectArray helpers.

CI

All completed dotnet-android legs on the previous commit's build were green; the build for the latest commit (b959f4a) was still in progress at review time. Not itself a blocker — but the value/value dictionary test above is worth landing before merge, since that's exactly the instantiation most likely to surface only under NativeAOT.

Generated by Android PR Reviewer for #12030 · 358.6 AIC · ⌖ 28.2 AIC · ⊞ 6.8K
_{Comment /review to run again}

[simonrozsival]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

[github-actions]

🤖 Code Review — ⚠️ Minor suggestions

Reviewed the AOT-safe array/collection factory refactor. Overall this is a well-constructed, carefully-reasoned change — the suppression justifications are precise, the value-type-vs-__Canon rooting model is explained clearly, and the new factories cleanly consolidate construction policy that was previously scattered across JNIEnv, JavaConvert, and TrimmableTypeMapTypeManager. Nullable-primitive array/collection marshaling has good round-trip test coverage (GetArray_NullableInt32, FromJniHandle_IListNullableInt32, FromJniHandle_IDictionaryInt32Int64).

Findings

• ⚠️ Performance (JavaConvert.cs:79) — collection type is resolved by TryGetCollectionType for a boolean gate, then re-resolved inside TryCreateFromJniHandle, causing MakeGenericType() to run twice per uncached FromJniHandle call for reference generic collections. Inline comment posted.
• 💡 Formatting (JNIEnv.cs:1106) — use nameof (elementType) instead of the string literal. Inline comment posted.

CI

The GitHub combined status shows pending (Azure DevOps dnceng-public validation not yet complete / no required checks reported). Neither finding is merge-blocking, but confirm the pipeline goes green before merge. The primitive/nullable rooting story is only exercised on the reflection-based runtimes by the added tests; NativeAOT rooting for exact value/value instantiations (JavaDictionary<int,long>) relies on generic-virtual rooting and isn't directly asserted under PublishAot in CI — worth keeping in mind for the validation follow-up.

Nice work on the documentation of the AOT reasoning.

Generated by Android PR Reviewer for #12030 · 144.5 AIC · ⌖ 18.8 AIC · ⊞ 6.8K
_{Comment /review to run again}

[jonathanpeppers]

Merge conflicts

[simonrozsival]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

[github-actions]

🤖 Code Review — ⚠️ A couple of questions

Thorough, well-documented refactor. Consolidating the trimmable-typemap construction policy into SafeArrayFactory / SafeJavaCollectionFactory / ValueTypeFactory (with the AOT reasoning captured in comments and UnconditionalSuppressMessage justifications) is a clear improvement over the inline suppressions previously scattered across JNIEnv/JavaConvert. Nice touches: boxed-nullable primitive array support, the BuildRuntimeArrayTypes fallback that mirrors the generator's proxy contract, and the regression test (BuildRuntimeArrayTypes_ReferenceLeaf_ReturnsProxyContract) that guards the previously-uncaught "returns only [T[]]" bug fixed in the latest commit.

Findings

• ⚠️ Behavioral regression for byte collections on the trimmable path — the old ScalarContainerFactories included typeof (byte); the new factories omit it, so IList<byte> etc. now silently resolve to null rather than marshaling or throwing. Flagged inline on JavaConvert.cs. Documented as out-of-scope, but worth an explicit error instead of a silent null.
• ⚠️ NativeAOT value/value dictionary rooting — ValueTypeFactory<T>.CreateDictionaryWithKey<TKey> builds value/value JavaDictionary<TKey,T> via generic-virtual dispatch with no dedicated rooting token; the new test only exercises the Mono/CoreCLR path. Flagged inline on ValueTypeFactory.cs — please confirm NativeAOT coverage.

Other observations (non-blocking)

• The _ = ReferenceKeyDictionaryType; / ReferenceValueDictionaryType discard-to-root pattern works but is subtle; the existing comments help.
• SafeArrayFactory.TryCreateInstance handles the rank-1 primitive case both via the explicit short-circuit and again inside TryGetVectorType — minor duplication, not a bug.

CI

Checks are currently pending (no failures reported); the ready-to-review label notes any failures are expected flakes. Not treating CI as blocking, but it should be green before merge.

Great work overall — the two ⚠️ items are questions to resolve rather than definite defects.

Generated by Android PR Reviewer for #12030 · 121.6 AIC · ⌖ 13.4 AIC · ⊞ 6.8K
_{Comment /review to run again}

[github-actions]

🤖 ⚠️ Patterns — This narrows the trimmable-typemap collection support relative to the old ScalarContainerFactories/TryGetFactoryBasedConverter, which included typeof (byte). SafeJavaCollectionFactory / ValueTypeFactory.PrimitiveTypeFactories deliberately omit byte (and unsigned aliases), so IList<byte>, ICollection<byte>, and IDictionary<byte,...> now return false here and fall through to null (the non-generic IList/IDictionary checks below don't match generic interfaces). On the trimmable path this is a silent behavioral regression for byte collections that previously marshaled. The PR marks byte as out of scope, but the old path supported it — is dropping it at runtime (vs. throwing a clear NotSupportedException) intended? At minimum consider a targeted error so callers don't get a silent null.

{Rule: Preserve existing behavior / avoid silent regressions}

[github-actions]

🤖 ⚠️ Trimmer/AOT — CreateDictionaryWithKey<TKey> is a generic virtual method that constructs new JavaDictionary<TKey,T>() for value TKey/T (the value/value dictionary path). Generic-virtual dispatch instantiated over value types is one of the most fragile constructs under NativeAOT: each exact JavaDictionary<int,long>, JavaDictionary<int?,double>, ... instantiation (up to ~16×16) must be statically rooted, and unlike the reference/mixed cases there's no dedicated rooting token here — the instantiation is only reachable through this virtual call. The added FromJniHandle_IDictionaryInt32Int64 test exercises this on Mono/CoreCLR, where dynamic code masks the problem; it won't catch a missing NativeAOT template. Can you confirm this is covered by an actual NativeAOT run (e.g. the .NativeAOT.apkdesc scenario), or add explicit rooting tokens for these value/value shapes like the mixed-dictionary case does?

{Rule: NativeAOT template rooting for value-type generic instantiations}