Some C# methods that hide inherited methods with the same signature can't be called

degory commented 9 months ago

Problem

Attempting to call certain methods from assemblies built with C# can result in an "call is ambiguous" error.

Expected behavior

Given an assembly written in C# that exposes these types:

    public class Base {
        public virtual object? Method() {
            return null;
        }
    }

    // scenario 1
    public class OverrideSameReturnType: Base {
        // only this method is exposed via reflection so this scenario works correctly
        public override object? Method()
        {
            return null;
        }
    }

    // scenario 2
    public class NewVirtualSameReturnType: Base {
        // reflection exposes both this method and Base.Method() on this class
        public virtual new object? Method() {
            return null;
        }
    }

    // scenario 3
    public class OverrideCovariantReturnType: Base {
        // reflection exposes both this method and Base.Method() on this class
        public override string? Method()
        {
            return null;
        }
    }

    // scenario 4
    public class NewVirtualCovariantReturnType: Base {
        // reflection exposes both this method and Base.Method() on this class
        public virtual new string? Method() {
            return null;
        }
    }

Method() should be callable from ghūl on instances of all of the derived classes, without any "call is ambiguous" error.

Actual behaviour

Problem scenarios

There are four potentially problematic scenarios:

Calling an overriding method where the return type is identical to the return type of the method it overrides.
Calling a new method where the return type is identical to the return type of the method it hides
Calling an overriding method where the return type differs from the return type of the method it overrides
Calling a new method where the return type differs from the return type of the method it hides

Working scenario

Scenario 1 works as expected, because the .NET runtime exposes only the overridden method as a member of the derived class but with declaring type set to the base class. There is no ambiguity regarding which method to call, and the ghūl compiler knows to use the declaring class method signature when emitting the IL to invoke the method

Broken scenarios

Scenarios 2, 3 and 4 don't work because the .NET runtime exposes both the overridden or hiding method and the method from the parent type that it overrides or hides as duplicate members of the derived type.

Solution

For the problematic scenarios (2, 3, and 4) the ghūl compiler needs to:

[x] Determine whether any method signatures are ambiguous with any other methods, such that those methods can't unambiguously be called
[x] If any methods are ambiguous, choose a preferred method that will take precedence over the others
[x] Hide the other ambiguous methods
[x] When the preferred method is called, emit invoke IL referencing the correct declaring class

This is potentially complicated by scenarios 3 and 4 appearing identically in the IL when viewed from metadata reflection context (i.e. we can't tell if the method in the derived class is new or overrides). However, based on the code the C# compiler emits to invoke methods in scenarios 3 and 4, this doesn't actually matter - in both cases it calls methods through the derived class method signature (unlike scenario 1 where the base class method signature is required)

    // scenario 1
    callvirt   instance object Borken.Base::Method()

    // scenario 2
    callvirt   instance object Borken.NewVirtualSameReturnType::Method()

    // scenario 3
    callvirt   instance string Borken.OverrideCovariantReturnType::Method()

    // scenario 4
    callvirt   instance string Borken.NewVirtualCovariantReturnType::Method()

Determine whether any method signatures are ambiguous with any other methods

For each reflected method group, for each distinct number of arguments, for each method:

If the method doesn't have a method override group, create one
For each other method in the group with the same number of arguments
- If the other method doesn't have a method override group, create one for it
- Compare the methods' override groups: if they're equal then the methods are ambiguous

(this is essentially what the duplicate method checker already does for ghūl methods and functions, but we need to do it earlier, before we merge materialized method symbols into their owning symbols and cache the owning symbols)

Choose a preferred method from any ambiguous set

The C# compiler doesn't expose the original intent explicitly in a way that we can consume it. However it seems to be implicit that the correct method to pick is always the one in the more derived type.

Hide the other ambiguous methods

Once we've determined which method we prefer to expose, any other methods with ambiguous signatures in the same type need to be hidden. We can do this by simply not materializing symbol symbols for them.

When the preferred method is called, emit IL referencing the correct declaring class

It looks like we'll get this for free, as reflection will give the correct declaring class for the preferred method

As we retain reflected symbols across compiles, this shouldn't significantly impact the language extension's responsiveness.

degory commented 9 months ago

@quanglewangle this is the Npgsql "call is ambiguous" issue you're experiencing

quanglewangle commented 9 months ago

workaround using abstract classes works ok

degory / ghul