Skip to content

Commit

Permalink
[Java.Interop] ignore remaining trimming warnings (dotnet#1190)
Browse files Browse the repository at this point in the history
Fixes: dotnet#1157

The remaining trimmer warnings are around usage of System.Reflection
such as:

  * Creating an `Invoker` type from an original type via
    `Type.Assembly.GetType()`, within
    `JniRuntime.JniValueManager.GetInvokerType()`.

  * Create a `JniValueMarshaler` corresponding to
    `ParameterInfo.ParameterType` in
    `JniRuntime.JniMarshalMemberBuilder.GetParameterMarshaler()`.

  * Create a value corresponding to `ParameterInfo.ParameterType` in
    `ManagedPeer.GetValues()`.

  * Use `typeof(JavaObjectArray<>).MakeGenericType()` in
    `JniRuntime.JniTypeManager.GetTypes(JniTypeSignature)`.

  * Use `Type.MakeArrayType()` in
    `JniRuntime.JniTypeManager.GetTypes(JniTypeSignature)`.

The trimming warnings themselves all indicate that these types "might
be trimmed", in that nothing explicitly preserves them.  However, we
have a plethora of custom trimmer steps that work to preserve these
types involved in Java interop, such as:

  * https://github.com/xamarin/xamarin-android/blob/71b6fcc92f9e86246e0fe575bc1f44bbf73c3132/src/Microsoft.Android.Sdk.ILLink/PreserveApplications.cs
  * https://github.com/xamarin/xamarin-android/blob/71b6fcc92f9e86246e0fe575bc1f44bbf73c3132/src/Microsoft.Android.Sdk.ILLink/PreserveExportedTypes.cs
  * https://github.com/xamarin/xamarin-android/blob/71b6fcc92f9e86246e0fe575bc1f44bbf73c3132/src/Microsoft.Android.Sdk.ILLink/PreserveJavaExceptions.cs
  * https://github.com/xamarin/xamarin-android/blob/71b6fcc92f9e86246e0fe575bc1f44bbf73c3132/src/Microsoft.Android.Sdk.ILLink/PreserveJavaInterfaces.cs
  * https://github.com/xamarin/xamarin-android/blob/71b6fcc92f9e86246e0fe575bc1f44bbf73c3132/src/Microsoft.Android.Sdk.ILLink/PreserveRegistrations.cs

One day, in a perfect world, we could remove these trimmer steps and
completely rely on the trimmer's warnings & attributing mechanisms.
That day doesn't have to be today -- as our goal is begin surfacing
trimmer warnings to users in their own code and dependencies.

With these warnings ignored, we can fully enable analyzers with:

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

We can then remove:

	[assembly: AssemblyMetadata ("IsTrimmable", "True")]

As the MSBuild property does this for us, in addition to enabling
analyzers.

I don't think we should enable `$(IsAotCompatible)` quite yet, as
`Java.Interop.dll` likely *won't work* yet on NativeAOT, and this
places metadata that says an assembly is supported.  We should just
use the `$(EnableAotAnalyzer)` analyzers for now, so we don't
introduce new issues.
  • Loading branch information
jonathanpeppers authored Feb 14, 2024
1 parent b8f6f88 commit 7d1e705
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 18 deletions.
5 changes: 2 additions & 3 deletions src/Java.Interop/Java.Interop.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@
<Nullable>enable</Nullable>
<ProduceReferenceAssembly>true</ProduceReferenceAssembly>
<EnableSingleFileAnalyzer>true</EnableSingleFileAnalyzer>
<!-- TODO: enable these when all warnings are solved -->
<!--<IsTrimmable>true</IsTrimmable>-->
<!--<EnableAotAnalyzer>true</EnableAotAnalyzer>-->
<IsTrimmable>true</IsTrimmable>
<EnableAotAnalyzer>true</EnableAotAnalyzer>
<MSBuildWarningsAsMessages>NU1702</MSBuildWarningsAsMessages>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' == 'Debug' ">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,12 @@ string GetTypeSignature (ParameterInfo p)

public JniValueMarshaler GetParameterMarshaler (ParameterInfo parameter)
{
// Activator.CreateInstance requires DynamicallyAccessedMemberTypes.PublicParameterlessConstructor
// GetValueMarshaler requires DynamicallyAccessedMemberTypes.Interfaces
[UnconditionalSuppressMessage ("Trimming", "IL2072", Justification = "JniValueMarshalerAttribute is decorated with [DynamicallyAccessedMembers]")]
static JniValueMarshaler GetValueMarshaler (JniValueManager manager, ParameterInfo parameter) =>
manager.GetValueMarshaler (parameter.ParameterType);

if (parameter.ParameterType == typeof (IntPtr))
return IntPtrValueMarshaler.Instance;

Expand All @@ -164,7 +170,7 @@ public JniValueMarshaler GetParameterMarshaler (ParameterInfo parameter)
if (attr != null) {
return (JniValueMarshaler) Activator.CreateInstance (attr.MarshalerType)!;
}
return Runtime.ValueManager.GetValueMarshaler (parameter.ParameterType);
return GetValueMarshaler (Runtime.ValueManager, parameter);
}

// Heuristic: if first two parameters are IntPtr, this is a "direct" wrapper.
Expand Down
23 changes: 19 additions & 4 deletions src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,18 @@ protected virtual IEnumerable<string> GetSimpleReferences (Type type)

static readonly string[] EmptyStringArray = Array.Empty<string> ();
static readonly Type[] EmptyTypeArray = Array.Empty<Type> ();
const string NotUsedInAndroid = "This code path is not used in Android projects.";

// FIXME: https://github.com/xamarin/java.interop/issues/1192
[UnconditionalSuppressMessage ("AOT", "IL3050", Justification = NotUsedInAndroid)]
static Type MakeArrayType (Type type) => type.MakeArrayType ();

// FIXME: https://github.com/xamarin/java.interop/issues/1192
[UnconditionalSuppressMessage ("Trimming", "IL2055", Justification = NotUsedInAndroid)]
[UnconditionalSuppressMessage ("AOT", "IL3050", Justification = NotUsedInAndroid)]
static Type MakeGenericType (Type type, Type arrayType) => type.MakeGenericType (arrayType);

[UnconditionalSuppressMessage ("Trimming", "IL2073", Justification = "Types returned here should be preserved via other means.")]
[return: DynamicallyAccessedMembers (MethodsConstructorsInterfaces)]
public Type? GetType (JniTypeSignature typeSignature)
{
Expand Down Expand Up @@ -309,7 +319,7 @@ IEnumerable<Type> CreateGetTypesEnumerator (JniTypeSignature typeSignature)
var rank = typeSignature.ArrayRank;
var arrayType = type;
while (rank-- > 0) {
arrayType = typeof (JavaObjectArray<>).MakeGenericType (arrayType);
arrayType = MakeGenericType (typeof (JavaObjectArray<>), arrayType);
}
yield return arrayType;
}
Expand All @@ -318,7 +328,7 @@ IEnumerable<Type> CreateGetTypesEnumerator (JniTypeSignature typeSignature)
var rank = typeSignature.ArrayRank;
var arrayType = type;
while (rank-- > 0) {
arrayType = arrayType.MakeArrayType ();
arrayType = MakeArrayType (arrayType);
}
yield return arrayType;
}
Expand All @@ -341,14 +351,14 @@ IEnumerable<Type> GetPrimitiveArrayTypesForSimpleReference (JniTypeSignature typ
var rank = typeSignature.ArrayRank-1;
var arrayType = t;
while (rank-- > 0) {
arrayType = typeof (JavaObjectArray<>).MakeGenericType (arrayType);
arrayType = MakeGenericType (typeof (JavaObjectArray<>), arrayType);
}
yield return arrayType;

rank = typeSignature.ArrayRank-1;
arrayType = t;
while (rank-- > 0) {
arrayType = arrayType.MakeArrayType ();
arrayType = MakeArrayType (arrayType);
}
yield return arrayType;
}
Expand Down Expand Up @@ -462,6 +472,11 @@ protected bool TryRegisterNativeMembers (

static Type [] registerMethodParameters = new Type [] { typeof (JniNativeMethodRegistrationArguments) };

// https://github.com/xamarin/xamarin-android/blob/5472eec991cc075e4b0c09cd98a2331fb93aa0f3/src/Microsoft.Android.Sdk.ILLink/PreserveRegistrations.cs#L85
const string MarshalMethods = "'jni_marshal_methods' is preserved by the PreserveRegistrations trimmer step.";

[UnconditionalSuppressMessage ("Trimming", "IL2072", Justification = MarshalMethods)]
[UnconditionalSuppressMessage ("Trimming", "IL2075", Justification = MarshalMethods)]
bool TryLoadJniMarshalMethods (
JniType nativeClass,
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.NonPublicNestedTypes)]
Expand Down
36 changes: 31 additions & 5 deletions src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -359,18 +359,36 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type)
static Type? GetInvokerType (Type type)
{
const string suffix = "Invoker";

// https://github.com/xamarin/xamarin-android/blob/5472eec991cc075e4b0c09cd98a2331fb93aa0f3/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs#L176-L186
const string assemblyGetTypeMessage = "'Invoker' types are preserved by the MarkJavaObjects trimmer step.";
const string makeGenericTypeMessage = "Generic 'Invoker' types are preserved by the MarkJavaObjects trimmer step.";

[UnconditionalSuppressMessage ("Trimming", "IL2026", Justification = assemblyGetTypeMessage)]
[UnconditionalSuppressMessage ("Trimming", "IL2073", Justification = assemblyGetTypeMessage)]
[return: DynamicallyAccessedMembers (Constructors)]
static Type? AssemblyGetType (Assembly assembly, string typeName) =>
assembly.GetType (typeName);

// FIXME: https://github.com/xamarin/java.interop/issues/1192
[UnconditionalSuppressMessage ("Trimming", "IL2055", Justification = makeGenericTypeMessage)]
[UnconditionalSuppressMessage ("AOT", "IL3050", Justification = makeGenericTypeMessage)]
[return: DynamicallyAccessedMembers (Constructors)]
static Type MakeGenericType (Type type, Type [] arguments) =>
type.MakeGenericType (arguments);

Type[] arguments = type.GetGenericArguments ();
if (arguments.Length == 0)
return type.Assembly.GetType (type + suffix);
return AssemblyGetType (type.Assembly, type + suffix);
Type definition = type.GetGenericTypeDefinition ();
int bt = definition.FullName!.IndexOf ("`", StringComparison.Ordinal);
if (bt == -1)
throw new NotSupportedException ("Generic type doesn't follow generic type naming convention! " + type.FullName);
Type? suffixDefinition = definition.Assembly.GetType (
Type? suffixDefinition = AssemblyGetType (definition.Assembly,
definition.FullName.Substring (0, bt) + suffix + definition.FullName.Substring (bt));
if (suffixDefinition == null)
return null;
return suffixDefinition.MakeGenericType (arguments);
return MakeGenericType (suffixDefinition, arguments);
}

public object? CreateValue (
Expand Down Expand Up @@ -634,9 +652,17 @@ public JniValueMarshaler GetValueMarshaler (

static JniValueMarshaler GetObjectArrayMarshaler (Type elementType)
{
const string makeGenericMethodMessage = "This code path is not used in Android projects.";

// FIXME: https://github.com/xamarin/java.interop/issues/1192
[UnconditionalSuppressMessage ("Trimming", "IL2060", Justification = makeGenericMethodMessage)]
[UnconditionalSuppressMessage ("AOT", "IL3050", Justification = makeGenericMethodMessage)]
static MethodInfo MakeGenericMethod (MethodInfo method, Type type) =>
method.MakeGenericMethod (type);

Func<JniValueMarshaler> indirect = GetObjectArrayMarshalerHelper<object>;
var reifiedMethodInfo = indirect.Method.GetGenericMethodDefinition ()
.MakeGenericMethod (elementType);
var reifiedMethodInfo = MakeGenericMethod (
indirect.Method.GetGenericMethodDefinition (), elementType);
Func<JniValueMarshaler> direct = (Func<JniValueMarshaler>) Delegate.CreateDelegate (typeof (Func<JniValueMarshaler>), reifiedMethodInfo);
return direct ();
}
Expand Down
9 changes: 6 additions & 3 deletions src/Java.Interop/Java.Interop/JniValueMarshalerAttribute.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#nullable enable
#nullable enable

using System;
using System.Diagnostics.CodeAnalysis;
Expand All @@ -8,13 +8,16 @@ namespace Java.Interop {

[AttributeUsage (Targets, AllowMultiple=false)]
public class JniValueMarshalerAttribute : Attribute {
const DynamicallyAccessedMemberTypes ParameterlessConstructorsInterfaces = DynamicallyAccessedMemberTypes.PublicParameterlessConstructor | DynamicallyAccessedMemberTypes.Interfaces;

const AttributeTargets Targets =
AttributeTargets.Class | AttributeTargets.Enum |
AttributeTargets.Interface | AttributeTargets.Struct |
AttributeTargets.Parameter | AttributeTargets.ReturnValue;

public JniValueMarshalerAttribute (Type marshalerType)
public JniValueMarshalerAttribute (
[DynamicallyAccessedMembers (ParameterlessConstructorsInterfaces)]
Type marshalerType)
{
if (marshalerType == null)
throw new ArgumentNullException (nameof (marshalerType));
Expand All @@ -27,7 +30,7 @@ public JniValueMarshalerAttribute (Type marshalerType)
}

public Type MarshalerType {
[return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
[return: DynamicallyAccessedMembers (ParameterlessConstructorsInterfaces)]
get;
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/Java.Interop/Java.Interop/ManagedPeer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,11 @@ static List<Type>[] GetConstructorCandidateParameterTypes (string signature)

static object?[]? GetValues (JniRuntime runtime, JniObjectReference values, ConstructorInfo cinfo)
{
// https://github.com/xamarin/xamarin-android/blob/5472eec991cc075e4b0c09cd98a2331fb93aa0f3/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs#L51-L132
[UnconditionalSuppressMessage ("Trimming", "IL2072", Justification = "Constructors are preserved by the MarkJavaObjects trimmer step.")]
static object? ValueManagerGetValue (JniRuntime runtime, ref JniObjectReference value, ParameterInfo parameter) =>
runtime.ValueManager.GetValue (ref value, JniObjectReferenceOptions.CopyAndDispose, parameter.ParameterType);

if (!values.IsValid)
return null;

Expand All @@ -240,7 +245,7 @@ static List<Type>[] GetConstructorCandidateParameterTypes (string signature)
var pvalues = new object? [len];
for (int i = 0; i < len; ++i) {
var n_value = JniEnvironment.Arrays.GetObjectArrayElement (values, i);
var value = runtime.ValueManager.GetValue (ref n_value, JniObjectReferenceOptions.CopyAndDispose, parameters [i].ParameterType);
var value = ValueManagerGetValue (runtime, ref n_value, parameters [i]);
pvalues [i] = value;
}

Expand Down
1 change: 0 additions & 1 deletion src/Java.Interop/Properties/AssemblyInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
[assembly: AssemblyDescription ("")]
[assembly: AssemblyCulture ("")]
[assembly: AssemblyTrademark ("Microsoft Corporation")]
[assembly: AssemblyMetadata ("IsTrimmable", "True")]

[assembly: InternalsVisibleTo (
"Java.Interop.GenericMarshaler, PublicKey=" +
Expand Down

0 comments on commit 7d1e705

Please sign in to comment.