Skip to content

Commit

Permalink
Consider type declaration order in MethodImpls (#111998)
Browse files Browse the repository at this point in the history
This fixes the Loader\classloader\InterfaceFolding\Ambiguous test.

For the following program in pseudo-C# (test is in IL since this is not valid C#):

```csharp
B<int, int> b = new B<int, int>();
if (!(((I<int>)b).Foo() != "B::Foo2"))
{
    B<string, string> b2 = new B<string, string>();
    if (!(((I<string>)b2).Foo() != "B::Foo2"))
    {
        Console.WriteLine("Pass");
        return 100;
    }
}

Console.WriteLine("Failed!");
return -1;

interface I<T>
{
    string Foo();
}

class A<U> : I<U>
{
    string I<U>.Foo() => "A::Foo";
}

internal class B<V, W> : A<V>, I<W>
{
    string I<W>.Foo() => "B::Foo1";
    string I<V>.Foo() => "B::Foo2";
}

```

We incorrectly resolve the interface call into `B::Foo1` because `I<int>.Foo` could be implemented by both `Foo1` and `Foo2` methods and we just pick whatever we see first. Per ECMA-335: "If there are multiple methods for the same interface method (i.e. with different generic type parameters), place them in the list in type declaration order of the associated interfaces."

This implements the tie break.
  • Loading branch information
MichalStrehovsky authored Feb 6, 2025
1 parent 481eab6 commit adb2af2
Showing 1 changed file with 55 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -277,22 +277,22 @@ private static bool IsInterfaceImplementedOnType(MetadataType type, MetadataType

private static MethodDesc FindImplFromDeclFromMethodImpls(MetadataType type, MethodDesc decl)
{
if (decl.OwningType.IsInterface)
return FindInterfaceImplFromDeclFromMethodImpls(type, decl);

MethodImplRecord[] foundMethodImpls = type.FindMethodsImplWithMatchingDeclName(decl.Name);

if (foundMethodImpls == null)
return null;

bool interfaceDecl = decl.OwningType.IsInterface;

foreach (MethodImplRecord record in foundMethodImpls)
{
MethodDesc recordDecl = record.Decl;

if (interfaceDecl != recordDecl.OwningType.IsInterface)
if (recordDecl.OwningType.IsInterface)
continue;

if (!interfaceDecl)
recordDecl = FindSlotDefiningMethodForVirtualMethod(recordDecl);
recordDecl = FindSlotDefiningMethodForVirtualMethod(recordDecl);

if (recordDecl == decl)
{
Expand All @@ -303,6 +303,56 @@ private static MethodDesc FindImplFromDeclFromMethodImpls(MetadataType type, Met
return null;
}

private static MethodDesc FindInterfaceImplFromDeclFromMethodImpls(MetadataType type, MethodDesc decl)
{
Debug.Assert(decl.OwningType.IsInterface);

MethodImplRecord[] foundMethodImpls = type.FindMethodsImplWithMatchingDeclName(decl.Name);

if (foundMethodImpls == null)
return null;

// We might find more than one result due to generic parameter folding
var results = new ArrayBuilder<int>(1);
for (int i = 0; i < foundMethodImpls.Length; i++)
{
MethodDesc recordDecl = foundMethodImpls[i].Decl;
if (recordDecl == decl)
{
results.Add(i);
}
}

if (results.Count == 0)
return null;

int resultIndex = results[0];

// If we found multiple MethodImpls, need to do a tie break using type declaration order
if (results.Count > 1)
{
MetadataType typeDefinition = (MetadataType)type.GetTypeDefinition();
DefType[] interfacesOnDefinition = typeDefinition.RuntimeInterfaces;
MethodImplRecord[] foundMethodImplsOnDefinition = typeDefinition.FindMethodsImplWithMatchingDeclName(decl.Name);
Debug.Assert(foundMethodImplsOnDefinition.Length == foundMethodImpls.Length);

int bestInterfaceIndex = int.MaxValue;

for (int i = 0; i < results.Count; i++)
{
int index = Array.IndexOf(interfacesOnDefinition, foundMethodImplsOnDefinition[results[i]].Decl.OwningType);
Debug.Assert(index >= 0);
if (index < bestInterfaceIndex)
{
bestInterfaceIndex = index;
resultIndex = i;
}
}
}

return FindSlotDefiningMethodForVirtualMethod(foundMethodImpls[resultIndex].Body);
}

private static bool IsInterfaceExplicitlyImplementedOnType(MetadataType type, MetadataType interfaceType)
{
foreach (TypeDesc iface in type.ExplicitlyImplementedInterfaces)
Expand Down

0 comments on commit adb2af2

Please sign in to comment.