Skip to content

Commit

Permalink
Fix remaining ILLink warnings in System.Reflection (#45984)
Browse files Browse the repository at this point in the history
* Add RequiresUnreferencedCode to Delegate constructors

Contributes to #45623

* Add RequiresUnreferencedCode to some internal Reflection methods to address ILLink warnings.

* Address remaining ILLink warnings in Reflection.Emit.

* Respond to PR feedback

Annotate Delegate.BindToMethod with DynamicallyAccessedMembers

Fix up requires unreferenced code comments

* Update API Compat txt for Delegate.CreateDelegate attribute removal

* Fix Mono ILLink warnings
  • Loading branch information
eerhardt authored Dec 16, 2020
1 parent f272bc5 commit 3e1e1c1
Show file tree
Hide file tree
Showing 14 changed files with 86 additions and 152 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public abstract partial class Delegate : ICloneable, ISerializable

// This constructor is called from the class generated by the
// compiler generated code
[RequiresUnreferencedCode("The target method might be removed")]
protected Delegate(object target, string method)
{
if (target == null)
Expand All @@ -56,7 +57,7 @@ protected Delegate(object target, string method)
// This constructor is called from a class to generate a
// delegate based upon a static method name and the Type object
// for the class defining the method.
protected Delegate(Type target, string method)
protected Delegate([DynamicallyAccessedMembers(AllMethods)] Type target, string method)
{
if (target == null)
throw new ArgumentNullException(nameof(target));
Expand Down Expand Up @@ -257,8 +258,7 @@ protected virtual MethodInfo GetMethodImpl()
}

// V1 API.
[RequiresUnreferencedCode("The target method might be removed")]
public static Delegate? CreateDelegate(Type type, Type target, string method, bool ignoreCase, bool throwOnBindFailure)
public static Delegate? CreateDelegate(Type type, [DynamicallyAccessedMembers(AllMethods)] Type target, string method, bool ignoreCase, bool throwOnBindFailure)
{
if (type == null)
throw new ArgumentNullException(nameof(type));
Expand Down Expand Up @@ -417,10 +417,8 @@ internal static Delegate CreateDelegateNoSecurityCheck(Type type, object? target
//
// internal implementation details (FCALLS and utilities)
//

[RequiresUnreferencedCode("The target method might be removed")]
[MethodImpl(MethodImplOptions.InternalCall)]
private extern bool BindToMethodName(object? target, RuntimeType methodType, string method, DelegateBindingFlags flags);
private extern bool BindToMethodName(object? target, [DynamicallyAccessedMembers(AllMethods)] RuntimeType methodType, string method, DelegateBindingFlags flags);

[MethodImpl(MethodImplOptions.InternalCall)]
private extern bool BindToMethodInfo(object? target, IRuntimeMethodInfo method, RuntimeType methodType, DelegateBindingFlags flags);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@ public abstract class MulticastDelegate : Delegate
// This constructor is called from the class generated by the
// compiler generated code (This must match the constructor
// in Delegate
[RequiresUnreferencedCode("The target method might be removed")]
protected MulticastDelegate(object target, string method) : base(target, method)
{
}

// This constructor is called from a class to generate a
// delegate based upon a static method name and the Type object
// for the class defining the method.
protected MulticastDelegate(Type target, string method) : base(target, method)
protected MulticastDelegate([DynamicallyAccessedMembers(AllMethods)] Type target, string method) : base(target, method)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ internal ConstructorBuilder(string name, MethodAttributes attributes, CallingCon
}

internal ConstructorBuilder(string name, MethodAttributes attributes, CallingConventions callingConvention,
Type[]? parameterTypes, ModuleBuilder mod, TypeBuilder type) :
Type[]? parameterTypes, ModuleBuilder mod, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TypeBuilder type) :
this(name, attributes, callingConvention, parameterTypes, null, null, mod, type)
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,8 @@ public override Type MakeArrayType(int rank)

// Constructs a EnumBuilder.
// EnumBuilder can only be a top-level (not nested) enum type.
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2064:UnrecognizedReflectionPattern",
Justification = "Reflection.Emit is not subject to trimming")]
internal EnumBuilder(
string name, // name of type
Type underlyingType, // underlying type for an Enum
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,11 @@ internal SignatureHelper GetMemberRefSignature(CallingConventions call, Type? re
return sig;
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "Module.ResolveMethod is marked as RequiresUnreferencedCode because it relies on tokens " +
"which are not guaranteed to be stable across trimming. So if somebody hardcodes a token it could break. " +
"The usage here is not like that as all these tokens come from existing metadata loaded from some IL " +
"and so trimming has no effect (the tokens are read AFTER trimming occured).")]
private static MethodBase GetGenericMethodBaseDefinition(MethodBase methodBase)
{
// methodInfo = G<Foo>.M<Bar> ==> methDef = G<T>.M<S>
Expand Down Expand Up @@ -601,6 +606,7 @@ internal Type[] GetTypesNoLock()
}
}

