Skip to content

[Mono.Android] suppress/solve more illink warnings #8063

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 14, 2023
Merged

[Mono.Android] suppress/solve more illink warnings #8063

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b4e058a
[Mono.Android] suppress/solve more illink warnings
jonathanpeppers May 24, 2023
b5fcc0d
Code formatting
jonathanpeppers Jun 12, 2023
10b6cb9
Missing space!
jonathanpeppers Jun 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/Mono.Android/Android.Runtime/AndroidRuntime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ protected override IEnumerable<string> GetSimpleReferences (Type type)

// See ExportAttribute.cs
[UnconditionalSuppressMessage ("Trimming", "IL2026", Justification = "Mono.Android.Export.dll is preserved when [Export] is used via [DynamicDependency].")]
[UnconditionalSuppressMessage ("Trimming", "IL2075", Justification = "Mono.Android.Export.dll is preserved when [Export] is used via [DynamicDependency].")]
static Delegate CreateDynamicCallback (MethodInfo method)
{
if (dynamic_callback_gen == null) {
Expand Down Expand Up @@ -479,10 +480,13 @@ static bool CallRegisterMethodByIndex (JniNativeMethodRegistrationArguments argu
}
}

public override void RegisterNativeMembers (JniType nativeClass, Type type, string? methods) =>
public override void RegisterNativeMembers (JniType nativeClass, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)] Type type, string? methods) =>
RegisterNativeMembers (nativeClass, type, methods.AsSpan ());

public void RegisterNativeMembers (JniType nativeClass, Type type, ReadOnlySpan<char> methods)
[UnconditionalSuppressMessage ("Trimming", "IL2057", Justification = "Type.GetType() can never statically know the string value parsed from parameter 'methods'.")]
[UnconditionalSuppressMessage ("Trimming", "IL2067", Justification = "Delegate.CreateDelegate() can never statically know the string value parsed from parameter 'methods'.")]
[UnconditionalSuppressMessage ("Trimming", "IL2072", Justification = "Delegate.CreateDelegate() can never statically know the string value parsed from parameter 'methods'.")]
public void RegisterNativeMembers (JniType nativeClass, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)] Type type, ReadOnlySpan<char> methods)
Copy link
Contributor

@jonpryor jonpryor Jun 12, 2023

Choose a reason for hiding this comment

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

Code-formatting wise, I think adding these parameter custom attributes makes the resulting code less readable, so I would prefer changing these into multi-line signatures:

public void RegisterNativeMembers (
        JniType nativeClass,
        [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)]
        Type type,
        ReadOnlySpan<char> methods)
{
    // …
}

Additionally, why [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)] and not [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)]?

Copy link
Contributor

@jonpryor jonpryor Jun 13, 2023

Choose a reason for hiding this comment

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

Apparently the trimmer warning specifies which values to use.

{
try {
if (methods.IsEmpty) {
Expand Down
1 change: 1 addition & 0 deletions src/Mono.Android/Java.Interop/JavaObjectExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ static IJavaObject CastClass (IJavaObject instance, Type resultType)
// typeof(Foo) -> FooInvoker
// typeof(Foo<>) -> FooInvoker`1
[UnconditionalSuppressMessage ("Trimming", "IL2026", Justification = "*Invoker types are preserved by the MarkJavaObjects linker step.")]
[UnconditionalSuppressMessage ("Trimming", "IL2055", Justification = "*Invoker types are preserved by the MarkJavaObjects linker step.")]
internal static Type? GetInvokerType (Type type)
{
const string suffix = "Invoker";
Expand Down
8 changes: 7 additions & 1 deletion src/Mono.Android/Java.Interop/TypeManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using System.Linq;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -116,6 +117,7 @@ internal static bool ActivationEnabled {
}
#endif // !JAVA_INTEROP

[UnconditionalSuppressMessage ("Trimming", "IL2057", Justification = "Type.GetType() can never statically know the string value from parameter 'signature'.")]
static Type[] GetParameterTypes (string? signature)
{
if (String.IsNullOrEmpty (signature))
Expand All @@ -127,6 +129,7 @@ static Type[] GetParameterTypes (string? signature)
return result;
}

[UnconditionalSuppressMessage ("Trimming", "IL2057", Justification = "Type.GetType() can never statically know the string value from parameter 'typename_ptr'.")]
static void n_Activate (IntPtr jnienv, IntPtr jclass, IntPtr typename_ptr, IntPtr signature_ptr, IntPtr jobject, IntPtr parameters_ptr)
{
var o = Java.Lang.Object.PeekObject (jobject);
Expand Down Expand Up @@ -163,6 +166,7 @@ static void n_Activate (IntPtr jnienv, IntPtr jclass, IntPtr typename_ptr, IntPt
Activate (jobject, cinfo, parms);
}

[UnconditionalSuppressMessage ("Trimming", "IL2072", Justification = "RuntimeHelpers.GetUninitializedObject() does not statically know the return value from ConstructorInfo.DeclaringType.")]
internal static void Activate (IntPtr jobject, ConstructorInfo cinfo, object? []? parms)
{
try {
Expand Down Expand Up @@ -257,6 +261,8 @@ internal static IJavaPeerable CreateInstance (IntPtr handle, JniHandleOwnership
return CreateInstance (handle, transfer, null);
}

[UnconditionalSuppressMessage ("Trimming", "IL2067", Justification = "TypeManager.CreateProxy() does not statically know the value of the 'type' local variable.")]
[UnconditionalSuppressMessage ("Trimming", "IL2072", Justification = "TypeManager.CreateProxy() does not statically know the value of the 'type' local variable.")]
internal static IJavaPeerable CreateInstance (IntPtr handle, JniHandleOwnership transfer, Type? targetType)
{
Type? type = null;
Expand Down Expand Up @@ -318,7 +324,7 @@ internal static IJavaPeerable CreateInstance (IntPtr handle, JniHandleOwnership
static readonly Type[] XAConstructorSignature = new Type [] { typeof (IntPtr), typeof (JniHandleOwnership) };
static readonly Type[] JIConstructorSignature = new Type [] { typeof (JniObjectReference).MakeByRefType (), typeof (JniObjectReferenceOptions) };

internal static object CreateProxy (Type type, IntPtr handle, JniHandleOwnership transfer)
internal static object CreateProxy ([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type type, IntPtr handle, JniHandleOwnership transfer)
Copy link
Contributor

@jonpryor jonpryor Jun 12, 2023

Choose a reason for hiding this comment

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

Same code-formatting comments as above.

{
// Skip Activator.CreateInstance() as that requires public constructors,
// and we want to hide some constructors for sanity reasons.
Expand Down