Skip to content
Merged
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Apply the same Type checks for Mono
GetConstructor and GetField had the same incorrect checks order
comparing to GetMethod as in the CoreCLR.
To keep the consistent solution, Mono has also been adjusted.
  • Loading branch information
BartoszKlonowski committed Jun 14, 2021
commit 3472615b40d26ef16039cf3d51a4fe7de8a46f6d
Original file line number Diff line number Diff line change
Expand Up @@ -1835,11 +1835,34 @@ public GenericTypeParameterBuilder[] DefineGenericParameters(params string[] nam
return generic_params;
}

private static bool IsValidGetMethodType(Type type)
Copy link
Member

@eerhardt eerhardt Jul 14, 2021

Choose a reason for hiding this comment

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

Can you put this method back where it was originally declared? That will make reviewing the change much easier. Also when people look at source history, will be able to see what change was made here easily. #Closed

{
if (type is TypeBuilder || type is TypeBuilderInstantiation)
return true;
/*GetMethod() must work with TypeBuilders after CreateType() was called.*/
if (type.Module is ModuleBuilder)
return true;
if (type.IsGenericParameter)
return false;

Type[] inst = type.GetGenericArguments();
if (inst == null)
return false;
for (int i = 0; i < inst.Length; ++i)
{
if (IsValidGetMethodType(inst[i]))
return true;
}
return false;
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern",
Justification = "Linker thinks Type.GetConstructor(ConstructorInfo) is one of the public APIs because it doesn't analyze method signatures. We already have ConstructorInfo.")]
public static ConstructorInfo GetConstructor(Type type, ConstructorInfo constructor)
{
/*FIXME I would expect the same checks of GetMethod here*/
if (!IsValidGetMethodType(type))
throw new ArgumentException("type is not TypeBuilder but " + type.GetType(), nameof(type));

if (type == null)
throw new ArgumentException("Type is not generic", nameof(type));

Copy link
Member

Choose a reason for hiding this comment

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

I think this needs the same code that GetMethod has:

            if (type is TypeBuilder && type.ContainsGenericParameters)
                type = type.MakeGenericType(type.GetGenericArguments());

(and same for GetField)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've tried, but it won't build.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like you also need

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2055:UnrecognizedReflectionPattern",
            Justification = "Type.MakeGenericType is used to create a typical instantiation")]

Expand All @@ -1849,13 +1872,12 @@ public static ConstructorInfo GetConstructor(Type type, ConstructorInfo construc
if (type.IsGenericTypeDefinition)
throw new ArgumentException("Type cannot be a generic type definition", nameof(type));

if (constructor == null)
throw new NullReferenceException(); //MS raises this instead of an ArgumentNullException

if (!constructor.DeclaringType!.IsGenericTypeDefinition)
throw new ArgumentException("constructor declaring type is not a generic type definition", nameof(constructor));
if (constructor.DeclaringType != type.GetGenericTypeDefinition())
throw new ArgumentException("constructor declaring type is not the generic type definition of type", nameof(constructor));
if (constructor == null)
throw new NullReferenceException(); //MS raises this instead of an ArgumentNullException

ConstructorInfo res = type.GetConstructor(constructor);
if (res == null)
Expand All @@ -1864,27 +1886,6 @@ public static ConstructorInfo GetConstructor(Type type, ConstructorInfo construc
return res;
}

private static bool IsValidGetMethodType(Type type)
{
if (type is TypeBuilder || type is TypeBuilderInstantiation)
return true;
/*GetMethod() must work with TypeBuilders after CreateType() was called.*/
if (type.Module is ModuleBuilder)
return true;
if (type.IsGenericParameter)
return false;

Type[] inst = type.GetGenericArguments();
if (inst == null)
return false;
for (int i = 0; i < inst.Length; ++i)
{
if (IsValidGetMethodType(inst[i]))
return true;
}
return false;
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2055:UnrecognizedReflectionPattern",
Justification = "Type.MakeGenericType is used to create a typical instantiation")]
public static MethodInfo GetMethod(Type type, MethodInfo method)
Expand Down Expand Up @@ -1914,6 +1915,9 @@ public static MethodInfo GetMethod(Type type, MethodInfo method)

public static FieldInfo GetField(Type type, FieldInfo field)
{
if (!IsValidGetMethodType(type))
throw new ArgumentException("type is not TypeBuilder but " + type.GetType(), nameof(type));

if (!type.IsGenericType)
throw new ArgumentException("Type is not a generic type", nameof(type));

Expand Down