(=) and (<>) throw NullReferenceExceptions when left operand is null

[simoncowen88]

(=) and (<>) operators handle left operand being null incorrectly - throwing a NullReferenceException.
This is particularly unexpected, and makes these operators non-reflexive.

Repro steps

type Foo = Foo
let nullFoo = Unchecked.defaultof<Foo>

Foo = nullFoo // false - as expected
nullFoo = Foo // throws a NullReferenceException

Foo <> nullFoo // true - as expected
nullFoo <> Foo // throws a NullReferenceException

Expected behavior

(=) and (<>) operators should be reflexive.

Actual behavior

When the left operand of / first argument to the operator is null, a NullReferenceException is thrown.

Known workarounds

System.Object.Equals has the expected behaviour as described above.

Related information

Provide any related information

• Windows 7
• .NET 4.6.1
• F# 4.0
• Severity : within F# this is a relatively minor issue as you are unlikely to encounter null instances, however in interop scenarios this is fairly painful and, I would argue, extremely unexpected behaviour.

[forki]

this sounds like we don't have property-based tests for this ;-(

[dsyme]

I think it's always been like this. TBH the use of Unchecked.default<_> on a type that doesn't support null as a normal value is just really not a good idea and the "Unchecked" in the name is real. BTW this issue is related #1044 and we can probably merge this with that

[simoncowen88]

@dsyme - sure, agree that I'd never expect anything from {{Unchecked.defaultof}} to be a sane instance, however just using to show the issue succinctly and create a null instance here. The main issue I suspect here is one of interop, particularly with C# where having null references floating around is fairly 'usual'.
I did have a search for related issue to this, however didn't recognise that issue as related.

[asik]

Seems to work fine with C# types:

let s:string = null;
s = "";

val s : string = null
val it : bool = false

let l:System.Collections.ArrayList = null
l = System.Collections.ArrayList();;

val l : System.Collections.ArrayList = null
val it : bool = false

I think what @dsyme is saying is that if you need to use Unchecked to repro the issue then it's not really an issue. If we get NullReferenceException in a "legit" case then that's an issue.

[matthid]

To be honest this is completely fine with the "fail-fast" principle. My first impression was that is should handle null just like you are suggesting. However than it will just fail at a later point as this type (from an F# compiler perspective) should never be null.

The only case which maybe could be allowed is that we don't fail when comparing with null, but you cannot even do that ;).

Triggering this means your interop layer didn't catch a null and converted it properly.

[simoncowen88]

I suppose it depends on how "legit" you consider null handling in F# code...
It appears certainly to be legitimate enough for isNull to have been implemented in the standard library - it doesn't seem unreasonable to expect equality checks to gracefully handle null, and even more so I think devs would tend to expect equality (when properly implemented - and this is completely out-of-the-box auto-implemented) to be reflexive. As mentioned System.Object.Equals appears to manage to do this 'as expected'.

Another case in which this can be an issue is from code interoperating with F#, for example:

type Foo = Foo with static member op_Equality ((a:Foo), (b:Foo)) = a = b

Which, again due to F#s questionable interop story (in this case for == and != from C#) seems like reasonable code to have written. Now, in C#, e.g. foo == null will throw instead of returning true. I would argue that all of the code written looks to be completely reasonable, and appears the 'obvious' code to write - however seems violates the principle of least surprise.

Perhaps these are unreasonable assumptions about how gracefully F# should interop / interact with nulls - however it feels like a very small change that would not only increase consistency (i.e. by making = actually reflexive) but would also significantly improve interop with other .Net languages where null references are more commonplace.

Also, to further add to the confusion, this works 'as expected' for 'a option - presumably due to the magic of None being null &c

[cartermp]

Closing this in favor of the Nullable Reference Types spec: #3108

This is, I'd argue, an expected oddity that if not changed in the core library, should be considered in nullability warnings with the feature.