Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/8.0] Fix runtime dispatch to static virtuals on interface types #91440

Merged
merged 1 commit into from
Sep 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,8 @@ private static MethodDesc ResolveInterfaceMethodToVirtualMethodOnType(MethodDesc
{
Debug.Assert(!interfaceMethod.Signature.IsStatic);

// This would be a default interface method resolution. The algorithm below would sort of work, but doesn't handle
// things like diamond cases and it's better not to let it resolve as such.
if (currentType.IsInterface)
return null;

Expand Down Expand Up @@ -781,7 +783,7 @@ private static DefaultInterfaceMethodResolution ResolveInterfaceMethodToDefaultI
// If we're asking about an interface, include the interface in the list.
consideredInterfaces = new DefType[currentType.RuntimeInterfaces.Length + 1];
Array.Copy(currentType.RuntimeInterfaces, consideredInterfaces, currentType.RuntimeInterfaces.Length);
consideredInterfaces[consideredInterfaces.Length - 1] = (DefType)currentType.InstantiateAsOpen();
consideredInterfaces[consideredInterfaces.Length - 1] = currentType.IsGenericDefinition ? (DefType)currentType.InstantiateAsOpen() : currentType;
}

foreach (MetadataType runtimeInterface in consideredInterfaces)
Expand Down Expand Up @@ -921,6 +923,11 @@ public static IEnumerable<MethodDesc> EnumAllVirtualSlots(MetadataType type)
/// <returns>MethodDesc of the resolved virtual static method, null when not found (runtime lookup must be used)</returns>
public static MethodDesc ResolveInterfaceMethodToStaticVirtualMethodOnType(MethodDesc interfaceMethod, MetadataType currentType)
{
// This would be a default interface method resolution. The algorithm below would sort of work, but doesn't handle
// things like diamond cases and it's better not to let it resolve as such.
if (currentType.IsInterface)
return null;

// Search for match on a per-level in the type hierarchy
for (MetadataType typeToCheck = currentType; typeToCheck != null; typeToCheck = typeToCheck.MetadataBaseType)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,16 +372,8 @@ public sealed override IEnumerable<CombinedDependencyListEntry> GetConditionalSt

DefType defType = _type.GetClosestDefType();

// Interfaces don't have vtables and we don't need to track their slot use.
// The only exception are those interfaces that provide IDynamicInterfaceCastable implementations;
// those have slots and we dispatch on them.
bool needsDependenciesForVirtualMethodImpls = !defType.IsInterface
|| ((MetadataType)defType).IsDynamicInterfaceCastableImplementation();

// If we're producing a full vtable, none of the dependencies are conditional.
needsDependenciesForVirtualMethodImpls &= !factory.VTable(defType).HasFixedSlots;

if (needsDependenciesForVirtualMethodImpls)
if (!factory.VTable(defType).HasFixedSlots)
{
bool isNonInterfaceAbstractType = !defType.IsInterface && ((MetadataType)defType).IsAbstract;

Expand Down Expand Up @@ -436,6 +428,12 @@ public sealed override IEnumerable<CombinedDependencyListEntry> GetConditionalSt
((System.Collections.IStructuralEquatable)defType.RuntimeInterfaces).Equals(_type.RuntimeInterfaces,
EqualityComparer<DefType>.Default));

// Interfaces don't have vtables and we don't need to track their instance method slot use.
// The only exception are those interfaces that provide IDynamicInterfaceCastable implementations;
// those have slots and we dispatch on them.
bool needsDependenciesForInstanceInterfaceMethodImpls = !defType.IsInterface
|| ((MetadataType)defType).IsDynamicInterfaceCastableImplementation();

// Add conditional dependencies for interface methods the type implements. For example, if the type T implements
// interface IFoo which has a method M1, add a dependency on T.M1 dependent on IFoo.M1 being called, since it's
// possible for any IFoo object to actually be an instance of T.
Expand All @@ -456,6 +454,9 @@ public sealed override IEnumerable<CombinedDependencyListEntry> GetConditionalSt

bool isStaticInterfaceMethod = interfaceMethod.Signature.IsStatic;

if (!isStaticInterfaceMethod && !needsDependenciesForInstanceInterfaceMethodImpls)
continue;

MethodDesc implMethod = isStaticInterfaceMethod ?
defType.ResolveInterfaceMethodToStaticVirtualMethodOnType(interfaceMethod) :
defType.ResolveInterfaceMethodToVirtualMethodOnType(interfaceMethod);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public static bool MightHaveInterfaceDispatchMap(TypeDesc type, NodeFactory fact
if (!type.IsArray && !type.IsDefType)
return false;

// Interfaces don't have a dispatch map because we dispatch them based on the
// Interfaces don't have a dispatch map for instance methods because we dispatch them based on the
// dispatch map of the implementing class.
// The only exception are IDynamicInterfaceCastable scenarios that dispatch
// using the interface dispatch map.
Expand All @@ -83,8 +83,9 @@ public static bool MightHaveInterfaceDispatchMap(TypeDesc type, NodeFactory fact
// wasn't marked as [DynamicInterfaceCastableImplementation]" and "we couldn't find an
// implementation". We don't want to use the custom attribute for that at runtime because
// that's reflection and this should work without reflection.
if (type.IsInterface)
return ((MetadataType)type).IsDynamicInterfaceCastableImplementation();
bool isInterface = type.IsInterface;
if (isInterface && ((MetadataType)type).IsDynamicInterfaceCastableImplementation())
return true;

DefType declType = type.GetClosestDefType();

Expand Down Expand Up @@ -112,6 +113,11 @@ public static bool MightHaveInterfaceDispatchMap(TypeDesc type, NodeFactory fact

Debug.Assert(declMethod.IsVirtual);

// Only static methods get placed in dispatch maps of interface types (modulo
// IDynamicInterfaceCastable we already handled above).
if (isInterface && !declMethod.Signature.IsStatic)
continue;

if (interfaceOnDefinitionType != null)
declMethod = factory.TypeSystemContext.GetMethodForInstantiatedType(declMethod.GetTypicalMethodDefinition(), interfaceOnDefinitionType);

Expand Down Expand Up @@ -154,6 +160,10 @@ private void EmitDispatchMap(ref ObjectDataBuilder builder, NodeFactory factory)
var staticImplementations = new List<(int InterfaceIndex, int InterfaceMethodSlot, int ImplMethodSlot, int Context)>();
var staticDefaultImplementations = new List<(int InterfaceIndex, int InterfaceMethodSlot, int ImplMethodSlot, int Context)>();

bool isInterface = declType.IsInterface;
bool needsEntriesForInstanceInterfaceMethodImpls = !isInterface
|| ((MetadataType)declType).IsDynamicInterfaceCastableImplementation();

// Resolve all the interfaces, but only emit non-static and non-default implementations
for (int interfaceIndex = 0; interfaceIndex < declTypeRuntimeInterfaces.Length; interfaceIndex++)
{
Expand All @@ -166,6 +176,10 @@ private void EmitDispatchMap(ref ObjectDataBuilder builder, NodeFactory factory)
for (int interfaceMethodSlot = 0; interfaceMethodSlot < virtualSlots.Count; interfaceMethodSlot++)
{
MethodDesc declMethod = virtualSlots[interfaceMethodSlot];

if (!declMethod.Signature.IsStatic && !needsEntriesForInstanceInterfaceMethodImpls)
continue;

if(!interfaceType.IsTypeDefinition)
declMethod = factory.TypeSystemContext.GetMethodForInstantiatedType(declMethod.GetTypicalMethodDefinition(), (InstantiatedType)interfaceDefinitionType);

Expand Down Expand Up @@ -244,9 +258,17 @@ private void EmitDispatchMap(ref ObjectDataBuilder builder, NodeFactory factory)
// For default interface methods, the generic context is acquired by indexing
// into the interface list of the owning type.
Debug.Assert(providingInterfaceDefinitionType != null);
int indexOfInterface = Array.IndexOf(declTypeDefinitionRuntimeInterfaces, providingInterfaceDefinitionType);
Debug.Assert(indexOfInterface >= 0);
genericContext = StaticVirtualMethodContextSource.ContextFromFirstInterface + indexOfInterface;
if (declTypeDefinition.HasSameTypeDefinition(providingInterfaceDefinitionType) &&
providingInterfaceDefinitionType == declTypeDefinition.InstantiateAsOpen())
{
genericContext = StaticVirtualMethodContextSource.ContextFromThisClass;
}
else
{
int indexOfInterface = Array.IndexOf(declTypeDefinitionRuntimeInterfaces, providingInterfaceDefinitionType);
Debug.Assert(indexOfInterface >= 0);
genericContext = StaticVirtualMethodContextSource.ContextFromFirstInterface + indexOfInterface;
}
}
staticDefaultImplementations.Add((
interfaceIndex,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,17 +108,21 @@ public bool BuildSealedVTableSlots(NodeFactory factory, bool relocsOnly)

_sealedVTableEntries = new List<SealedVTableEntry>();

// Interfaces don't have any virtual slots with the exception of interfaces that provide
// Interfaces don't have any instance virtual slots with the exception of interfaces that provide
// IDynamicInterfaceCastable implementation.
// Normal interface don't need one because the dispatch is done at the class level.
// For IDynamicInterfaceCastable, we don't have an implementing class.
if (_type.IsInterface && !((MetadataType)_type).IsDynamicInterfaceCastableImplementation())
return true;
bool isInterface = declType.IsInterface;
bool needsEntriesForInstanceInterfaceMethodImpls = !isInterface
|| ((MetadataType)declType).IsDynamicInterfaceCastableImplementation();

IReadOnlyList<MethodDesc> virtualSlots = factory.VTable(declType).Slots;

for (int i = 0; i < virtualSlots.Count; i++)
{
if (!virtualSlots[i].Signature.IsStatic && !needsEntriesForInstanceInterfaceMethodImpls)
continue;

MethodDesc implMethod = declType.FindVirtualFunctionTargetMethodOnObjectType(virtualSlots[i]);

if (implMethod.CanMethodBeInSealedVTable())
Expand All @@ -143,6 +147,10 @@ public bool BuildSealedVTableSlots(NodeFactory factory, bool relocsOnly)
for (int interfaceMethodSlot = 0; interfaceMethodSlot < virtualSlots.Count; interfaceMethodSlot++)
{
MethodDesc declMethod = virtualSlots[interfaceMethodSlot];

if (!declMethod.Signature.IsStatic && !needsEntriesForInstanceInterfaceMethodImpls)
continue;

if (!interfaceType.IsTypeDefinition)
declMethod = factory.TypeSystemContext.GetMethodForInstantiatedType(declMethod.GetTypicalMethodDefinition(), (InstantiatedType)interfaceDefinitionType);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,8 @@ private static int GetNumberOfSlotsInCurrentType(NodeFactory factory, TypeDesc i
{
if (implType.IsInterface)
{
// We normally don't need to ask about vtable slots of interfaces. It's not wrong to ask
// that question, but we currently only ask it for IDynamicInterfaceCastable implementations.
Debug.Assert(((MetadataType)implType).IsDynamicInterfaceCastableImplementation());
// Interface types don't have physically assigned virtual slots, so the number of slots
// is always 0. They may have sealed slots.
return (implType.HasGenericDictionarySlot() && countDictionarySlots) ? 1 : 0;
}

Expand Down
73 changes: 73 additions & 0 deletions src/tests/nativeaot/SmokeTests/UnitTests/Interfaces.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ public static int Run()
TestMoreConstraints.Run();
TestSimpleNonGeneric.Run();
TestSimpleGeneric.Run();
TestDefaultDynamicStaticNonGeneric.Run();
TestDefaultDynamicStaticGeneric.Run();
TestDynamicStaticGenericVirtualMethods.Run();

return Pass;
Expand Down Expand Up @@ -1502,6 +1504,77 @@ public static void Run()
}
}

class TestDefaultDynamicStaticNonGeneric
{
interface IFoo
{
abstract static string ImHungryGiveMeCookie();
}

interface IBar : IFoo
{
static string IFoo.ImHungryGiveMeCookie() => "IBar";
}

class Baz : IBar
{
}

class Gen<T> where T : IFoo
{
public static string GrabCookie() => T.ImHungryGiveMeCookie();
}

public static void Run()
{
var r = (string)typeof(Gen<>).MakeGenericType(typeof(Baz)).GetMethod("GrabCookie").Invoke(null, Array.Empty<object>());
if (r != "IBar")
throw new Exception(r);

r = (string)typeof(Gen<>).MakeGenericType(typeof(IBar)).GetMethod("GrabCookie").Invoke(null, Array.Empty<object>());
if (r != "IBar")
throw new Exception(r);
}
}

class TestDefaultDynamicStaticGeneric
{
class Atom1 { }
class Atom2 { }

interface IFoo
{
abstract static string ImHungryGiveMeCookie();
}

interface IBar<T> : IFoo
{
static string IFoo.ImHungryGiveMeCookie() => $"IBar<{typeof(T).Name}>";
}

class Baz<T> : IBar<T>
{
}

class Gen<T> where T : IFoo
{
public static string GrabCookie() => T.ImHungryGiveMeCookie();
}

public static void Run()
{
Activator.CreateInstance(typeof(Baz<>).MakeGenericType(typeof(Atom1)));

var r = (string)typeof(Gen<>).MakeGenericType(typeof(Baz<>).MakeGenericType(typeof(Atom1))).GetMethod("GrabCookie").Invoke(null, Array.Empty<object>());
if (r != "IBar<Atom1>")
throw new Exception(r);

r = (string)typeof(Gen<>).MakeGenericType(typeof(IBar<>).MakeGenericType(typeof(Atom2))).GetMethod("GrabCookie").Invoke(null, Array.Empty<object>());
if (r != "IBar<Atom2>")
throw new Exception(r);
}
}

class TestDynamicStaticGenericVirtualMethods
{
interface IEntry
Expand Down
Loading