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

Ambiguous variant call resolution differs from .NET Framework #6312

Open
elasota opened this issue Dec 21, 2017 · 4 comments
Open

Ambiguous variant call resolution differs from .NET Framework #6312

elasota opened this issue Dec 21, 2017 · 4 comments

Comments

@elasota
Copy link

elasota commented Dec 21, 2017

When resolving ambiguous calls using variance, Mono and the .NET Framework have different resolution behavior.

The relevant section of the spec (II.12.2) is extremely confusing, and if I'm reading it correctly, both implementations are non-compliant for different reasons.

The algorithm that .NET appears to use when making a variant call is to look for an exact match of the interface at the each level of the class hierarchy, and if one exists, call using it. If that fails, it tries to find a variant match at that level of the class hierarchy, and if that fails too, it descends to the next class in the hierarchy in type declaration order.

An important thing is that an exact match won't be used if a variant match is found with a shallower search, which the spec has examples of. Mono doesn't appear to handle that correctly to begin with (i.e. in the example, removing IVar<B> from Ambiguous should cause it to use the IIfcA and IIfcB matches, because it can resolve those before IVar<B> is visible).

However, there's an additional quirk with how .NET does it: If an interface is reimplemented, then any methods that are the same as a previous implementation of the same interface will be skipped. In the example below, implementing IVar<B> in Ambiguous doesn't supply a new implementation of P, so it's ignored when searching for a method to call.

Steps to Reproduce

Compile and run this program:

using System;

namespace Tests
{
    public class TestInheritedImplementationDeprioritization
    {
        interface IIfcA { }
        interface IIfcB { }

        class A { }
        class B : A { }
        class C : B, IIfcA { }
        class D : B, IIfcB { }

        interface IVar<in T> { void P(T v, bool callB); }
        class S1
        {
            public virtual void P(B v, bool callB) { Console.WriteLine("Wrong (Fell through)"); }
            public virtual void P(A v, bool callB) { Console.WriteLine("Wrong (Called A)"); }
        }
        class S2 : S1, IVar<B>
        {
        }

        class Ambiguous : S2, IVar<B>, IVar<IIfcA>, IVar<IIfcB>
        {
            void IVar<IIfcA>.P(IIfcA v, bool callB)
            {
                Console.WriteLine(callB ? "OK" : "Wrong");
            }

            void IVar<IIfcB>.P(IIfcB v, bool callB)
            {
                Console.WriteLine(callB ? "Wrong" : "OK");
            }
        }

        public static void Main()
        {
            ((IVar<C>)new Ambiguous()).P(null, true);
            ((IVar<D>)new Ambiguous()).P(null, false);
        }
    }
}

Current Behavior

Prints "Wrong (Fell through)" twice unmodified
If IVar<B> is removed from Ambiguous, prints "Wrong (Fell through)" twice
If IVar<B> is removed from S2, prints "Wrong (Fell through)" twice
If IVar<B> is removed from S2 and Ambiguous, prints "OK" twice

Expected Behavior

Should print "OK" twice unmodified. (.NET Framework does this, but may be a spec violation?)
If IVar<B> is removed from Ambiguous, should print "OK" twice
If IVar<B> is removed from S2, should print "Wrong (Fell through)" twice
If IVar<B> is removed from S2 and Ambiguous, should print "OK" twice

On which platforms did you notice this

Windows

Version Used: 5.4.1

@lewurm
Copy link
Contributor

lewurm commented Dec 21, 2017

Is that related to https://bugzilla.xamarin.com/show_bug.cgi?id=59400 ?

/cc @alexischr

@marek-safar
Copy link
Member

@vargaz @lambdageek could you guys look into this issue

@elasota
Copy link
Author

elasota commented Dec 21, 2017

That bug looks like it's calling an implementation that isn't even a valid variant match, but this bug relates to the disambiguation behavior when there are multiple valid matches, so I think it's different.

