dotnet / csharpstandard

Working space for ECMA-TC49-TG2, the C# standard committee.
Creative Commons Attribution 4.0 International
693 stars 81 forks source link

C#6 §11.11.9 (Delegate equality operators) vs. delegate variance #663

Open KalleOlaviNiemitalo opened 1 year ago

KalleOlaviNiemitalo commented 1 year ago

ECMA-334:2022 §11.11.9 (Delegate equality operators) specifies:

If operator overload resolution resolves to either delegate equality operator, and the binding-time types of both operands are delegate types as described in §19 rather than System.Delegate, and there is no identity conversion between the binding-type operand types, a binding-time error occurs.

Note: This rule prevents comparisons which can never consider non-null values as equal due to being references to instances of different types of delegates. end note

Does that require a binding-time error in this case:

namespace DelegateVariance
{
    delegate void Dee<in T>(T value);

    class Program
    {
        static void Main()
        {
            Dee<object> d1 = Ignore;
            Dee<string> d2 = d1;

            System.Console.WriteLine(d1 == d2);
        }

        static void Ignore(object obj) {}
    }
}

If I understand correctly, the binding-time types are Dee\<object> and Dee\<string>, and there is no identity conversion between those, although there is an implicit reference conversion from Dee\<object> to Dee\. The standard would then seem to require a binding-time error, but:

(Inspired by https://github.com/dotnet/command-line-api/issues/1888.)

jskeet commented 1 year ago

Thanks, we'll discuss this. (I wonder whether the implicit conversion is being used as part of overload resolution, leading to a result of the delegate equality operator between two (effective) operands of type Dee<object>. But I haven't checked in detail yet.)

KalleOlaviNiemitalo commented 1 year ago

I thought this could be fixed by changing the wording to "and there is neither an identity conversion nor an implicit reference conversion between the binding-type operand types", but consider the case with two type parameters:


namespace DelegateVariance
{
    delegate void Dee2<in T1, in T2>(T1 value1, T2 value2);

    class Program
    {
        static void Main()
        {
            Dee2<object, object> d = Ignore;
            Dee2<object, string> d1 = d;
            Dee2<string, object> d2 = d;

            System.Console.WriteLine(d1 == d2);
        }

        static void Ignore(object obj1, object obj2) {}
    }
}

Here, there is no implicit reference conversion between the compile-time types of d1 and d2, but comparing them for equality can still be meaningful.

Surprisingly though, sharplab.io shows:

warning CS0252: Possible unintended reference comparison; to get a value comparison, cast the left hand side to type 'MulticastDelegate'

i.e. it did not use bool operator ==(System.Delegate x, System.Delegate y); from §11.11.9. I don't understand how this is justified by §11.4.5 (Binary operator overload resolution) and §11.6.4 (Overload resolution). Microsoft does define public static bool operator == (MulticastDelegate? d1, MulticastDelegate? d2);, which is not a predefined equality operator in §11.11.9; is it then considered a user-defined operator?

KalleOlaviNiemitalo commented 1 year ago

If I read section 11.11.7 correctly, there is a predefined operator public static bool operator == (MulticastDelegate d1, MulticastDelegate d2);, which compares by reference equality instead of looking at invocation lists; and likewise for each delegate type (declared with the delegate keyword). However, these predefined operators are never selected by overload resolution because MulticastDelegate has a user-defined equality operator.

KalleOlaviNiemitalo commented 1 year ago

If that is the intended interpretation, then the standard should specify the MulticastDelegate type so that implementations are not allowed to select the predefined equality comparison operators in overload resolution.

Nigel-Ecma commented 1 year ago

MulticastDelegate is intentionally in neither the CLI or C# Standards. It is a historical artefact from the MS CLI implementation and is not intended to be user-visible.

KalleOlaviNiemitalo commented 1 year ago

I now see 11.11.7 (Reference type equality operators) says "Every class type C", which excludes delegate types, but includes MulticastDelegate.

If an implementation interposes MulticastDelegate between the Delegate class and delegate types like .NET does, but does not define a user-defined operator ==(MulticastDelegate, MulticastDelegate), then 11.11.7 requires a predefined reference-equality operator ==(MulticastDelegate, MulticastDelegate), which can be selected by overload resolution on delegate types. Would such an implementation be conforming? If that is not intended, I think 11.11.7 should exclude all types derived from Delegate, although it wouldn't need to mention MulticastDelegate specifically.

jskeet commented 1 year ago

(We've failed to discuss this before.)

Aim for today's meeting, to pick between:

jskeet commented 1 year ago

Meeting notes: