Skip to content

Commit

Permalink
[Java.Interop] remove DynamicallyAccessedMemberTypes.Interfaces (#1285
Browse files Browse the repository at this point in the history
)

Context: dotnet/android#9630
Context: ec2813a

As dotnet/android#9630 required new declarations of
`DynamicallyAccessedMemberTypes.Interfaces`, we intestigated to see
*why* these were needed in Java.Interop.

There are two cases that involve `.Interfaces` within 
`JniRuntime.JniValueManager.GetValueMarshaler(Type)`:

The first case is around attempts to special-case array marshaling
for builtin types:

	var listIface   = typeof (IList<>);
	var listType    =
	    (from iface in type.GetInterfaces ().Concat (new[]{type})
	        where (listIface).IsAssignableFrom (iface.IsGenericType ? iface.GetGenericTypeDefinition () : iface)
	        select iface)
	    .FirstOrDefault ();

If `IList<T>` is trimmed away (lol), then *we don't care*, as
everything else within `GetValueMarshaler(Type)` still works,
and if we can't get a value marshaler for `int[]` (which implements
`IList<T>`!), then so be it.  Suppress this IL2070 message.

The second case is around looking for `[JniValueMarshaler]` on
implemented interface types, from ec2813a:

	JniValueMarshalerAttribute? ifaceAttribute = null;
	foreach (var iface in type.GetInterfaces ()) {
	    marshalerAttr = iface.GetCustomAttribute<JniValueMarshalerAttribute> ();
	    if (marshalerAttr != null) {
	        if (ifaceAttribute != null)
	            throw new NotSupportedException ($"There is more than one interface with custom marshaler for type {type}.");

	        ifaceAttribute = marshalerAttr;
	    }
	}
	if (ifaceAttribute != null)
	    return (JniValueMarshaler) Activator.CreateInstance (ifaceAttribute.MarshalerType)!;