[RequiresUnreferencedCode("Types might be removed")]
private Type? GetTypeNoLock(string className, bool throwOnError, bool ignoreCase)
{
// public API to to a type. The reason that we need this function override from module
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ public void Bake(ModuleBuilder module, int token)
#endregion

#region Public Static Methods
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2055:UnrecognizedReflectionPattern",
Justification = "MakeGenericType is only called on a TypeBuilder which is not subject to trimming")]
public static MethodInfo GetMethod(Type type, MethodInfo method)
{
if (!(type is TypeBuilder) && !(type is TypeBuilderInstantiation))
Expand Down Expand Up @@ -94,6 +96,9 @@ public static MethodInfo GetMethod(Type type, MethodInfo method)

return MethodOnTypeBuilderInstantiation.GetMethod(method, (type as TypeBuilderInstantiation)!);
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2055:UnrecognizedReflectionPattern",
Justification = "MakeGenericType is only called on a TypeBuilder which is not subject to trimming")]
public static ConstructorInfo GetConstructor(Type type, ConstructorInfo constructor)
{
if (!(type is TypeBuilder) && !(type is TypeBuilderInstantiation))
Expand All @@ -114,6 +119,9 @@ public static ConstructorInfo GetConstructor(Type type, ConstructorInfo construc

return ConstructorOnTypeBuilderInstantiation.GetConstructor(constructor, (type as TypeBuilderInstantiation)!);
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2055:UnrecognizedReflectionPattern",
Justification = "MakeGenericType is only called on a TypeBuilder which is not subject to trimming")]
public static FieldInfo GetField(Type type, FieldInfo field)
{
if (!(type is TypeBuilder) && !(type is TypeBuilderInstantiation))
Expand Down Expand Up @@ -1265,6 +1273,8 @@ public MethodBuilder DefineMethod(string name, MethodAttributes attributes, Call
}
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2082:UnrecognizedReflectionPattern",
Justification = "Reflection.Emit is not subject to trimming")]
private MethodBuilder DefineMethodNoLock(string name, MethodAttributes attributes, CallingConventions callingConvention,
Type? returnType, Type[]? returnTypeRequiredCustomModifiers, Type[]? returnTypeOptionalCustomModifiers,
Type[]? parameterTypes, Type[][]? parameterTypeRequiredCustomModifiers, Type[][]? parameterTypeOptionalCustomModifiers)
Expand Down Expand Up @@ -1344,6 +1354,8 @@ public MethodBuilder DefinePInvokeMethod(string name, string dllName, string ent
return method;
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2082:UnrecognizedReflectionPattern",
Justification = "Reflection.Emit is not subject to trimming")]
private MethodBuilder DefinePInvokeMethodHelper(
string name, string dllName, string importName, MethodAttributes attributes, CallingConventions callingConvention,
Type? returnType, Type[]? returnTypeRequiredCustomModifiers, Type[]? returnTypeOptionalCustomModifiers,
Expand Down Expand Up @@ -1459,6 +1471,8 @@ public ConstructorBuilder DefineTypeInitializer()
}
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2082:UnrecognizedReflectionPattern",
Justification = "Reflection.Emit is not subject to trimming")]
private ConstructorBuilder DefineTypeInitializerNoLock()
{
ThrowIfCreated();
Expand All @@ -1485,6 +1499,10 @@ public ConstructorBuilder DefineDefaultConstructor(MethodAttributes attributes)
}
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2055:UnrecognizedReflectionPattern",
Justification = "MakeGenericType is only called on a TypeBuilderInstantiation which is not subject to trimming")]
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern",
Justification = "GetConstructor is only called on a TypeBuilderInstantiation which is not subject to trimming")]
private ConstructorBuilder DefineDefaultConstructorNoLock(MethodAttributes attributes)
{
ConstructorBuilder constBuilder;
Expand Down Expand Up @@ -1558,6 +1576,8 @@ public ConstructorBuilder DefineConstructor(MethodAttributes attributes, Calling
}
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2082:UnrecognizedReflectionPattern",
Justification = "Reflection.Emit is not subject to trimming")]
private ConstructorBuilder DefineConstructorNoLock(MethodAttributes attributes, CallingConventions callingConvention,
Type[]? parameterTypes, Type[][]? requiredCustomModifiers, Type[][]? optionalCustomModifiers)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,7 @@ public sealed override Type[] GetForwardedTypes()
return types.ToArray();
}

[RequiresUnreferencedCode("Types might be removed because recursive nested types can't currently be annotated for dynamic access.")]
private static void AddPublicNestedTypes(Type type, List<Type> types, List<Exception> exceptions)
{
Type[] nestedTypes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ public override byte[] ResolveSignature(int metadataToken)
}
}

[RequiresUnreferencedCode("Trimming changes metadata tokens")]
private FieldInfo? ResolveLiteralField(int metadataToken, Type[]? genericTypeArguments, Type[]? genericMethodArguments)
{
MetadataToken tk = new MetadataToken(metadataToken);
Expand Down Expand Up @@ -353,13 +354,14 @@ public override void GetPEKind(out PortableExecutableKinds peKind, out ImageFile
#endregion

#region Protected Virtuals
[RequiresUnreferencedCode("Methods might be removed")]
[RequiresUnreferencedCode("Methods might be removed because Module methods can't currently be annotated for dynamic access.")]
protected override MethodInfo? GetMethodImpl(string name, BindingFlags bindingAttr, Binder? binder,
CallingConventions callConvention, Type[]? types, ParameterModifier[]? modifiers)
{
return GetMethodInternal(name, bindingAttr, binder, callConvention, types, modifiers);
}

[RequiresUnreferencedCode("Methods might be removed because Module methods can't currently be annotated for dynamic access.")]
internal MethodInfo? GetMethodInternal(string name, BindingFlags bindingAttr, Binder? binder,
CallingConventions callConvention, Type[]? types, ParameterModifier[]? modifiers)
{
Expand Down
Loading

0 comments on commit 3e1e1c1

Please sign in to comment.