Delegate equality comparisons are very strange indeed

[jskeet]

Discovered during an ECMA meeting. The ECMA standard and the C# 5 specification disagree slightly at the moment, and Roslyn behaves differently to both of them. (The difference between the ECMA standard and the C# 5 spec is not exposed in the code here.)

Roslyn versions: 2.0.0.61213 and 1.3.1.60616; same behaviour

Sample code:

using System;

public class Delegates
{
    static void Main()
    {
        Action a1 = Main, a2 = Main;
        MulticastDelegate md1 = a1, md2 = a2;
        Delegate d1 = a1, d2 = a2;
        Object o1 = a1, o2 = a2;

        // Comparing with the same level in the type hierarchy...
        Console.WriteLine("a1 == a2: " + (a1 == a2));
        Console.WriteLine("md1 == md2: " + (md1 == md2));
        Console.WriteLine("d1 == d2: " + (d1 == d2));
        Console.WriteLine("o1 == o2: " + (o1 == o2));

        // Comparing one non-Action with Action
        Console.WriteLine("md1 == a2: " + (md1 == a2));
        Console.WriteLine("d1 == a2: " + (d1 == a2));
        Console.WriteLine("o1 == a2: " + (o1 == a2));

        // Comparing one non-MulticastDelegate with MulticastDelegate
        Console.WriteLine("d1 == md2: " + (d1 == md2));
        Console.WriteLine("o1 == md2: " + (o1 == md2));
    }   
}

Note that this code assumes that two separate delegate instances are created by the two method group conversions. This is currently required, but may not be in future versions, at which point all the comparisons would produce true and it wouldn't be an interesting example.

First problem: strange warnings

Compiler output: 5 warnings of "warning CS0252: Possible unintended reference comparison; to get a value comparison, cast the left hand side to type 'MulticastDelegate'", corresponding to the following comparisons:

• md1 == md2
• md1 == a2
• d1 == a2
• o1 == a2
• o1 == md2

Note that for two of these, the compile-time type of the LHS is already MulticastDelegate. Casting the LHS to MulticastDelegate neither removes the warning nor changes the execution-time behaviour.

Second problem: strange program results

Expected behaviour as per ECMA-334 / C# 5 specification: every line that doesn't use object as one of the operand types should end up calling ==(Delegate, Delegate) which compares the delegate type and the invocation list. That would lead to:

a1 == a2: True
md1 == md2: True
d1 == d2: True
o1 == o2: False
md1 == a2: True
d1 == a2: True
o1 == a2: False
d1 == md2: True
o1 == md2: False

Actual behaviour:

a1 == a2: True
md1 == md2: False // Different to expected
d1 == d2: True
o1 == o2: False
md1 == a2: False // Different to expected
d1 == a2: False // Different to expected
o1 == a2: False
d1 == md2: True
o1 == md2: False

[jskeet]

Update: it's even more surprising, as there's actually an ==(MulticastDelegate, MulticastDelegate) operator, but it's never called by the compiler in the above code... and shouldn't really need to exist, as ==(Delegate, Delegate) should behave the same way. (Indeed, both just call Equals after performing nullity checks.)

[jskeet]

Oh, and just to add dynamic typing into the mix:

dynamic dyn = a1;
Console.WriteLine("dyn == a2: " + (dyn == a2));
Console.WriteLine("dyn == md2: " + (dyn == md2));
Console.WriteLine("dyn == d2: " + (dyn == d2));
Console.WriteLine("dyn == o2: " + (dyn == o2));

Output:

dyn == a2: True
dyn == md2: True
dyn == d2: True
dyn == o2: False

So this behaves how we'd probably like it to behave, but it doesn't follow the normal "Resolve overloads as if the compile-time type is the actual execution-time type." Yay.

[jskeet]

More dynamic typing fun: the dynamic binding does enforce the restriction that you can't compare two different concrete delegate types:

using System;

class Program
{    
    delegate void D();
    delegate void E();
    static void Main(string[] args)
    {
        D d = Foo;
        E e = Foo;
        // Compile-time failure - reasonable
        // Console.WriteLine(d == e);
        dynamic d1 = d;
        dynamic d2 = e;
        // RuntimeBinderException - less reasonable
        Console.WriteLine(d1 == d2);
    }

    static void Foo() {}
}

I would propose that the restriction should only be enforced at genuine compile-time, rather than binding-time - so in this case, it would end up calling ==(Delegate,Delegate) and return false.

(The same restriction is also enforced for reference type equality, so it's reasonable to be consistent here, even though it's annoying.)

[poizan42]

Quote from MSDN:

X AVOID throwing exceptions from equality operators.

It seems wrong for Roslyn generated code to not follow the official guidelines even if it's not technically part of a spec.

[jskeet]

@poizan42: Do you mean in the dynamic case?

[poizan42]

@jskeet Yes where it throws RuntimeBinderException - the static case is just a compile time error, that's fine as there isn't any point in a comparison that is always false anyways.

[jskeet]

@poizan42: I'd say there's a design tension here. The normal behaviour of dynamic binding is that if there'd be a compile-time error, an exception is thrown instead. But I agree that it's unfortunate in this particular case, where the "error" could really have been a warning instead - and is in other cases, such as if ("" is int[]).

I don't think this should be changed for just delegates though - the same error occurs when comparing references of unrelated types with ==. The dynamic behaviour for == should be considered as one big lump, I'd say - I wouldn't want delegates to change without other reference types changing in the same way.

[jaredpar]

CC @MadsTorgersen as well for visibility.