It is to support the scenario

	namespace Android.Runtime {
	    [JniValueMarshaler(typeof(IJavaObjectValueMarshaler))]
	    partial interface /* Android.RuntimIJavaObject {}
	}
	namespace Java.Lang {
	    partial class Object : Android.Runtime.IJavaObject {}
	}

to "retrofit" `Java.Lang.Object` and `Java.Lang.Throwable` to use
`IJavaObjectValueMarshaler`.

There are three problems with this case:

 1. It's not actually covered by unit tests!  Removing the above
    `foreach` loop doesn't break any unit tests.

 2. While dotnet/android was updated for this scenario, it was only
    as part of trying to use `jnimarshalmethod-gen` on
    Xamarin.Android projects, which was never finished.

 3. It doesn't "scale": the code throws a `NotSupportedException`
    if a type implements more than one interface that has
    `[JniValueMarshaler]`.

This isn't used, and *shouldn't* be relied upon.  Excise it entirely.

With these changes we can remove
`DynamicallyAccessedMemberTypes.Interfaces`.

I also introduced `build-tools/trim-analyzers/trim-analyzers.props`
that will setup the appropriate trimmer MSBuild properties to make
trimmer warnings an error.  This should keep us from accidentally
creating warnings.  I only use this setting in projects that were
already using `$(EnableAotAnalyzer)`.
  • Loading branch information
jonathanpeppers authored Dec 18, 2024
1 parent f800ea5 commit 2c06b3c
Show file tree
Hide file tree
Showing 14 changed files with 146 additions and 113 deletions.
55 changes: 55 additions & 0 deletions build-tools/trim-analyzers/trim-analyzers.props
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<!-- Import this to enable trim warnings of all kinds -->
<Project>
<PropertyGroup>
<!-- Sets assembly metadata, enable analyzers -->
<IsTrimmable>true</IsTrimmable>
<EnableSingleFileAnalyzer>true</EnableSingleFileAnalyzer>
<EnableAotAnalyzer>true</EnableAotAnalyzer>
<!-- In app projects, tells ILLink to emit warnings as errors -->
<ILLinkTreatWarningsAsErrors>true</ILLinkTreatWarningsAsErrors>
<!--
Trim warnings, codes listed here:
* https://github.com/dotnet/runtime/blob/7403a062960d092c73ce3f07d3ff323ffdf7de43/src/tools/illink/src/linker/Resources/Strings.resx
* https://github.com/dotnet/docs/tree/9cb45cf9cd34f3b7259a023c3d92a124a87090d5/docs/core/deploying/trimming/trim-warnings
-->
<WarningsAsErrors>
$(WarningsAsErrors);
IL2000;IL2001;IL2002;IL2003;IL2004;
IL2005;IL2006;IL2007;IL2008;IL2009;
IL2010;IL2011;IL2012;IL2013;IL2014;
IL2015;IL2016;IL2017;IL2018;IL2019;
IL2020;IL2021;IL2022;IL2023;IL2024;
IL2025;IL2026;IL2027;IL2028;IL2029;
IL2030;IL2031;IL2032;IL2033;IL2034;
IL2035;IL2036;IL2037;IL2038;IL2039;
IL2040;IL2041;IL2042;IL2043;IL2044;
IL2045;IL2046;IL2047;IL2048;IL2049;
IL2050;IL2051;IL2052;IL2053;IL2054;
IL2055;IL2056;IL2057;IL2058;IL2059;
IL2060;IL2061;IL2062;IL2063;IL2064;
IL2065;IL2066;IL2067;IL2068;IL2069;
IL2070;IL2071;IL2072;IL2073;IL2074;
IL2075;IL2076;IL2077;IL2078;IL2079;
IL2080;IL2081;IL2082;IL2083;IL2084;
IL2085;IL2086;IL2087;IL2088;IL2089;
IL2090;IL2091;IL2092;IL2093;IL2094;
IL2095;IL2096;IL2097;IL2098;IL2099;
IL2100;IL2101;IL2102;IL2103;IL2104;
IL2105;IL2106;IL2107;IL2108;IL2109;
IL2110;IL2111;IL2112;IL2113;IL2114;
IL2115;IL2116;IL2117;IL2118;IL2119;
IL2120;IL2121;IL2122;IL2123;IL2124;
IL2125;IL2126;IL2127;IL2128;IL2129;
</WarningsAsErrors>
<!-- In NativeAOT app projects, tells Ilc to emit warnings as errors -->
<IlcTreatWarningsAsErrors>true</IlcTreatWarningsAsErrors>
<!--
NativeAOT warnings, codes listed here:
* https://github.com/dotnet/docs/tree/9cb45cf9cd34f3b7259a023c3d92a124a87090d5/docs/core/deploying/native-aot/warnings
-->
<WarningsAsErrors>
$(WarningsAsErrors);
IL3050;IL3051;IL3052;IL3053;IL3054;IL3055;IL3056;
</WarningsAsErrors>
</PropertyGroup>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@
<TargetFramework>$(DotNetTargetFramework)</TargetFramework>
<Nullable>enable</Nullable>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<IsTrimmable>true</IsTrimmable>
<EnableSingleFileAnalyzer>true</EnableSingleFileAnalyzer>
<EnableAotAnalyzer>true</EnableAotAnalyzer>
<DefineConstants>$(DefineConstants);JCW_ONLY_TYPE_NAMES;HAVE_CECIL</DefineConstants>
</PropertyGroup>

<Import Project="..\..\TargetFrameworkDependentValues.props" />
<Import Project="..\..\build-tools\trim-analyzers\trim-analyzers.props" />
<Import Project="..\..\build-tools\scripts\cecil.projitems" />

<ItemGroup>
Expand Down
4 changes: 1 addition & 3 deletions src/Java.Interop/Java.Interop.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,13 @@
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Nullable>enable</Nullable>
<ProduceReferenceAssembly>true</ProduceReferenceAssembly>
<EnableSingleFileAnalyzer>true</EnableSingleFileAnalyzer>
<IsTrimmable>true</IsTrimmable>
<EnableAotAnalyzer>true</EnableAotAnalyzer>
<MSBuildWarningsAsMessages>NU1702</MSBuildWarningsAsMessages>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' == 'Debug' ">
<DefineConstants>DEBUG;$(DefineConstants)</DefineConstants>
</PropertyGroup>
<Import Project="..\..\TargetFrameworkDependentValues.props" />
<Import Project="..\..\build-tools\trim-analyzers\trim-analyzers.props" />
<PropertyGroup>
<DefineConstants>INTEROP;FEATURE_JNIOBJECTREFERENCE_INTPTRS;$(JavaInteropDefineConstants)</DefineConstants>
<IntermediateOutputPath>$(BaseIntermediateOutputPath)$(Configuration)\$(TargetFramework.ToLowerInvariant())\</IntermediateOutputPath>
Expand Down
2 changes: 1 addition & 1 deletion src/Java.Interop/Java.Interop/JavaArray.cs
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ public void Dispose ()
}

public abstract class JavaPrimitiveArray<
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
T
>
: JavaArray<T>
Expand Down
1 change: 0 additions & 1 deletion src/Java.Interop/Java.Interop/JavaObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ namespace Java.Interop
unsafe public class JavaObject : IJavaPeerable
{
internal const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors;
internal const DynamicallyAccessedMemberTypes ConstructorsAndInterfaces = Constructors | DynamicallyAccessedMemberTypes.Interfaces;

readonly static JniPeerMembers _members = new JniPeerMembers ("java/lang/Object", typeof (JavaObject));

Expand Down
6 changes: 3 additions & 3 deletions src/Java.Interop/Java.Interop/JavaObjectArray.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
namespace Java.Interop
{
public class JavaObjectArray<
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
T
>
: JavaArray<T>
Expand Down Expand Up @@ -171,7 +171,7 @@ internal sealed class ValueMarshaler : JniValueMarshaler<IList<T>> {
public override IList<T> CreateGenericValue (
ref JniObjectReference reference,
JniObjectReferenceOptions options,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return JavaArray<T>.CreateValue (ref reference, options, targetType, (ref JniObjectReference h, JniObjectReferenceOptions t) => new JavaObjectArray<T> (ref h, t) {
Expand Down Expand Up @@ -203,7 +203,7 @@ partial class JniEnvironment {
partial class Arrays {

public static JavaObjectArray<T>? CreateMarshalObjectArray<
[DynamicallyAccessedMembers (JavaObject.ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (JavaObject.Constructors)]
T
> (
IEnumerable<T>? value)
Expand Down
32 changes: 16 additions & 16 deletions src/Java.Interop/Java.Interop/JavaPrimitiveArrays.cs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)

public static object? CreateMarshaledValue (
IntPtr handle,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
Expand All @@ -257,7 +257,7 @@ internal sealed class ValueMarshaler : JniValueMarshaler<IList<Boolean>> {
public override IList<Boolean> CreateGenericValue (
ref JniObjectReference reference,
JniObjectReferenceOptions options,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return JavaArray<Boolean>.CreateValue (
Expand Down Expand Up @@ -450,7 +450,7 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)

public static object? CreateMarshaledValue (
IntPtr handle,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
Expand All @@ -461,7 +461,7 @@ internal sealed class ValueMarshaler : JniValueMarshaler<IList<SByte>> {
public override IList<SByte> CreateGenericValue (
ref JniObjectReference reference,
JniObjectReferenceOptions options,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return JavaArray<SByte>.CreateValue (
Expand Down Expand Up @@ -654,7 +654,7 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)

public static object? CreateMarshaledValue (
IntPtr handle,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
Expand All @@ -665,7 +665,7 @@ internal sealed class ValueMarshaler : JniValueMarshaler<IList<Char>> {
public override IList<Char> CreateGenericValue (
ref JniObjectReference reference,
JniObjectReferenceOptions options,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return JavaArray<Char>.CreateValue (
Expand Down Expand Up @@ -858,7 +858,7 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)

public static object? CreateMarshaledValue (
IntPtr handle,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
Expand All @@ -869,7 +869,7 @@ internal sealed class ValueMarshaler : JniValueMarshaler<IList<Int16>> {
public override IList<Int16> CreateGenericValue (
ref JniObjectReference reference,
JniObjectReferenceOptions options,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return JavaArray<Int16>.CreateValue (
Expand Down Expand Up @@ -1062,7 +1062,7 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)

public static object? CreateMarshaledValue (
IntPtr handle,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
Expand All @@ -1073,7 +1073,7 @@ internal sealed class ValueMarshaler : JniValueMarshaler<IList<Int32>> {
public override IList<Int32> CreateGenericValue (
ref JniObjectReference reference,
JniObjectReferenceOptions options,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return JavaArray<Int32>.CreateValue (
Expand Down Expand Up @@ -1266,7 +1266,7 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)

public static object? CreateMarshaledValue (
IntPtr handle,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
Expand All @@ -1277,7 +1277,7 @@ internal sealed class ValueMarshaler : JniValueMarshaler<IList<Int64>> {
public override IList<Int64> CreateGenericValue (
ref JniObjectReference reference,
JniObjectReferenceOptions options,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return JavaArray<Int64>.CreateValue (
Expand Down Expand Up @@ -1470,7 +1470,7 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)

public static object? CreateMarshaledValue (
IntPtr handle,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
Expand All @@ -1481,7 +1481,7 @@ internal sealed class ValueMarshaler : JniValueMarshaler<IList<Single>> {
public override IList<Single> CreateGenericValue (
ref JniObjectReference reference,
JniObjectReferenceOptions options,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return JavaArray<Single>.CreateValue (
Expand Down Expand Up @@ -1674,7 +1674,7 @@ internal override bool TargetTypeIsCurrentType (Type? targetType)

public static object? CreateMarshaledValue (
IntPtr handle,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return ArrayMarshaler.CreateValue (handle, targetType);
Expand All @@ -1685,7 +1685,7 @@ internal sealed class ValueMarshaler : JniValueMarshaler<IList<Double>> {
public override IList<Double> CreateGenericValue (
ref JniObjectReference reference,
JniObjectReferenceOptions options,
[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
return JavaArray<Double>.CreateValue (
Expand Down
Loading

0 comments on commit 2c06b3c

Please sign in to comment.