Skip to content

Commit

Permalink
[java.interop] address some "easy" trimmer warnings
Browse files Browse the repository at this point in the history
Context: dotnet#1157

On the path to enabling `IsAotCompatible`, we can enable some settings
to get started:

    <IsTrimmable>true</IsTrimmable>
    <EnableSingleFileAnalyzer>true</EnableSingleFileAnalyzer>
    <EnableAotAnalyzer>true</EnableAotAnalyzer>

This opts into the analyzers without declaring the assembly is fully
AOT-compatible.

Starting out, I got 33 warnings: this is an attempt to address the
ones that don't require too much thinking. Unfortunately, solving 1
warning likely will create dozens more -- as you have to update all
callers.

This results in 24 warnings remaining. Since `Release` builds have
`$(TreatWarningsAsErrors)`, I will wait to enable the analyzers until
all warnings are addressed.

~~ Example Warnings ~~

`System.Linq.Expression` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniValueManager.cs(672,58):
    warning IL2026: Using member 'System.Linq.Expressions.Expression.Property(Expression, String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed.

I decorated this one with:

    [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]

`Type.GetNestedType()` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniTypeManager.cs(447,28):
    warning IL2070: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.NonPublicNestedTypes' in call to 'System.Type.GetNestedType(String, BindingFlags)'. The parameter 'type' of method 'Java.Interop.JniRuntime.JniTypeManager.TryLoadJniMarshalMethods(JniType, Type, String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Added:

    [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.NonPublicNestedTypes)]

`Activator.CreateInstance()` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniValueManager.cs(542,33): warning IL2072: 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Activator.CreateInstance(Type)'. The return value of method 'Java.Interop.JniValueMarshalerAttribute.MarshalerType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Added:

    [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]

~~ Code that Actually Changed ~~

As I enabled more attributes, these snowball into more and more
warnings! I eventually had to make `GetPeerType()` look like:

    [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)]
    static Type GetPeerType ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type type)

The analyzer was not able to understand code like:

    static  readonly    KeyValuePair<Type, Type>[]      PeerTypeMappings = new []{
        new KeyValuePair<Type, Type>(typeof (object),           typeof (JavaObject)),
        new KeyValuePair<Type, Type>(typeof (IJavaPeerable),    typeof (JavaObject)),
        new KeyValuePair<Type, Type>(typeof (Exception),        typeof (JavaException)),
    };
    //...
    foreach (var m in PeerTypeMappings) {
        if (m.Key == type)
            return m.Value;
    }

Simply removing the `PeerTypeMappings` array and using `if` statements
solved the warnings. This may be the only real code change if any.
  • Loading branch information
jonathanpeppers committed Feb 8, 2024
1 parent 07c7300 commit d586016
Show file tree
Hide file tree
Showing 14 changed files with 182 additions and 97 deletions.
4 changes: 4 additions & 0 deletions src/Java.Interop/Java.Interop.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Nullable>enable</Nullable>
<ProduceReferenceAssembly>true</ProduceReferenceAssembly>
<EnableSingleFileAnalyzer>true</EnableSingleFileAnalyzer>
<!-- TODO: enable these when all warnings are solved -->
<!--<IsTrimmable>true</IsTrimmable>-->
<!--<EnableAotAnalyzer>true</EnableAotAnalyzer>-->
<MSBuildWarningsAsMessages>NU1702</MSBuildWarningsAsMessages>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' == 'Debug' ">
Expand Down
4 changes: 3 additions & 1 deletion src/Java.Interop/Java.Interop/JavaArray.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ namespace Java.Interop
{
public abstract class JavaArray<T> : JavaObject, IList, IList<T>
{
internal const DynamicallyAccessedMemberTypes ConstructorsAndInterfaces = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors | DynamicallyAccessedMemberTypes.Interfaces;

internal delegate TArray ArrayCreator<TArray> (ref JniObjectReference reference, JniObjectReferenceOptions transfer)
where TArray : JavaArray<T>;

Expand Down Expand Up @@ -362,7 +364,7 @@ public void Dispose ()
}
}