Here's a more thorough test which should cover multiple cases. Mono fails 4 of them, and it also fails Case 5 in II.12.2.1 of the spec (it might fail others, but I didn't check).

using System;

namespace Tests
{
    public class TestInheritedImplementationDeprioritization
    {
        static void Check(string a, string b)
        {
            if (a == b)
                Console.WriteLine("OK");
            else
                Console.WriteLine("Failed: Expected " + b + ", but called " + a);
        }

        interface IIfcA { }
        interface IIfcB { }

        class A { }
        class B : A { }
        class C : B, IIfcA { }
        class D : B, IIfcB { }

        interface IVar<in T> { void P(T v, string expected); }
        class S1
        {
            public virtual void P(B v, string expected) { Check("S1.IVar<B>", expected); }
            public virtual void P(A v, string expected) { Check("S1.IVar<A>", expected); }
        }
        class S2 : S1, IVar<B>
        {
        }

        class Ambiguous : S2, IVar<B>, IVar<IIfcA>, IVar<IIfcB>
        {
            void IVar<IIfcA>.P(IIfcA v, string expected) { Check("Ambiguous.IVar<IIfcA>", expected); }
            void IVar<IIfcB>.P(IIfcB v, string expected) { Check("Ambiguous.IVar<IIfcB>", expected); }
        }

        class Ambiguous2 : S1, IVar<B>, IVar<IIfcA>, IVar<IIfcB>
        {
            void IVar<IIfcA>.P(IIfcA v, string expected) { Check("Ambiguous2.IVar<IIfcA>", expected); }
            void IVar<IIfcB>.P(IIfcB v, string expected) { Check("Ambiguous2.IVar<IIfcB>", expected); }
        }

        class Ambiguous3 : S2, IVar<IIfcA>, IVar<IIfcB>
        {
            void IVar<IIfcA>.P(IIfcA v, string expected) { Check("Ambiguous3.IVar<IIfcA>", expected); }
            void IVar<IIfcB>.P(IIfcB v, string expected) { Check("Ambiguous3.IVar<IIfcB>", expected); }
        }

        class Ambiguous4 : S2, IVar<B>, IVar<IIfcA>, IVar<IIfcB>
        {
            void IVar<B>.P(B v, string expected) { Check("Ambiguous4.IVar<B>", expected); }
            void IVar<IIfcA>.P(IIfcA v, string expected) { Check("Ambiguous4.IVar<IIfcA>", expected); }
            void IVar<IIfcB>.P(IIfcB v, string expected) { Check("Ambiguous4.IVar<IIfcB>", expected); }
        }

        public static void Main()
        {
            ((IVar<B>)new Ambiguous()).P(null, "S1.IVar<B>");
            ((IVar<C>)new Ambiguous()).P(null, "Ambiguous.IVar<IIfcA>");
            ((IVar<D>)new Ambiguous()).P(null, "Ambiguous.IVar<IIfcB>");

            ((IVar<B>)new Ambiguous2()).P(null, "S1.IVar<B>");
            ((IVar<C>)new Ambiguous2()).P(null, "S1.IVar<B>");
            ((IVar<D>)new Ambiguous2()).P(null, "S1.IVar<B>");

            ((IVar<B>)new Ambiguous3()).P(null, "S1.IVar<B>");
            ((IVar<C>)new Ambiguous3()).P(null, "Ambiguous3.IVar<IIfcA>");
            ((IVar<D>)new Ambiguous3()).P(null, "Ambiguous3.IVar<IIfcB>");

            ((IVar<B>)new Ambiguous4()).P(null, "Ambiguous4.IVar<B>");
            ((IVar<C>)new Ambiguous4()).P(null, "Ambiguous4.IVar<B>");
            ((IVar<D>)new Ambiguous4()).P(null, "Ambiguous4.IVar<B>");
        }
    }
}

@lambdageek lambdageek self-assigned this Dec 22, 2017
@lambdageek
Copy link
Member

Unassigning myself. I'm not going to be able to work on this in the near term. Thanks for the great examples @elasota!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants