[TrimmableTypeMap] Remove InvokerType from JavaPeerProxy#11970
Closed
simonrozsival wants to merge 3 commits into
Closed
[TrimmableTypeMap] Remove InvokerType from JavaPeerProxy#11970simonrozsival wants to merge 3 commits into
simonrozsival wants to merge 3 commits into
Conversation
The trimmable typemap activates managed peers exclusively through the generated `JavaPeerProxy.CreateInstance`, so the `InvokerType` exposed on the proxy attribute is never consumed at runtime: `JavaMarshalValueManager.CreatePeer` calls `TrimmableTypeMap.CreateInstance` and never falls back to invoker-type reflection. Remove the `invokerType` constructor parameter and the `InvokerType` property from both `JavaPeerProxy` and `JavaPeerProxy<T>`, drop the now-dead `TrimmableTypeMap.GetInvokerType`, and make `TrimmableTypeMapTypeManager.GetInvokerTypeCore` throw `NotSupportedException` since it should never be reached in this path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Now that `JavaPeerProxy` no longer exposes an `InvokerType`, the generated proxy `.ctor` no longer passes an invoker `Type` (or `ldnull`) to the base constructor: the non-generic base ctor ref drops to `(string, Type)` and the generic base ctor ref to `(string)`. The model's `InvokerType` is retained where it is genuinely required — to emit `CreateInstance` and the UCO constructor activation for interfaces and abstract classes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- JavaPeerProxyTests: drop the InvokerType assertions/fixture and cover the
generic `JavaPeerProxy<T>` constructor instead.
- JniRuntimeJniTypeManagerTests.GetInvokerType: tag with
[Category ("TrimmableTypeMapUnsupported")]; the trimmable typemap type
manager throws from GetInvokerTypeCore, so this reflection-based test only
applies to the other typemap implementations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the now-unused InvokerType surface from JavaPeerProxy in the trimmable typemap activation path, and updates the trimmable typemap generator/runtime/tests to rely exclusively on generated JavaPeerProxy.CreateInstance for peer activation.
Changes:
- Removed
InvokerTypefromJavaPeerProxy/JavaPeerProxy<T>and deleted the corresponding runtime lookup (TrimmableTypeMap.GetInvokerType). - Updated the trimmable typemap generator (
TypeMapAssemblyEmitter) to call the new base proxy constructors without an invoker-type argument. - Updated tests to stop asserting
InvokerTypeonJavaPeerProxyand to mark reflection-based invoker tests as unsupported for the trimmable typemap path.
Show a summary per file
| File | Description |
|---|---|
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaPeerProxyTests.cs | Updates proxy tests to match the new JavaPeerProxy constructor shapes and removes invoker-type assertions. |
| src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMapTypeManager.cs | Makes GetInvokerTypeCore unreachable in the trimmable typemap path (now throws). |
| src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMap.cs | Removes the dead GetInvokerType helper previously used to read InvokerType from proxy attributes. |
| src/Mono.Android/Java.Interop/JavaPeerProxy.cs | Removes InvokerType from the attribute API and simplifies base ctor signatures. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs | Adjusts emitted proxy IL to call updated base constructors and stops emitting invoker-type arguments there. |
| external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRuntime.JniTypeManagerTests.cs | Categorizes GetInvokerType test as unsupported under trimmable typemap. |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 2
Comment on lines
45
to
50
| protected JavaPeerProxy ( | ||
| string jniName, | ||
| Type targetType, | ||
| [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] | ||
| Type? invokerType) | ||
| Type targetType) | ||
| { | ||
| JniName = jniName ?? throw new ArgumentNullException (nameof (jniName)); | ||
| TargetType = targetType ?? throw new ArgumentNullException (nameof (targetType)); |
Comment on lines
+99
to
+102
| throw new NotSupportedException ( | ||
| $"GetInvokerTypeCore should not be called in the trimmable typemap path. " + | ||
| $"Peers for '{type.FullName}' are activated through the generated {nameof (JavaPeerProxy)}.CreateInstance, " + | ||
| $"so invoker types are never used."); |
Member
Author
|
Superseded by #11971, which has the same commits rebased on latest |
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.
Summary
The trimmable typemap activates managed peers exclusively through the generated
JavaPeerProxy.CreateInstance, so theInvokerTypeexposed on theJavaPeerProxyattribute is never consumed at runtime.JavaMarshalValueManager.CreatePeercallsTrimmableTypeMap.CreateInstanceand never falls back to invoker-type reflection.This PR removes
InvokerTypefromJavaPeerProxyand everything that fed it.Changes
Runtime (
src/Mono.Android)JavaPeerProxy/JavaPeerProxy<T>: dropped theinvokerTypeconstructor parameter and theInvokerTypeproperty.TrimmableTypeMap: removed the now-deadGetInvokerType.TrimmableTypeMapTypeManager.GetInvokerTypeCore: now throwsNotSupportedException, since it should never be reached in the trimmable path (peers are activated viaJavaPeerProxy.CreateInstance).Generator (
src/Microsoft.Android.Sdk.TrimmableTypeMap)TypeMapAssemblyEmitter: the generated proxy.ctorno longer passes an invokerType/ldnullto the base constructor; base ctor refs are now(string, Type)(non-generic) and(string)(generic). The model'sInvokerTypeis retained where genuinely required — emittingCreateInstanceand UCO constructor activation for interfaces/abstract classes.Tests
JavaPeerProxyTests: dropped theInvokerTypeassertions/fixture; added coverage for the genericJavaPeerProxy<T>constructor.JniRuntimeJniTypeManagerTests.GetInvokerType: tagged[Category ("TrimmableTypeMapUnsupported")](the trimmable type manager throws fromGetInvokerTypeCore, so this reflection-based test only applies to the other typemap implementations).Test plan
Microsoft.Android.Sdk.TrimmableTypeMap.Tests: 604/604 passing.