public abstract class JavaPrimitiveArray<T> : JavaArray<T> {
public abstract class JavaPrimitiveArray<[DynamicallyAccessedMembers (ConstructorsAndInterfaces)] T> : JavaArray<T> {

internal JavaPrimitiveArray (ref JniObjectReference reference, JniObjectReferenceOptions transfer)
: base (ref reference, transfer)
Expand Down
8 changes: 4 additions & 4 deletions src/Java.Interop/Java.Interop/JavaObjectArray.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#nullable enable
#nullable enable

using System;
using System.Collections.Generic;
Expand All @@ -7,7 +7,7 @@

namespace Java.Interop
{
public class JavaObjectArray<T> : JavaArray<T>
public class JavaObjectArray<[DynamicallyAccessedMembers (ConstructorsAndInterfaces)] T> : JavaArray<T>
{
internal static readonly ValueMarshaler Instance = new ValueMarshaler ();

Expand Down Expand Up @@ -164,7 +164,7 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)

internal sealed class ValueMarshaler : JniValueMarshaler<IList<T>> {

public override IList<T> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
public override IList<T> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] Type? targetType)
{
return JavaArray<T>.CreateValue (ref reference, options, targetType, (ref JniObjectReference h, JniObjectReferenceOptions t) => new JavaObjectArray<T> (ref h, t) {
forMarshalCollection = true,
Expand Down Expand Up @@ -194,7 +194,7 @@ partial class JniEnvironment {
[SuppressMessage ("Design", "CA1034", Justification = "https://github.com/xamarin/Java.Interop/commit/bb7ca5d02aa3fc2b447ad57af1256e74e5f954fa")]
partial class Arrays {

public static JavaObjectArray<T>? CreateMarshalObjectArray<T> (IEnumerable<T>? value)
public static JavaObjectArray<T>? CreateMarshalObjectArray<[DynamicallyAccessedMembers (JavaArray<T>.ConstructorsAndInterfaces)] T> (IEnumerable<T>? value)
{
if (value == null) {
return null;
Expand Down
40 changes: 24 additions & 16 deletions src/Java.Interop/Java.Interop/JavaPrimitiveArrays.cs
Original file line number Diff line number Diff line change
Expand Up @@ -244,14 +244,14 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)
typeof (JavaBooleanArray) == targetType;
}

public static object? CreateMarshaledValue (IntPtr handle, Type? targetType)
public static object? CreateMarshaledValue (IntPtr handle, [DynamicallyAccessedMembers(JniValueMarshaler.ConstructorsAndInterfaces)] Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
}

internal sealed class ValueMarshaler : JniValueMarshaler<IList<Boolean>> {

public override IList<Boolean> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
public override IList<Boolean> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] Type? targetType)
{
return JavaArray<Boolean>.CreateValue (
ref reference,
Expand All @@ -276,6 +276,7 @@ public override void DestroyGenericArgumentState ([AllowNull] IList<Boolean> val
JavaArray<Boolean>.DestroyArgumentState<JavaBooleanArray> (value, ref state, synchronize);
}

[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize = 0, Type? targetType = null)
{
Func<IntPtr, Type?, object?> m = JavaBooleanArray.CreateMarshaledValue;
Expand Down Expand Up @@ -439,14 +440,14 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)
typeof (JavaSByteArray) == targetType;
}

public static object? CreateMarshaledValue (IntPtr handle, Type? targetType)
public static object? CreateMarshaledValue (IntPtr handle, [DynamicallyAccessedMembers (JniValueMarshaler.ConstructorsAndInterfaces)] Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
}

internal sealed class ValueMarshaler : JniValueMarshaler<IList<SByte>> {

public override IList<SByte> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
public override IList<SByte> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] Type? targetType)
{
return JavaArray<SByte>.CreateValue (
ref reference,
Expand All @@ -471,6 +472,7 @@ public override void DestroyGenericArgumentState ([AllowNull] IList<SByte> value
JavaArray<SByte>.DestroyArgumentState<JavaSByteArray> (value, ref state, synchronize);
}

[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize = 0, Type? targetType = null)
{
Func<IntPtr, Type?, object?> m = JavaSByteArray.CreateMarshaledValue;
Expand Down Expand Up @@ -634,14 +636,14 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)
typeof (JavaCharArray) == targetType;
}

public static object? CreateMarshaledValue (IntPtr handle, Type? targetType)
public static object? CreateMarshaledValue (IntPtr handle, [DynamicallyAccessedMembers (JniValueMarshaler.ConstructorsAndInterfaces)] Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
}

internal sealed class ValueMarshaler : JniValueMarshaler<IList<Char>> {

public override IList<Char> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
public override IList<Char> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] Type? targetType)
{
return JavaArray<Char>.CreateValue (
ref reference,
Expand All @@ -666,6 +668,7 @@ public override void DestroyGenericArgumentState ([AllowNull] IList<Char> value,
JavaArray<Char>.DestroyArgumentState<JavaCharArray> (value, ref state, synchronize);
}

[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize = 0, Type? targetType = null)
{
Func<IntPtr, Type?, object?> m = JavaCharArray.CreateMarshaledValue;
Expand Down Expand Up @@ -829,14 +832,14 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)
typeof (JavaInt16Array) == targetType;
}

public static object? CreateMarshaledValue (IntPtr handle, Type? targetType)
public static object? CreateMarshaledValue (IntPtr handle, [DynamicallyAccessedMembers (JniValueMarshaler.ConstructorsAndInterfaces)] Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
}

internal sealed class ValueMarshaler : JniValueMarshaler<IList<Int16>> {

public override IList<Int16> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
public override IList<Int16> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] Type? targetType)
{
return JavaArray<Int16>.CreateValue (
ref reference,
Expand All @@ -861,6 +864,7 @@ public override void DestroyGenericArgumentState ([AllowNull] IList<Int16> value
JavaArray<Int16>.DestroyArgumentState<JavaInt16Array> (value, ref state, synchronize);
}

[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize = 0, Type? targetType = null)
{
Func<IntPtr, Type?, object?> m = JavaInt16Array.CreateMarshaledValue;
Expand Down Expand Up @@ -1024,14 +1028,14 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)
typeof (JavaInt32Array) == targetType;
}

public static object? CreateMarshaledValue (IntPtr handle, Type? targetType)
public static object? CreateMarshaledValue (IntPtr handle, [DynamicallyAccessedMembers (JniValueMarshaler.ConstructorsAndInterfaces)] Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
}

internal sealed class ValueMarshaler : JniValueMarshaler<IList<Int32>> {

public override IList<Int32> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
public override IList<Int32> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] Type? targetType)
{
return JavaArray<Int32>.CreateValue (
ref reference,
Expand All @@ -1056,6 +1060,7 @@ public override void DestroyGenericArgumentState ([AllowNull] IList<Int32> value
JavaArray<Int32>.DestroyArgumentState<JavaInt32Array> (value, ref state, synchronize);
}

[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize = 0, Type? targetType = null)
{
Func<IntPtr, Type?, object?> m = JavaInt32Array.CreateMarshaledValue;
Expand Down Expand Up @@ -1219,14 +1224,14 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)
typeof (JavaInt64Array) == targetType;
}

public static object? CreateMarshaledValue (IntPtr handle, Type? targetType)
public static object? CreateMarshaledValue (IntPtr handle, [DynamicallyAccessedMembers (JniValueMarshaler.ConstructorsAndInterfaces)] Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
}

internal sealed class ValueMarshaler : JniValueMarshaler<IList<Int64>> {

public override IList<Int64> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
public override IList<Int64> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] Type? targetType)
{
return JavaArray<Int64>.CreateValue (
ref reference,
Expand All @@ -1251,6 +1256,7 @@ public override void DestroyGenericArgumentState ([AllowNull] IList<Int64> value
JavaArray<Int64>.DestroyArgumentState<JavaInt64Array> (value, ref state, synchronize);
}

[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize = 0, Type? targetType = null)
{
Func<IntPtr, Type?, object?> m = JavaInt64Array.CreateMarshaledValue;
Expand Down Expand Up @@ -1414,14 +1420,14 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)
typeof (JavaSingleArray) == targetType;
}

public static object? CreateMarshaledValue (IntPtr handle, Type? targetType)
public static object? CreateMarshaledValue (IntPtr handle, [DynamicallyAccessedMembers (JniValueMarshaler.ConstructorsAndInterfaces)] Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
}

internal sealed class ValueMarshaler : JniValueMarshaler<IList<Single>> {

public override IList<Single> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
public override IList<Single> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] Type? targetType)
{
return JavaArray<Single>.CreateValue (
ref reference,
Expand All @@ -1446,6 +1452,7 @@ public override void DestroyGenericArgumentState ([AllowNull] IList<Single> valu
JavaArray<Single>.DestroyArgumentState<JavaSingleArray> (value, ref state, synchronize);
}

[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize = 0, Type? targetType = null)
{
Func<IntPtr, Type?, object?> m = JavaSingleArray.CreateMarshaledValue;
Expand Down Expand Up @@ -1609,14 +1616,14 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)
typeof (JavaDoubleArray) == targetType;
}

public static object? CreateMarshaledValue (IntPtr handle, Type? targetType)
public static object? CreateMarshaledValue (IntPtr handle, [DynamicallyAccessedMembers (JniValueMarshaler.ConstructorsAndInterfaces)] Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
}

internal sealed class ValueMarshaler : JniValueMarshaler<IList<Double>> {

public override IList<Double> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
public override IList<Double> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] Type? targetType)
{
return JavaArray<Double>.CreateValue (
ref reference,
Expand All @@ -1641,6 +1648,7 @@ public override void DestroyGenericArgumentState ([AllowNull] IList<Double> valu
JavaArray<Double>.DestroyArgumentState<JavaDoubleArray> (value, ref state, synchronize);
}

[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize = 0, Type? targetType = null)
{
Func<IntPtr, Type?, object?> m = JavaDoubleArray.CreateMarshaledValue;
Expand Down
2 changes: 1 addition & 1 deletion src/Java.Interop/Java.Interop/JavaPrimitiveArrays.tt
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ namespace Java.Interop {

internal sealed class ValueMarshaler : JniValueMarshaler<IList<<#= info.TypeModifier #>>> {

public override IList<<#= info.TypeModifier #>> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
public override IList<<#= info.TypeModifier #>> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] Type? targetType)
{
return JavaArray<<#= info.TypeModifier #>>.CreateValue (
ref reference,
Expand Down
Loading

0 comments on commit d586016

Please sign in to comment.