[2.6.2] Incorrect comparison of null values of nullable structures when == is overridden

[JechoJekov]

The compiler does not compares correctly null values of nullable structures when the == operator is overridden.

Demo code below. When the structure overrides the == operator for non-nullable values only then the compiler fails to correctly compare two null values. If the == operator is overridden for both non-nullable and nullable values when the result is correct.

The output of the code below is:

b1 != b2
c1 == c2

Code:

[StructLayout(LayoutKind.Sequential, Size = 1)]
public struct B
{
    readonly int _value;

    public B(int value)
    {
        this._value = value;
    }

    public static bool operator ==(B a, B b)
    {
        return a._value == b._value;
    }

    public static bool operator !=(B a, B b)
    {
        return a._value != b._value;
    }

    public override bool Equals(object o)
    {
        return o is B && (B)o == this;
    }

    public override int GetHashCode()
    {
        return _value.GetHashCode();
    }
}

[StructLayout(LayoutKind.Sequential, Size = 1)]
public struct C
{
    readonly int _value;

    public C(int value)
    {
        this._value = value;
    }

    public static bool operator ==(C a, C b)
    {
        return a._value == b._value;
    }

    public static bool operator !=(C a, C b)
    {
        return a._value != b._value;
    }

    public static bool operator ==(C? a, C? b)
    {
        if (object.ReferenceEquals(a, b))
        {
            return true;
        }
        else if (object.ReferenceEquals(a, null) || object.ReferenceEquals(b, null))
        {
            return false;
        }
        else
        {
            return a.Value._value == b.Value._value;
        }
    }

    public static bool operator !=(C? a, C? b)
    {
        return false == (a == b);
    }

    public override bool Equals(object o)
    {
        return o is C && (C)o == this;
    }

    public override int GetHashCode()
    {
        return _value.GetHashCode();
    }
}

public static class Application
{
    public static void Main()
    {
        Console.WriteLine("Application started.");

        {
            B? b1 = null;
            B? b2 = null;

            if (b1 == b2)
            {
                Console.WriteLine("b1 == b2");
            }
            else
            {
                Console.WriteLine("b1 != b2");
            }
        }

        {
            C? c1 = null;
            C? c2 = null;

            if (c1 == c2)
            {
                Console.WriteLine("c1 == c2");
            }
            else
            {
                Console.WriteLine("c1 != c2");
            }
        }
    }
}

The code compiles to the following:

{
    var b1 = null;
    var b2 = null;
    if (ss.Nullable$1.lift($Test_App_B.op_Equality, b1, b2)) {
        console.log('b1 == b2');
    }
    else {
        console.log('b1 != b2');
    }
}
{
    var c1 = null;
    var c2 = null;
    if ($Test_App_C.op_Equality$1(c1, c2)) {
        console.log('c1 == c2');
    }
    else {
        console.log('c1 != c2');
    }
}

The issue is the ss.Nullable$1.lift method which returns "null" when any of the arguments is null. It seems its usage is inappropriate in this case.

ss_Nullable$1.lift = function Nullable$lift() {
    for (var i = 0; i < arguments.length; i++) {
        if (!ss.isValue(arguments[i]))
            return null;
    }
    return arguments[0].apply(null, Array.prototype.slice.call(arguments, 1));
};

[erik-kallen]

I guess it should return false instead of null for comparison operators, is that what you mean?

[JechoJekov]

This depends on the purpose of the function. If it is used only in the case described then it should check if all arguments are null and return "true" if so. Otherwise, if at least one argument is null then it should return "false". Otherwise, it should call the operator.

In my opinion it should also fail with an exception if the first argument is null (at least in the case described) since it is a function pointer. If compilation is correct though this should never happen so perhaps such a check is not required.

I can create a pull request if you confirm that this function is not used for anything else. It is not invoked anywhere in mscorlib so I assume it is used only by compiled code.

[erik-kallen]

The function is used to lift operators. It currently works as intended for non-comparison operators; any change would need to be made for comparison operators only, and for those I guess different operators have different rules for what should happen if one or both arguments is null.

[JechoJekov]

For comparison operators the rule is that if only one of the operands is null then the result is always false with the exception of the "!=" operator. If both operands are null then the result is always false with the exception of the "==" operator (note that >= and <= also return false when both values are null).

I assume that a different function should be used for comparison operators since it is not possible to detect the type of the operator in the current implementation.