From 71039b4ce2ae809a5ee736e5998865342abd54b7 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Fri, 16 Feb 2024 09:40:05 -0600 Subject: [PATCH 1/3] [Mono.Android] fix a set of the "easiest" trimmer warnings Context: https://github.com/xamarin/xamarin-android/issues/5652 Introduce a new `trim-analyzers.targets` file to be used in this repo: 1. Enables appropriate analyzers. 2. `$(ILLinkTreatWarningsAsErrors)` only does anything in builds that would run `ILLink`, such as an application build. Useful to be here. 3. List all the explicit ILLink and AOT warnings, so that they trigger build errors. Eventually we should be able to import this file for `Mono.Android.csproj`. Fixing the following so far: ~~ TypeConverter ~~ Context: https://source.dot.net/#System.ComponentModel.TypeConverter/System/ComponentModel/TypeConverter.cs,226 src\Mono.Android\System.Drawing\SizeFConverter.cs(121,49): error IL2046: Base member 'System.ComponentModel.TypeConverter.GetProperties(ITypeDescriptorContext, Object, Attribute[])' with 'RequiresUnreferencedCodeAttribute' has a derived member 'System.Drawing.SizeFConverter.GetProperties(ITypeDescriptorContext, Object, Attribute[])' without 'RequiresUnreferencedCodeAttribute'. 'RequiresUnreferencedCodeAttribute' annotations must match across all interface implementations or overrides. Various `TypeConverter` implementations need to specify `[RequiresUnreferencedCode]` to match the base type. ~~ ColorValueMarshaler & IJavaObjectValueMarshaler ~~ Context: https://github.com/xamarin/java.interop/commit/7d1e7057cf4b0adcf65e7064186326dafce11b72 From the trimmer warnings solved in `Java.Interop.dll`, we need to bring these changes forward to any `*Marshaler` types. ~~ AndroidClientHandler ~~ 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.NonPublicFields' in call to 'System.Type.GetField(String, BindingFlags)'. The return value of method 'System.Collections.IEnumerator.Current.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. This class had a loop that was not trimming safe: for (var type = GetType (); type != null; type = type.BaseType) { field = type.GetField (fieldName, BindingFlags.Instance | BindingFlags.NonPublic); if (field != null) break; } If we *look* at the actual base types of `AndroidClientHandler`, we can simplify this loop to something that *is* trimming safe: const BindingFlags flags = BindingFlags.Instance | BindingFlags.NonPublic; FieldInfo? field = typeof (HttpClientHandler).GetField (fieldName, flags) ?? typeof (HttpMessageHandler).GetField (fieldName, flags); There should be no need to iterate *beyond* `HttpMessageHandler`, the code is looking for this field: https://github.com/dotnet/runtime/blob/135fec006e727a31763271984cd712f1659ccbd3/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs#L25 --- build-tools/trim-analyzers/trim-analyzers.targets | 15 +++++++++++++++ src/Mono.Android/Android.Graphics/Color.cs | 12 +++++++++++- .../Android.Runtime/IJavaObjectValueMarshaler.cs | 11 ++++++++++- src/Mono.Android/Mono.Android.csproj | 2 ++ src/Mono.Android/System.Drawing/PointConverter.cs | 2 ++ .../System.Drawing/RectangleConverter.cs | 2 ++ src/Mono.Android/System.Drawing/SizeConverter.cs | 2 ++ src/Mono.Android/System.Drawing/SizeFConverter.cs | 2 ++ .../Xamarin.Android.Net/AndroidClientHandler.cs | 11 +++-------- 9 files changed, 49 insertions(+), 10 deletions(-) create mode 100644 build-tools/trim-analyzers/trim-analyzers.targets diff --git a/build-tools/trim-analyzers/trim-analyzers.targets b/build-tools/trim-analyzers/trim-analyzers.targets new file mode 100644 index 00000000000..fffb0656c63 --- /dev/null +++ b/build-tools/trim-analyzers/trim-analyzers.targets @@ -0,0 +1,15 @@ + + + + + true + true + true + + true + + $(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;IL2130;IL2131;IL2132;IL2133;IL2134;IL2135;IL2136;IL2137;IL2138;IL2139;IL2140;IL2141;IL2142;IL2143;IL2144;IL2145;IL2146;IL2147;IL2148;IL2149;IL2150;IL2151;IL2152;IL2153;IL2154;IL2155;IL2156;IL2157 + + $(WarningsAsErrors);IL3050;IL3051;IL3052;IL3053;IL3054;IL3055;IL3056 + + diff --git a/src/Mono.Android/Android.Graphics/Color.cs b/src/Mono.Android/Android.Graphics/Color.cs index bf57968663c..97a46fd5b11 100644 --- a/src/Mono.Android/Android.Graphics/Color.cs +++ b/src/Mono.Android/Android.Graphics/Color.cs @@ -395,11 +395,18 @@ public static void RGBToHSV (int red, int green, int blue, float[] hsv) public class ColorValueMarshaler : JniValueMarshaler { + const DynamicallyAccessedMemberTypes ConstructorsAndInterfaces = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors | DynamicallyAccessedMemberTypes.Interfaces; + const string ExpressionRequiresUnreferencedCode = "System.Linq.Expression usage may trim away required code."; + public override Type MarshalType { get { return typeof (int); } } - public override Color CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type targetType) + public override Color CreateGenericValue ( + ref JniObjectReference reference, + JniObjectReferenceOptions options, + [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] + Type targetType) { throw new NotImplementedException (); } @@ -414,6 +421,7 @@ public override void DestroyGenericArgumentState (Color value, ref JniValueMarsh throw new NotImplementedException (); } + [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)] public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize, Type targetType) { var c = typeof (Color).GetConstructor (new[]{typeof (int)})!; @@ -424,6 +432,7 @@ public override Expression CreateParameterToManagedExpression (JniValueMarshaler return v; } + [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)] public override Expression CreateParameterFromManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize) { var r = Expression.Variable (MarshalType, sourceValue.Name + "_p"); @@ -433,6 +442,7 @@ public override Expression CreateParameterFromManagedExpression (JniValueMarshal return r; } + [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)] public override Expression CreateReturnValueFromManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue) { return CreateParameterFromManagedExpression (context, sourceValue, 0); diff --git a/src/Mono.Android/Android.Runtime/IJavaObjectValueMarshaler.cs b/src/Mono.Android/Android.Runtime/IJavaObjectValueMarshaler.cs index f5cab61ec1d..89436ab2b08 100644 --- a/src/Mono.Android/Android.Runtime/IJavaObjectValueMarshaler.cs +++ b/src/Mono.Android/Android.Runtime/IJavaObjectValueMarshaler.cs @@ -10,9 +10,16 @@ namespace Android.Runtime { sealed class IJavaObjectValueMarshaler : JniValueMarshaler { + const DynamicallyAccessedMemberTypes ConstructorsAndInterfaces = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors | DynamicallyAccessedMemberTypes.Interfaces; + const string ExpressionRequiresUnreferencedCode = "System.Linq.Expression usage may trim away required code."; + internal static IJavaObjectValueMarshaler Instance = new IJavaObjectValueMarshaler (); - public override IJavaObject CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType) + public override IJavaObject CreateGenericValue ( + ref JniObjectReference reference, + JniObjectReferenceOptions options, + [DynamicallyAccessedMembers (ConstructorsAndInterfaces)] + Type? targetType) { throw new NotImplementedException (); } @@ -27,6 +34,7 @@ public override void DestroyGenericArgumentState ([AllowNull]IJavaObject value, throw new NotImplementedException (); } + [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)] public override Expression CreateReturnValueFromManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue) { return Expression.Call ( @@ -36,6 +44,7 @@ public override Expression CreateReturnValueFromManagedExpression (JniValueMarsh sourceValue); } + [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)] public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize, Type? targetType) { var r = Expression.Variable (targetType, sourceValue.Name + "_val"); diff --git a/src/Mono.Android/Mono.Android.csproj b/src/Mono.Android/Mono.Android.csproj index 006ebd5eb41..c82b63692ce 100644 --- a/src/Mono.Android/Mono.Android.csproj +++ b/src/Mono.Android/Mono.Android.csproj @@ -3,6 +3,8 @@ + + $(DotNetTargetFramework) diff --git a/src/Mono.Android/System.Drawing/PointConverter.cs b/src/Mono.Android/System.Drawing/PointConverter.cs index 0255e55c87e..0b06d1a2d09 100644 --- a/src/Mono.Android/System.Drawing/PointConverter.cs +++ b/src/Mono.Android/System.Drawing/PointConverter.cs @@ -30,6 +30,7 @@ using System.Collections; using System.ComponentModel; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.ComponentModel.Design.Serialization; using System.Runtime.CompilerServices; @@ -133,6 +134,7 @@ public override bool GetCreateInstanceSupported (ITypeDescriptorContext context) return true; } + [RequiresUnreferencedCode("The Type of value cannot be statically discovered.")] public override PropertyDescriptorCollection? GetProperties ( ITypeDescriptorContext context, object value, Attribute[] attributes) diff --git a/src/Mono.Android/System.Drawing/RectangleConverter.cs b/src/Mono.Android/System.Drawing/RectangleConverter.cs index 364784f495f..332f94d86ea 100644 --- a/src/Mono.Android/System.Drawing/RectangleConverter.cs +++ b/src/Mono.Android/System.Drawing/RectangleConverter.cs @@ -31,6 +31,7 @@ using System.ComponentModel; using System.Collections; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Text; using System.ComponentModel.Design.Serialization; @@ -147,6 +148,7 @@ public override bool GetCreateInstanceSupported (ITypeDescriptorContext context) return true; } + [RequiresUnreferencedCode("The Type of value cannot be statically discovered.")] public override PropertyDescriptorCollection? GetProperties ( ITypeDescriptorContext context, object value, Attribute[] attributes) diff --git a/src/Mono.Android/System.Drawing/SizeConverter.cs b/src/Mono.Android/System.Drawing/SizeConverter.cs index 53fe055733c..2cf096d2a56 100644 --- a/src/Mono.Android/System.Drawing/SizeConverter.cs +++ b/src/Mono.Android/System.Drawing/SizeConverter.cs @@ -31,6 +31,7 @@ using System.Collections; using System.ComponentModel; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.ComponentModel.Design.Serialization; using System.Reflection; @@ -135,6 +136,7 @@ public override bool GetCreateInstanceSupported (ITypeDescriptorContext context) return true; } + [RequiresUnreferencedCode("The Type of value cannot be statically discovered.")] public override PropertyDescriptorCollection? GetProperties ( ITypeDescriptorContext context, object value, Attribute[] attributes) diff --git a/src/Mono.Android/System.Drawing/SizeFConverter.cs b/src/Mono.Android/System.Drawing/SizeFConverter.cs index 8933839fba2..5d831e30c52 100644 --- a/src/Mono.Android/System.Drawing/SizeFConverter.cs +++ b/src/Mono.Android/System.Drawing/SizeFConverter.cs @@ -32,6 +32,7 @@ using System; using System.Collections; using System.ComponentModel; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.ComponentModel.Design.Serialization; using System.Reflection; @@ -118,6 +119,7 @@ public override bool GetCreateInstanceSupported (ITypeDescriptorContext context) return true; } + [RequiresUnreferencedCode("The Type of value cannot be statically discovered.")] public override PropertyDescriptorCollection? GetProperties (ITypeDescriptorContext context, object value, Attribute[] attributes) { if (value is SizeF) diff --git a/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs b/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs index 83e858e1a62..f12317c1ead 100644 --- a/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs +++ b/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs @@ -313,14 +313,9 @@ protected virtual Task SetupRequest (HttpRequestMessage request, HttpURLConnecti object? GetUnderlyingHandler () { var fieldName = "_nativeHandler"; - FieldInfo? field = null; - - for (var type = GetType (); type != null; type = type.BaseType) { - field = type.GetField (fieldName, BindingFlags.Instance | BindingFlags.NonPublic); - if (field != null) - break; - } - + const BindingFlags flags = BindingFlags.Instance | BindingFlags.NonPublic; + FieldInfo? field = typeof (HttpClientHandler).GetField (fieldName, flags) ?? + typeof (HttpMessageHandler).GetField (fieldName, flags); if (field == null) { throw new InvalidOperationException ($"Field '{fieldName}' is missing from type '{GetType ()}'."); } From 617f1bb3eb711e7b421ce7db7a0c337798c85d08 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 21 Feb 2024 10:55:59 -0600 Subject: [PATCH 2/3] Remove `trim-analyzers.targets` for now --- build-tools/trim-analyzers/trim-analyzers.targets | 15 --------------- src/Mono.Android/Mono.Android.csproj | 2 -- 2 files changed, 17 deletions(-) delete mode 100644 build-tools/trim-analyzers/trim-analyzers.targets diff --git a/build-tools/trim-analyzers/trim-analyzers.targets b/build-tools/trim-analyzers/trim-analyzers.targets deleted file mode 100644 index fffb0656c63..00000000000 --- a/build-tools/trim-analyzers/trim-analyzers.targets +++ /dev/null @@ -1,15 +0,0 @@ - - - - - true - true - true - - true - - $(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;IL2130;IL2131;IL2132;IL2133;IL2134;IL2135;IL2136;IL2137;IL2138;IL2139;IL2140;IL2141;IL2142;IL2143;IL2144;IL2145;IL2146;IL2147;IL2148;IL2149;IL2150;IL2151;IL2152;IL2153;IL2154;IL2155;IL2156;IL2157 - - $(WarningsAsErrors);IL3050;IL3051;IL3052;IL3053;IL3054;IL3055;IL3056 - - diff --git a/src/Mono.Android/Mono.Android.csproj b/src/Mono.Android/Mono.Android.csproj index c82b63692ce..006ebd5eb41 100644 --- a/src/Mono.Android/Mono.Android.csproj +++ b/src/Mono.Android/Mono.Android.csproj @@ -3,8 +3,6 @@ - - $(DotNetTargetFramework) From 0898584daa89a378e5a0282ec1f52a726cc0db3e Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Fri, 23 Feb 2024 13:11:03 -0600 Subject: [PATCH 3/3] Formatting --- src/Mono.Android/System.Drawing/PointConverter.cs | 2 +- src/Mono.Android/System.Drawing/RectangleConverter.cs | 2 +- src/Mono.Android/System.Drawing/SizeConverter.cs | 2 +- src/Mono.Android/System.Drawing/SizeFConverter.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Mono.Android/System.Drawing/PointConverter.cs b/src/Mono.Android/System.Drawing/PointConverter.cs index 0b06d1a2d09..98a791cf951 100644 --- a/src/Mono.Android/System.Drawing/PointConverter.cs +++ b/src/Mono.Android/System.Drawing/PointConverter.cs @@ -134,7 +134,7 @@ public override bool GetCreateInstanceSupported (ITypeDescriptorContext context) return true; } - [RequiresUnreferencedCode("The Type of value cannot be statically discovered.")] + [RequiresUnreferencedCode ("The Type of value cannot be statically discovered.")] public override PropertyDescriptorCollection? GetProperties ( ITypeDescriptorContext context, object value, Attribute[] attributes) diff --git a/src/Mono.Android/System.Drawing/RectangleConverter.cs b/src/Mono.Android/System.Drawing/RectangleConverter.cs index 332f94d86ea..053f07882af 100644 --- a/src/Mono.Android/System.Drawing/RectangleConverter.cs +++ b/src/Mono.Android/System.Drawing/RectangleConverter.cs @@ -148,7 +148,7 @@ public override bool GetCreateInstanceSupported (ITypeDescriptorContext context) return true; } - [RequiresUnreferencedCode("The Type of value cannot be statically discovered.")] + [RequiresUnreferencedCode ("The Type of value cannot be statically discovered.")] public override PropertyDescriptorCollection? GetProperties ( ITypeDescriptorContext context, object value, Attribute[] attributes) diff --git a/src/Mono.Android/System.Drawing/SizeConverter.cs b/src/Mono.Android/System.Drawing/SizeConverter.cs index 2cf096d2a56..f60b57442b1 100644 --- a/src/Mono.Android/System.Drawing/SizeConverter.cs +++ b/src/Mono.Android/System.Drawing/SizeConverter.cs @@ -136,7 +136,7 @@ public override bool GetCreateInstanceSupported (ITypeDescriptorContext context) return true; } - [RequiresUnreferencedCode("The Type of value cannot be statically discovered.")] + [RequiresUnreferencedCode ("The Type of value cannot be statically discovered.")] public override PropertyDescriptorCollection? GetProperties ( ITypeDescriptorContext context, object value, Attribute[] attributes) diff --git a/src/Mono.Android/System.Drawing/SizeFConverter.cs b/src/Mono.Android/System.Drawing/SizeFConverter.cs index 5d831e30c52..fb2e81fce0f 100644 --- a/src/Mono.Android/System.Drawing/SizeFConverter.cs +++ b/src/Mono.Android/System.Drawing/SizeFConverter.cs @@ -119,7 +119,7 @@ public override bool GetCreateInstanceSupported (ITypeDescriptorContext context) return true; } - [RequiresUnreferencedCode("The Type of value cannot be statically discovered.")] + [RequiresUnreferencedCode ("The Type of value cannot be statically discovered.")] public override PropertyDescriptorCollection? GetProperties (ITypeDescriptorContext context, object value, Attribute[] attributes) { if (value is SizeF)