[TrimmableTypeMap] Generator correctness fixes#11988
Open
simonrozsival wants to merge 5 commits into
Open
Conversation
Java.Interop.ManagedPeer is a reflection-based helper marked
[RequiresUnreferencedCode] ("Uses reflection to find constructors and
invoke them."). The trimmable type map generator emitted a proxy for it
(_TypeMap.Proxies.Java_Interop_ManagedPeer_Proxy) whose constructor
references ManagedPeer's constructors, producing two IL2026 trim
warnings, aggregated by ILC into:
IL2104: Assembly '_Java.Interop.TypeMap' produced trim warnings
Because the default NativeAOT type map is now trimmable, this surfaced as
a real MSBuild warning and broke NativeAOT tests that assert no warnings
(e.g. BuildWithJavaToolOptions).
ManagedPeer is not supported by the trimmable type map: on the trimmable
path native registration goes through IAndroidCallableWrapper.RegisterNatives
and ManagedPeerNativeRegistration is disabled, so ManagedPeer is never
activated via the type map. Exclude it in the scanner so no proxy is
emitted.
Verified on an arm64 emulator: the HelloWorld NativeAOT (trimmable)
sample now builds with 0 warnings and still launches to MainActivity.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 8d424c0)
The trimmable manifest generator already had ComponentElementBuilder support for a <layout> child element, but the scanner never populated ComponentInfo.LayoutProperties, so [Layout(...)] on an activity produced no <layout> element (LayoutAttributeElement failed on NativeAOT). Parse the [Layout] attribute's named properties in AssemblyIndex.ParseAttributes (collected separately to tolerate attribute ordering, like [IntentFilter] and [MetaData]) and flow them through TypeAttributeInfo.LayoutProperties into ComponentInfo.LayoutProperties. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 968ac2c)
…ers) ManifestGenerator.ApplyPlaceholders already invoked a WarnInvalidPlaceholder callback for placeholder entries without '=', but the callback was never wired up, so the trimmable path silently dropped the XA1010 warning the legacy ManifestDocument emits (ManifestPlaceHoldersXA1010 failed on NativeAOT). Add ITrimmableTypeMapLogger.LogInvalidManifestPlaceholderWarning (logging XA1010 from the MSBuild logger) and wire the ManifestGenerator instance's WarnInvalidPlaceholder to it. The rooting-only PrepareManifestForRooting pass stays silent so the warning is not emitted twice. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 518d9b4)
PEAssemblyBuilder.WritePE let ManagedPEBuilder fall back to a time-based PE content id, so every regeneration of a typemap assembly produced different bytes (different PE TimeDateStamp) even for identical input — the MVID was already deterministic, but the image was not. That churn broke incremental packaging on NativeAOT: an SDK CoreCompile rerun (e.g. touching a .csproj.user file, which the SDK treats as a compile input) rewrites the app assembly, reruns _GenerateTrimmableTypeMap, and — because the regenerated *.TypeMap.dll got a fresh timestamp — forced _BuildApkEmbed to repackage and _Sign to re-sign (CSProjUserFileChanges failed on NativeAOT). Supply a deterministic content-id provider (SHA-256 over the serialized image) so identical input yields byte-identical output; CopyIfStreamChanged then keeps the existing file/timestamp and downstream targets stay incremental. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 91204df)
For a JNI name mapped by multiple managed types (an alias group), the trimmable type map's GetTypeForSimpleReference returns the first (index [0]) alias. Order the aliases to match the native runtime's java->managed selection (clr_typemap_java_to_managed / monovm_typemap_java_to_managed, built by NativeTypeMappingData): the Mono.Android module is processed first and the first managed type to claim a Java name wins. So java/lang/Object must resolve to Java.Lang.Object (Mono.Android), not Java.Interop.JavaObject (Java.Interop). Order alias peers Mono.Android-assembly-first, with an ordinal managed-name tiebreak for deterministic proxy naming. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR applies a set of correctness and determinism fixes to the trimmable type map generator (-p:_AndroidTypeMapImplementation=trimmable), aligning generated output and resolution behavior with the native runtime and improving incremental/reproducible build behavior.
Changes:
- Adjust alias ordering so
Mono.Androidpeers sort first (matching the runtime’s first-writer-wins java→managed selection), and add a targeted regression test. - Extend scanning/model data to support emitting a
<layout>manifest child element from[Layout]attributes. - Add an
XA1010warning path for invalid$(AndroidManifestPlaceholders)and make generated typemap assemblies byte-deterministic via a deterministic PE content id.
Show a summary per file
| File | Description |
|---|---|
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapModelBuilderTests.cs | Adds regression test ensuring Mono.Android alias sorting wins for java/lang/Object. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TrimmableTypeMapGeneratorTests.cs | Extends test logger to capture invalid placeholder warnings. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs | Implements XA1010 coded warning logging for invalid manifest placeholders. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs | Wires invalid-placeholder warning callback into manifest generation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs | Excludes Java.Interop.ManagedPeer from emission; propagates layout properties into component info. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/AssemblyIndex.cs | Parses [Layout] attribute named properties and attaches them to component attribute info. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/ITrimmableTypeMapLogger.cs | Adds logger API for invalid placeholder warning. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/PEAssemblyBuilder.cs | Adds deterministic PE content id provider for byte-stable generated assemblies. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ModelBuilder.cs | Implements Mono.Android-first alias sorting with deterministic tiebreaker. |
Copilot's findings
- Files reviewed: 9/9 changed files
- Comments generated: 1
Comment on lines
+125
to
+129
| using var hash = IncrementalHash.CreateHash (HashAlgorithmName.SHA256); | ||
| foreach (var blob in content) { | ||
| var segment = blob.GetBytes (); | ||
| hash.AppendData (segment.Array!, segment.Offset, segment.Count); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Independent correctness fixes for the trimmable type map generator (the opt-in
-p:_AndroidTypeMapImplementation=trimmablepath). These stand alone and do not depend on making the trimmable type map the NativeAOT default (#11822) — extracting them keeps that PR focused and lets these be reviewed on their own.Fixes
Java.Interop.ManagedPeerfrom the type map — it is a base infrastructure peer, not an app/binding type, and must not participate in java→managed resolution.<layout>element for the[Layout]attribute — the scanner now surfaces[Layout]so the generated manifest/JCW is correct.$(AndroidManifestPlaceholders)— proper diagnostic instead of a silent/obscure failure.java/lang/ObjecttoJava.Lang.Object, notJava.Interop.JavaObject— order alias groups Mono.Android-assembly-first to match the native runtime's java→managed selection (clr_typemap_java_to_managed/monovm_typemap_java_to_managed, built byNativeTypeMappingData: Mono.Android module first, first-writer-wins), with an ordinal managed-name tiebreak for deterministic proxy naming.Testing
Microsoft.Android.Sdk.TrimmableTypeMap.Tests: 606/606 pass (includes a new alias-ordering test).Extracted from #11822.