-
Notifications
You must be signed in to change notification settings - Fork 509
Conversation
src/System.Private.Reflection.Core/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.cs
Outdated
Show resolved
Hide resolved
src/System.Private.Reflection.Core/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.cs
Outdated
Show resolved
Hide resolved
src/System.Private.Reflection.Core/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.cs
Outdated
Show resolved
Hide resolved
src/System.Private.Reflection.Core/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.cs
Outdated
Show resolved
Hide resolved
The original code also says
but VSD is always active for interfaces and impl appears to be non null? |
src/System.Private.Reflection.Core/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.cs
Outdated
Show resolved
Hide resolved
src/System.Private.Reflection.Core/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.cs
Outdated
Show resolved
Hide resolved
src/System.Private.Reflection.Core/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.cs
Outdated
Show resolved
Hide resolved
src/System.Private.Reflection.Core/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.cs
Outdated
Show resolved
Hide resolved
src/System.Private.Reflection.Core/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.cs
Outdated
Show resolved
Hide resolved
6c59a05
to
f3a822f
Compare
...on.Execution/src/Internal/Reflection/Execution/ExecutionEnvironmentImplementation.Runtime.cs
Outdated
Show resolved
Hide resolved
Ok, best of both worlds i hope. Not modifying existing code and no switches for now. |
throw new ArgumentException(SR.Arg_MustBeInterface); | ||
} | ||
|
||
foreach (RuntimeTypeHandle iface in RuntimeAugments.TryGetImplementedInterfaces(typeHandle)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RuntimeAugments.IsAssignableFrom might be better because it also covers variance:
interface IFoo<in T>
{
void Frob();
}
class Foo : IFoo<object>
{
public void Frob() { }
}
class Program
{
static void Main()
{
var m = typeof(Foo).GetInterfaceMap(typeof(IFoo<string>));
Console.WriteLine(m.InterfaceMethods[0].DeclaringType);
Console.WriteLine(m.TargetMethods[0].DeclaringType);
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the other hand, it bails on
interface IFoo<in T>
{
void Frob();
}
class Foo<T> : IFoo<object>
{
public void Frob() { }
}
class Program
{
static void Main()
{
var m = typeof(Foo<>).GetInterfaceMap(typeof(IFoo<string>));
Console.WriteLine(m.InterfaceMethods[0].DeclaringType);
Console.WriteLine(m.TargetMethods[0].DeclaringType);
}
}
due to
corert/src/Runtime.Base/src/System/Runtime/TypeCast.cs
Lines 493 to 497 in 0185f47
// Special case: Generic Type definitions are not assignable in a mrt sense | |
// in any way. Assignability of those types is handled by reflection logic. | |
// Call this case out first and here so that these only somewhat filled in | |
// types do not leak into the rest of the type casting logic. | |
if (pTargetType->IsGenericTypeDefinition || pSourceType->IsGenericTypeDefinition) |
while CoreCLR does not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add a comment that lists all known cases that CoreCLR handles, but this implementation does not?
src/System.Private.CoreLib/src/Internal/Runtime/CompilerServices/OpenMethodResolver.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Thank you @Suchiman! I didn't expect we would be able to get this far without extra tables generated by the compiler. |
Copy pasted from https://github.com/dotnet/runtime/blob/master/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs#L2609 and then made to work.