Skip to content
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

Jump to bottom

[Java.Interop] ignore remaining trimming warnings #1190

Merged
merged 11 commits into from
Feb 14, 2024
Merged

[Java.Interop] ignore remaining trimming warnings #1190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7482f46
[Java.Interop] ignore remaining trimming warnings
jonathanpeppers Feb 13, 2024
33bd501
Cleaner suppressions around `Invoker` types
jonathanpeppers Feb 13, 2024
4522d35
Cleaner suppressions for `GetObjectArrayMarshaler`
jonathanpeppers Feb 13, 2024
9c7a05a
Cleaner suppressions for GetParameterMarshaler
jonathanpeppers Feb 13, 2024
5300682
Cleaner suppression for ManagedPeer
jonathanpeppers Feb 13, 2024
502e852
Cleaner suppression for JniRuntime.JniTypeManager.cs
jonathanpeppers Feb 13, 2024
a493d15
Formatting
jonathanpeppers Feb 13, 2024
ea2ff53
Update JniRuntime.JniTypeManager.cs
jonathanpeppers Feb 13, 2024
ad5019d
Update JniRuntime.JniTypeManager.cs
jonathanpeppers Feb 13, 2024
f6bb010
Formatting
jonathanpeppers Feb 14, 2024
aea690e
Extra newline
jonathanpeppers Feb 14, 2024
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
5 changes: 2 additions & 3 deletions src/Java.Interop/Java.Interop.csproj
View file
Open in desktop
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
8 changes: 7 additions & 1 deletion src/Java.Interop/Java.Interop/JniRuntime.JniMarshalMemberBuilder.cs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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)]
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the late review:
This is potentially a trimming problem as well.
The trimming side of this is if you do something like:

class MyType
{
     public static void GenericMethod<[DAM(PublicMethods)] T>() {
          typeof(T).GetMethod("Foo").Invoke(...);
     }
}

class TestType
{
     public static void Foo() { }
}

public void Test()
{
     // This will crash at runtime, beacuse TestType.Foo is going to be trimmed away.
     // The only warning this code generates is below
     CallT(typeof(MyType).GetMethod("GenericMethod"), typeof(TestType));
}

private void CallT(MethodInfo genericMethod, Type typearg)
{
     // IL2060 (or something like it)
     genericMethod.MakeGenericMethod(typearg).Invoke(...);
}

The problem is that the trimmer sees that you're instantiating a generic method, but since it doesn't know which method, it must warn in the case the generic method would have an annotation on one of its type parameters.

It is also an AOT problem, that one is if the elementType is a value type, then this might fail since there might not be code for the value type instantiation of the generic method.

[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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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