From 1c8b672e9ce0dd31b16897c088f3ca5132bdaea3 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Thu, 28 Jan 2021 10:43:31 -0600 Subject: [PATCH 1/3] Resolve ILLink warnings in System.Linq.Expressions (Round 1) Contributes to #45623 --- .../Expressions/ExpressionResolverBuilder.cs | 4 +- .../src/ILLink/ILLink.Suppressions.xml | 122 +----------------- .../System/Dynamic/Utils/DelegateHelpers.cs | 10 +- .../src/System/Dynamic/Utils/TypeUtils.cs | 3 + .../Linq/Expressions/Compiler/AssemblyGen.cs | 4 +- .../System/Linq/Expressions/Compiler/ILGen.cs | 40 ++++-- .../Linq/Expressions/IndexExpression.cs | 12 +- .../Interpreter/CallInstruction.cs | 3 + .../Expressions/Interpreter/TypeOperations.cs | 5 + .../Linq/Expressions/MemberExpression.cs | 10 +- .../Linq/Expressions/MethodCallExpression.cs | 15 ++- .../System/Linq/Expressions/NewExpression.cs | 4 +- .../Runtime/CompilerServices/CallSite.cs | 5 +- .../System.Linq.Expressions.Tests.csproj | 1 + .../tests/TrimCompatibilityTests.cs | 43 ++++++ 15 files changed, 145 insertions(+), 136 deletions(-) create mode 100644 src/libraries/System.Linq.Expressions/tests/TrimCompatibilityTests.cs diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/Expressions/ExpressionResolverBuilder.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/Expressions/ExpressionResolverBuilder.cs index fc2447fca0195e..e2878ee82a140e 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/Expressions/ExpressionResolverBuilder.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/Expressions/ExpressionResolverBuilder.cs @@ -156,8 +156,8 @@ protected override Expression VisitDisposeCache(ServiceCallSite callSite, object VisitCallSiteMain(callSite, context)); } - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2060:UnrecognizedReflectionPattern", - Justification = "Calling Array.Empty() is safe since the T doesn't have linker annotations.")] + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2060:MakeGenericMethod", + Justification = "Calling Array.Empty() is safe since the T doesn't have ILLink annotations.")] internal static MethodInfo GetArrayEmptyMethodInfo(Type itemType) => ArrayEmptyMethodInfo.MakeGenericMethod(itemType); diff --git a/src/libraries/System.Linq.Expressions/src/ILLink/ILLink.Suppressions.xml b/src/libraries/System.Linq.Expressions/src/ILLink/ILLink.Suppressions.xml index dc33d005b3858f..e3bf746f90a911 100644 --- a/src/libraries/System.Linq.Expressions/src/ILLink/ILLink.Suppressions.xml +++ b/src/libraries/System.Linq.Expressions/src/ILLink/ILLink.Suppressions.xml @@ -1,96 +1,18 @@  - - ILLink - IL2060 - member - M:System.Dynamic.Utils.DelegateHelpers.#cctor - - - ILLink - IL2060 - member - M:System.Dynamic.Utils.DelegateHelpers.GetCSharpThunk(System.Type,System.Boolean,System.Reflection.ParameterInfo[]) - ILLink IL2060 member M:System.Linq.Expressions.Expression.ApplyTypeArgs(System.Reflection.MethodInfo,System.Type[]) - - ILLink - IL2060 - member - M:System.Runtime.CompilerServices.CallSite`1.CreateCustomUpdateDelegate(System.Reflection.MethodInfo) - - - ILLink - IL2060 - member - M:System.Runtime.CompilerServices.CallSite`1.MakeUpdateDelegate - - - ILLink - IL2067 - member - M:System.Linq.Expressions.Compiler.AssemblyGen.DefineType(System.String,System.Type,System.Reflection.TypeAttributes) - ILLink IL2070 member M:System.Dynamic.Utils.TypeExtensions.GetAnyStaticMethodValidated(System.Type,System.String,System.Type[]) - - ILLink - IL2070 - member - M:System.Dynamic.Utils.TypeUtils.GetInvokeMethod(System.Type) - - - ILLink - IL2070 - member - M:System.Linq.Expressions.Compiler.ILGen.EmitArray(System.Reflection.Emit.ILGenerator,System.Type) - - - ILLink - IL2070 - member - M:System.Linq.Expressions.Compiler.ILGen.EmitGetValue(System.Reflection.Emit.ILGenerator,System.Type) - - - ILLink - IL2070 - member - M:System.Linq.Expressions.Compiler.ILGen.EmitGetValueOrDefault(System.Reflection.Emit.ILGenerator,System.Type) - - - ILLink - IL2070 - member - M:System.Linq.Expressions.Compiler.ILGen.EmitHasValue(System.Reflection.Emit.ILGenerator,System.Type) - - - ILLink - IL2070 - member - M:System.Linq.Expressions.Compiler.ILGen.EmitNonNullableToNullableConversion(System.Reflection.Emit.ILGenerator,System.Type,System.Type,System.Boolean,System.Linq.Expressions.Compiler.ILocalCache) - - - ILLink - IL2070 - member - M:System.Linq.Expressions.Compiler.ILGen.EmitNullableToNullableConversion(System.Reflection.Emit.ILGenerator,System.Type,System.Type,System.Boolean,System.Linq.Expressions.Compiler.ILocalCache) - - - ILLink - IL2070 - member - M:System.Linq.Expressions.Compiler.ILGen.TryEmitILConstant(System.Reflection.Emit.ILGenerator,System.Object,System.Type) - ILLink IL2070 @@ -123,33 +45,21 @@ ILLink - IL2070 - member - M:System.Linq.Expressions.Expression.Field(System.Linq.Expressions.Expression,System.Type,System.String) - - - ILLink - IL2070 - member - M:System.Linq.Expressions.Expression.FindMethod(System.Type,System.String,System.Type[],System.Linq.Expressions.Expression[],System.Reflection.BindingFlags) - - - ILLink - IL2070 + IL2072 member - M:System.Linq.Expressions.Expression.FindProperty(System.Type,System.String,System.Linq.Expressions.Expression[],System.Reflection.BindingFlags) + M:System.Linq.Expressions.Expression.ListInit(System.Linq.Expressions.NewExpression,System.Collections.Generic.IEnumerable{System.Linq.Expressions.Expression}) ILLink - IL2070 + IL2072 member - M:System.Linq.Expressions.Expression.New(System.Type) + M:System.Linq.Expressions.Expression.Call(System.Linq.Expressions.Expression,System.String,System.Type[],System.Linq.Expressions.Expression[])) ILLink - IL2070 + IL2072 member - M:System.Linq.Expressions.Expression.Property(System.Linq.Expressions.Expression,System.Type,System.String) + M:System.Linq.Expressions.Expression.Property(System.Linq.Expressions.Expression,System.String,System.Linq.Expressions.Expression[]) ILLink @@ -193,12 +103,6 @@ member M:System.Linq.Expressions.Compiler.LambdaCompiler.EmitUnary(System.Linq.Expressions.UnaryExpression,System.Linq.Expressions.Compiler.LambdaCompiler.CompilationFlags) - - ILLink - IL2075 - member - M:System.Linq.Expressions.Expression.ArrayIndex(System.Linq.Expressions.Expression,System.Collections.Generic.IEnumerable{System.Linq.Expressions.Expression}) - ILLink IL2075 @@ -235,12 +139,6 @@ member M:System.Linq.Expressions.ExpressionStringBuilder.VisitExtension(System.Linq.Expressions.Expression) - - ILLink - IL2075 - member - M:System.Linq.Expressions.Interpreter.CallInstruction.GetArrayAccessor(System.Reflection.MethodInfo,System.Int32) - ILLink IL2075 @@ -289,12 +187,6 @@ member M:System.Linq.Expressions.Interpreter.InitializeLocalInstruction.MutableValue.Run(System.Linq.Expressions.Interpreter.InterpretedFrame) - - ILLink - IL2077 - member - M:System.Linq.Expressions.Interpreter.NullableMethodCallInstruction.GetValueOrDefault.Run(System.Linq.Expressions.Interpreter.InterpretedFrame) - ILLink IL2080 @@ -302,4 +194,4 @@ M:System.Linq.Expressions.Compiler.CompilerScope.ElementBoxStorage.#ctor(System.Linq.Expressions.Compiler.CompilerScope.Storage,System.Int32,System.Linq.Expressions.ParameterExpression) - \ No newline at end of file + diff --git a/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs b/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs index 8e2c32aeb3bc5c..646c79c85578f2 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics.CodeAnalysis; using System.Reflection; #if !FEATURE_DYNAMIC_DELEGATE @@ -27,7 +28,7 @@ internal static Delegate CreateObjectArrayDelegate(Type delegateType, Func s_thunks = new CacheDict(256); private static readonly MethodInfo s_FuncInvoke = typeof(Func).GetMethod("Invoke")!; - private static readonly MethodInfo s_ArrayEmpty = typeof(Array).GetMethod(nameof(Array.Empty))!.MakeGenericMethod(typeof(object)); + private static readonly MethodInfo s_ArrayEmpty = GetEmptyObjectArrayMethod(); private static readonly MethodInfo[] s_ActionThunks = GetActionThunks(); private static readonly MethodInfo[] s_FuncThunks = GetFuncThunks(); private static int s_ThunksCreated; @@ -62,6 +63,11 @@ public static TReturn FuncThunk2(Func handle return (TReturn)handler(new object?[]{t1, t2}); } + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2060:MakeGenericMethod", + Justification = "Calling Array.Empty() is safe since the T doesn't have ILLink annotations.")] + private static MethodInfo GetEmptyObjectArrayMethod() => + typeof(Array).GetMethod(nameof(Array.Empty))!.MakeGenericMethod(typeof(object)); + private static MethodInfo[] GetActionThunks() { Type delHelpers = typeof(DelegateHelpers); @@ -78,6 +84,8 @@ private static MethodInfo[] GetFuncThunks() delHelpers.GetMethod("FuncThunk2")!}; } + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2060:MakeGenericMethod", + Justification = "The above ActionThunk and FuncThunk methods don't have ILLink annotations.")] private static MethodInfo? GetCSharpThunk(Type returnType, bool hasReturnValue, ParameterInfo[] parameters) { try diff --git a/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/TypeUtils.cs b/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/TypeUtils.cs index c31dc3b7454d07..46a957ea89d58b 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/TypeUtils.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/TypeUtils.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Linq.Expressions; using System.Reflection; @@ -889,6 +890,8 @@ public static bool ValidateType(Type type, string? paramName, int index) return true; } + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", + Justification = "ILLinker will never remove the Invoke method from delegates.")] public static MethodInfo GetInvokeMethod(this Type delegateType) { Debug.Assert(typeof(Delegate).IsAssignableFrom(delegateType)); diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Compiler/AssemblyGen.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Compiler/AssemblyGen.cs index a0287218278353..5b197368e00a07 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Compiler/AssemblyGen.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Compiler/AssemblyGen.cs @@ -1,10 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics.CodeAnalysis; using System.Dynamic.Utils; using System.Reflection; using System.Reflection.Emit; -using System.Security; using System.Text; using System.Threading; @@ -38,7 +38,7 @@ private AssemblyGen() _myModule = myAssembly.DefineDynamicModule(name.Name!); } - private TypeBuilder DefineType(string name, Type parent, TypeAttributes attr) + private TypeBuilder DefineType(string name, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type parent, TypeAttributes attr) { ContractUtils.RequiresNotNull(name, nameof(name)); ContractUtils.RequiresNotNull(parent, nameof(parent)); diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Compiler/ILGen.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Compiler/ILGen.cs index 21e914fd7e72b7..e1bb52cb908e81 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Compiler/ILGen.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Compiler/ILGen.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Dynamic.Utils; using System.Reflection; using System.Reflection.Emit; @@ -484,7 +485,7 @@ private static bool TryEmitILConstant(this ILGenerator il, object value, Type ty if (TryEmitILConstant(il, value, nonNullType)) { - il.Emit(OpCodes.Newobj, type.GetConstructor(new[] { nonNullType })!); + il.Emit(OpCodes.Newobj, GetNullableConstructor(type, nonNullType)); return true; } @@ -825,7 +826,7 @@ private static void EmitNullableToNullableConversion(this ILGenerator il, Type t Type nnTypeTo = typeTo.GetNonNullableType(); il.EmitConvertToType(nnTypeFrom, nnTypeTo, isChecked, locals); // construct result type - ConstructorInfo ci = typeTo.GetConstructor(new Type[] { nnTypeTo })!; + ConstructorInfo ci = GetNullableConstructor(typeTo, nnTypeTo); il.Emit(OpCodes.Newobj, ci); labEnd = il.DefineLabel(); il.Emit(OpCodes.Br_S, labEnd); @@ -839,17 +840,26 @@ private static void EmitNullableToNullableConversion(this ILGenerator il, Type t il.MarkLabel(labEnd); } - private static void EmitNonNullableToNullableConversion(this ILGenerator il, Type typeFrom, Type typeTo, bool isChecked, ILocalCache locals) { Debug.Assert(!typeFrom.IsNullableType()); Debug.Assert(typeTo.IsNullableType()); Type nnTypeTo = typeTo.GetNonNullableType(); il.EmitConvertToType(typeFrom, nnTypeTo, isChecked, locals); - ConstructorInfo ci = typeTo.GetConstructor(new Type[] { nnTypeTo })!; + ConstructorInfo ci = GetNullableConstructor(typeTo, nnTypeTo); il.Emit(OpCodes.Newobj, ci); } + [DynamicDependency(DynamicallyAccessedMemberTypes.PublicConstructors, typeof(Nullable<>))] + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", + Justification = "The Nullable ctor will be preserved by the DynamicDependency.")] + private static ConstructorInfo GetNullableConstructor(Type nullableType, Type nonNullableType) + { + Debug.Assert(nullableType.IsNullableType()); + Debug.Assert(!nonNullableType.IsNullableType() && nonNullableType.IsValueType); + + return nullableType.GetConstructor(new Type[] { nonNullableType })!; + } private static void EmitNullableToNonNullableConversion(this ILGenerator il, Type typeFrom, Type typeTo, bool isChecked, ILocalCache locals) { @@ -899,26 +909,38 @@ private static void EmitNullableConversion(this ILGenerator il, Type typeFrom, T il.EmitNonNullableToNullableConversion(typeFrom, typeTo, isChecked, locals); } - + [DynamicDependency("get_HasValue", typeof(Nullable<>))] + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", + Justification = "The Nullable method will be preserved by the DynamicDependency.")] internal static void EmitHasValue(this ILGenerator il, Type nullableType) { + Debug.Assert(nullableType.IsNullableType()); + MethodInfo mi = nullableType.GetMethod("get_HasValue", BindingFlags.Instance | BindingFlags.Public)!; Debug.Assert(nullableType.IsValueType); il.Emit(OpCodes.Call, mi); } - + [DynamicDependency("get_Value", typeof(Nullable<>))] + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", + Justification = "The Nullable method will be preserved by the DynamicDependency.")] internal static void EmitGetValue(this ILGenerator il, Type nullableType) { + Debug.Assert(nullableType.IsNullableType()); + MethodInfo mi = nullableType.GetMethod("get_Value", BindingFlags.Instance | BindingFlags.Public)!; Debug.Assert(nullableType.IsValueType); il.Emit(OpCodes.Call, mi); } - + [DynamicDependency("GetValueOrDefault()", typeof(Nullable<>))] + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", + Justification = "The Nullable method will be preserved by the DynamicDependency.")] internal static void EmitGetValueOrDefault(this ILGenerator il, Type nullableType) { - MethodInfo mi = nullableType.GetMethod("GetValueOrDefault", System.Type.EmptyTypes)!; + Debug.Assert(nullableType.IsNullableType()); + + MethodInfo mi = nullableType.GetMethod("GetValueOrDefault", Type.EmptyTypes)!; Debug.Assert(nullableType.IsValueType); il.Emit(OpCodes.Call, mi); } @@ -965,6 +987,8 @@ internal static void EmitArray(this ILGenerator il, Type elementType, int count) /// The code assumes that bounds for all dimensions /// are already emitted. /// + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", + Justification = "The Array ctor is dynamically constructed and is not included in IL. It is not subject to trimming.")] internal static void EmitArray(this ILGenerator il, Type arrayType) { Debug.Assert(arrayType != null); diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/IndexExpression.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/IndexExpression.cs index 6d131fbfcc00c5..659418810676f9 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/IndexExpression.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/IndexExpression.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Dynamic.Utils; using System.Reflection; using System.Runtime.CompilerServices; @@ -222,7 +223,10 @@ public static IndexExpression Property(Expression instance, string propertyName, /// The method finds the instance property with the specified name in a type. The property's type signature needs to be compatible with /// the arguments if it is a indexer. If the arguments is null or empty, we get a normal property. /// - private static PropertyInfo FindInstanceProperty(Type type, string propertyName, Expression[]? arguments) + private static PropertyInfo FindInstanceProperty( + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.NonPublicProperties)] Type type, + string propertyName, + Expression[]? arguments) { // bind to public names first BindingFlags flags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.IgnoreCase | BindingFlags.FlattenHierarchy; @@ -262,7 +266,11 @@ private static string GetArgTypesString(Expression[] arguments) return argTypesStr.ToString(); } - private static PropertyInfo? FindProperty(Type type, string propertyName, Expression[]? arguments, BindingFlags flags) + private static PropertyInfo? FindProperty( + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.NonPublicProperties)] Type type, + string propertyName, + Expression[]? arguments, + BindingFlags flags) { PropertyInfo? property = null; diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/CallInstruction.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/CallInstruction.cs index c222f01151c40b..8e0c4bf32c0c25 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/CallInstruction.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/CallInstruction.cs @@ -122,6 +122,9 @@ public static CallInstruction Create(MethodInfo info, ParameterInfo[] parameters #endif } + [DynamicDependency("GetValue", typeof(Array))] + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern", + Justification = "The GetValue methods will be preserved by the DynamicDependency.")] private static CallInstruction GetArrayAccessor(MethodInfo info, int argumentCount) { Type arrayType = info.DeclaringType!; diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/TypeOperations.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/TypeOperations.cs index 96c851e9becf93..799f5617004f02 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/TypeOperations.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/TypeOperations.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Dynamic.Utils; using System.Reflection; using System.Runtime.CompilerServices; @@ -152,8 +153,12 @@ public GetValueOrDefault(MethodInfo mi) _defaultValueType = mi.ReturnType; } + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2077:UnrecognizedReflectionPattern", + Justification = "_defaultValueType is a ValueType, so it will always have a default constructor.")] public override int Run(InterpretedFrame frame) { + Debug.Assert(_defaultValueType.IsValueType, "Nullable only allows on value types."); + if (frame.Peek() == null) { frame.Pop(); diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberExpression.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberExpression.cs index 43594d96a74a8a..949b53aadaf0ea 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberExpression.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberExpression.cs @@ -173,7 +173,10 @@ public static MemberExpression Field(Expression expression, string fieldName) /// The containing the field. /// The field to be accessed. /// The created . - public static MemberExpression Field(Expression? expression, Type type, string fieldName) + public static MemberExpression Field( + Expression? expression, + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.NonPublicFields)] Type type, + string fieldName) { ContractUtils.RequiresNotNull(type, nameof(type)); ContractUtils.RequiresNotNull(fieldName, nameof(fieldName)); @@ -220,7 +223,10 @@ public static MemberExpression Property(Expression expression, string propertyNa /// The containing the property. /// The property to be accessed. /// The created . - public static MemberExpression Property(Expression? expression, Type type, string propertyName) + public static MemberExpression Property( + Expression? expression, + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties| DynamicallyAccessedMemberTypes.NonPublicProperties)] Type type, + string propertyName) { ContractUtils.RequiresNotNull(type, nameof(type)); ContractUtils.RequiresNotNull(propertyName, nameof(propertyName)); diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MethodCallExpression.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MethodCallExpression.cs index 05155a1646c8a4..248f63dba1d9ff 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MethodCallExpression.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MethodCallExpression.cs @@ -1162,7 +1162,11 @@ public static MethodCallExpression Call(Expression instance, string methodName, /// /// or is null. /// No method whose name is , whose type parameters match , and whose parameter types match is found in or its base types.-or-More than one method whose name is , whose type parameters match , and whose parameter types match is found in or its base types. - public static MethodCallExpression Call(Type type, string methodName, Type[]? typeArguments, params Expression[]? arguments) + public static MethodCallExpression Call( + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)] Type type, + string methodName, + Type[]? typeArguments, + params Expression[]? arguments) { ContractUtils.RequiresNotNull(type, nameof(type)); ContractUtils.RequiresNotNull(methodName, nameof(methodName)); @@ -1287,7 +1291,12 @@ private static bool TryQuote(Type parameterType, ref Expression argument) return ExpressionUtils.TryQuote(parameterType, ref argument); } - private static MethodInfo? FindMethod(Type type, string methodName, Type[]? typeArgs, Expression[] args, BindingFlags flags) + private static MethodInfo? FindMethod( + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)] Type type, + string methodName, + Type[]? typeArgs, + Expression[] args, + BindingFlags flags) { int count = 0; MethodInfo? method = null; @@ -1392,6 +1401,8 @@ public static MethodCallExpression ArrayIndex(Expression array, params Expressio /// or is null. /// /// .Type does not represent an array type.-or-The rank of .Type does not match the number of elements in .-or-The property of one or more elements of does not represent the type. + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern", + Justification = "The Array 'Get' method is dynamically constructed and are not included in IL. It is not subject to trimming.")] public static MethodCallExpression ArrayIndex(Expression array, IEnumerable indexes) { ExpressionUtils.RequiresCanRead(array, nameof(array), -1); diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/NewExpression.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/NewExpression.cs index 5587834c0b2beb..e6340fd888cf2f 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/NewExpression.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/NewExpression.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Dynamic.Utils; using System.Reflection; using System.Runtime.CompilerServices; @@ -178,7 +179,8 @@ public static NewExpression New(ConstructorInfo constructor, IEnumerable /// A that has a constructor that takes no arguments. /// A that has the property equal to and the property set to the that represents the parameterless constructor of the specified type. - public static NewExpression New(Type type) + public static NewExpression New( + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type type) { ContractUtils.RequiresNotNull(type, nameof(type)); if (type == typeof(void)) diff --git a/src/libraries/System.Linq.Expressions/src/System/Runtime/CompilerServices/CallSite.cs b/src/libraries/System.Linq.Expressions/src/System/Runtime/CompilerServices/CallSite.cs index a7c57130bd5a7e..a1715fd99157d7 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Runtime/CompilerServices/CallSite.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Runtime/CompilerServices/CallSite.cs @@ -280,7 +280,8 @@ internal void MoveRule(int i) } #if FEATURE_COMPILE - [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(UpdateDelegates))] + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2060:MakeGenericMethod", + Justification = "UpdateDelegates methods don't have ILLink annotations.")] #endif internal T MakeUpdateDelegate() { @@ -355,6 +356,8 @@ private static bool IsSimpleSignature(MethodInfo invoke, out Type[] sig) } #endif + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2060:MakeGenericMethod", + Justification = "CallSiteOps methods don't have ILLink annotations.")] private T CreateCustomUpdateDelegate(MethodInfo invoke) { Type returnType = invoke.GetReturnType(); diff --git a/src/libraries/System.Linq.Expressions/tests/System.Linq.Expressions.Tests.csproj b/src/libraries/System.Linq.Expressions/tests/System.Linq.Expressions.Tests.csproj index ea477dc9d04e57..b0a74eabbce8fa 100644 --- a/src/libraries/System.Linq.Expressions/tests/System.Linq.Expressions.Tests.csproj +++ b/src/libraries/System.Linq.Expressions/tests/System.Linq.Expressions.Tests.csproj @@ -180,6 +180,7 @@ + diff --git a/src/libraries/System.Linq.Expressions/tests/TrimCompatibilityTests.cs b/src/libraries/System.Linq.Expressions/tests/TrimCompatibilityTests.cs new file mode 100644 index 00000000000000..1c3061d2e3708b --- /dev/null +++ b/src/libraries/System.Linq.Expressions/tests/TrimCompatibilityTests.cs @@ -0,0 +1,43 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics.CodeAnalysis; +using System.Reflection; +using Xunit; + +namespace System.Linq.Expressions.Tests +{ + public static class TrimCompatibilityTests + { + /// + /// Verifies that the below Types don't have any DynamicallyAccessedMembers attributes, + /// so we can safely call MakeGenericMethod on their methods. + /// + [Fact] + public static void VerifyMethodsCalledWithMakeGenericMethod() + { + Assembly linqExpressions = typeof(Expression).Assembly; + Type[] types = new Type[] + { + linqExpressions.GetType("System.Dynamic.Utils.DelegateHelpers"), + linqExpressions.GetType("System.Dynamic.UpdateDelegates"), + linqExpressions.GetType("System.Runtime.CompilerServices.CallSiteOps"), + }; + + foreach (Type type in types) + { + foreach (MethodInfo method in type.GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static)) + { + Type[] genericTypes = method.GetGenericArguments(); + if (genericTypes != null) + { + foreach (Type genericType in genericTypes) + { + Assert.Null(genericType.GetCustomAttribute()); + } + } + } + } + } + } +} From 07c0fc14fbab9b092a2270083bd933cbee8b2248 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Fri, 29 Jan 2021 14:54:12 -0600 Subject: [PATCH 2/3] Address PR feedback --- .../Expressions/ExpressionResolverBuilder.cs | 2 +- .../src/System/Dynamic/Utils/DelegateHelpers.cs | 4 ++-- .../src/System/Dynamic/Utils/TypeUtils.cs | 2 +- .../Linq/Expressions/Interpreter/CallInstruction.cs | 9 +++------ .../Linq/Expressions/Interpreter/TypeOperations.cs | 9 +++++---- .../src/System/Runtime/CompilerServices/CallSite.cs | 2 +- 6 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/Expressions/ExpressionResolverBuilder.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/Expressions/ExpressionResolverBuilder.cs index e2878ee82a140e..865660e1f60142 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/Expressions/ExpressionResolverBuilder.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/Expressions/ExpressionResolverBuilder.cs @@ -157,7 +157,7 @@ protected override Expression VisitDisposeCache(ServiceCallSite callSite, object } [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2060:MakeGenericMethod", - Justification = "Calling Array.Empty() is safe since the T doesn't have ILLink annotations.")] + Justification = "Calling Array.Empty() is safe since the T doesn't have trimming annotations.")] internal static MethodInfo GetArrayEmptyMethodInfo(Type itemType) => ArrayEmptyMethodInfo.MakeGenericMethod(itemType); diff --git a/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs b/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs index 646c79c85578f2..4a204c0a74d79d 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs @@ -64,7 +64,7 @@ public static TReturn FuncThunk2(Func handle } [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2060:MakeGenericMethod", - Justification = "Calling Array.Empty() is safe since the T doesn't have ILLink annotations.")] + Justification = "Calling Array.Empty() is safe since the T doesn't have trimming annotations.")] private static MethodInfo GetEmptyObjectArrayMethod() => typeof(Array).GetMethod(nameof(Array.Empty))!.MakeGenericMethod(typeof(object)); @@ -85,7 +85,7 @@ private static MethodInfo[] GetFuncThunks() } [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2060:MakeGenericMethod", - Justification = "The above ActionThunk and FuncThunk methods don't have ILLink annotations.")] + Justification = "The above ActionThunk and FuncThunk methods don't have trimming annotations.")] private static MethodInfo? GetCSharpThunk(Type returnType, bool hasReturnValue, ParameterInfo[] parameters) { try diff --git a/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/TypeUtils.cs b/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/TypeUtils.cs index 46a957ea89d58b..e6cb75d8ba03c9 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/TypeUtils.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/TypeUtils.cs @@ -891,7 +891,7 @@ public static bool ValidateType(Type type, string? paramName, int index) } [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", - Justification = "ILLinker will never remove the Invoke method from delegates.")] + Justification = "The trimmer will never remove the Invoke method from delegates.")] public static MethodInfo GetInvokeMethod(this Type delegateType) { Debug.Assert(typeof(Delegate).IsAssignableFrom(delegateType)); diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/CallInstruction.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/CallInstruction.cs index 8e0c4bf32c0c25..103d1cfc5d2d92 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/CallInstruction.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/CallInstruction.cs @@ -122,9 +122,6 @@ public static CallInstruction Create(MethodInfo info, ParameterInfo[] parameters #endif } - [DynamicDependency("GetValue", typeof(Array))] - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern", - Justification = "The GetValue methods will be preserved by the DynamicDependency.")] private static CallInstruction GetArrayAccessor(MethodInfo info, int argumentCount) { Type arrayType = info.DeclaringType!; @@ -135,19 +132,19 @@ private static CallInstruction GetArrayAccessor(MethodInfo info, int argumentCou { case 1: alternativeMethod = isGetter ? - arrayType.GetMethod("GetValue", new[] { typeof(int) }) : + typeof(Array).GetMethod("GetValue", new[] { typeof(int) }) : typeof(CallInstruction).GetMethod(nameof(ArrayItemSetter1)); break; case 2: alternativeMethod = isGetter ? - arrayType.GetMethod("GetValue", new[] { typeof(int), typeof(int) }) : + typeof(Array).GetMethod("GetValue", new[] { typeof(int), typeof(int) }) : typeof(CallInstruction).GetMethod(nameof(ArrayItemSetter2)); break; case 3: alternativeMethod = isGetter ? - arrayType.GetMethod("GetValue", new[] { typeof(int), typeof(int), typeof(int) }) : + typeof(Array).GetMethod("GetValue", new[] { typeof(int), typeof(int), typeof(int) }) : typeof(CallInstruction).GetMethod(nameof(ArrayItemSetter3)); break; } diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/TypeOperations.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/TypeOperations.cs index 799f5617004f02..ef59a2d8eefc11 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/TypeOperations.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/TypeOperations.cs @@ -146,19 +146,20 @@ public override int Run(InterpretedFrame frame) private sealed class GetValueOrDefault : NullableMethodCallInstruction { + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] private readonly Type _defaultValueType; + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2074:UnrecognizedReflectionPattern", + Justification = "_defaultValueType is a ValueType, so you can always create an instance of it.")] public GetValueOrDefault(MethodInfo mi) { + Debug.Assert(mi.ReturnType.IsValueType, "Nullable only allows on value types."); + _defaultValueType = mi.ReturnType; } - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2077:UnrecognizedReflectionPattern", - Justification = "_defaultValueType is a ValueType, so it will always have a default constructor.")] public override int Run(InterpretedFrame frame) { - Debug.Assert(_defaultValueType.IsValueType, "Nullable only allows on value types."); - if (frame.Peek() == null) { frame.Pop(); diff --git a/src/libraries/System.Linq.Expressions/src/System/Runtime/CompilerServices/CallSite.cs b/src/libraries/System.Linq.Expressions/src/System/Runtime/CompilerServices/CallSite.cs index a1715fd99157d7..3421b85cd3410e 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Runtime/CompilerServices/CallSite.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Runtime/CompilerServices/CallSite.cs @@ -357,7 +357,7 @@ private static bool IsSimpleSignature(MethodInfo invoke, out Type[] sig) #endif [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2060:MakeGenericMethod", - Justification = "CallSiteOps methods don't have ILLink annotations.")] + Justification = "CallSiteOps methods don't have trimming annotations.")] private T CreateCustomUpdateDelegate(MethodInfo invoke) { Type returnType = invoke.GetReturnType(); From 370fb7abedebac5072c19466ca0fbcc058f58dfa Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Mon, 1 Feb 2021 14:34:40 -0600 Subject: [PATCH 3/3] PR feedback --- .../System/Linq/Expressions/Interpreter/TypeOperations.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/TypeOperations.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/TypeOperations.cs index ef59a2d8eefc11..3dbcade699b9b4 100644 --- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/TypeOperations.cs +++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/TypeOperations.cs @@ -146,18 +146,17 @@ public override int Run(InterpretedFrame frame) private sealed class GetValueOrDefault : NullableMethodCallInstruction { - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] private readonly Type _defaultValueType; - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2074:UnrecognizedReflectionPattern", - Justification = "_defaultValueType is a ValueType, so you can always create an instance of it.")] public GetValueOrDefault(MethodInfo mi) { - Debug.Assert(mi.ReturnType.IsValueType, "Nullable only allows on value types."); + Debug.Assert(mi.ReturnType.IsValueType, "Nullable is only allowed on value types."); _defaultValueType = mi.ReturnType; } + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2077:UnrecognizedReflectionPattern", + Justification = "_defaultValueType is a ValueType. You can always create an instance of a ValueType.")] public override int Run(InterpretedFrame frame) { if (frame.Peek() == null)