Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why doesn't the compiler at least warn on this == null

Tags:

Why does the C# compiler not even complain with a warning on this code? :

if (this == null) {    // ... } 

Obviously the condition will never be satisfied..

like image 720
Andrei Rînea Avatar asked Mar 17 '10 16:03

Andrei Rînea


2 Answers

Because you could override operator == to return true for that case.

public class Foo {     public void Test()     {         Console.WriteLine(this == null);     }      public static bool operator ==(Foo a, Foo b)     {         return true;     }      public static bool operator !=(Foo a, Foo b)     {         return true;     } } 

Running new Foo().Test() will print "True" to the console.

The other question here is: why doesn't the compiler issue a warning for ReferenceEquals(this, null)? From the bottom of the above link:

A common error in overloads of operator == is to use (a == b), (a == null), or (b == null) to check for reference equality. This instead results in a call to the overloaded operator ==, causing an infinite loop. Use ReferenceEquals or cast the type to Object, to avoid the loop.

That might be answered by @Aaronaught's response. And that's also why you should be doing (object)x == null or ReferenceEquals(x, null), not doing a simple x == null, when you're checking for null references. Unless, of course, you're sure that the == operator is not overloaded.

like image 97
Mark Rushakoff Avatar answered Nov 17 '22 15:11

Mark Rushakoff


Wow... I guess I was shamefully wrong

I disagree. I think you still make a good point.

The compiler knows whether the comparison is going to go to a user-defined comparison operator or not, and the compiler knows that if it does not, then 'this' is never null.

And in fact, the compiler does track whether a given expression can legally be null or not, in order to implement a minor optimization on non-virtual method calls. If you have a non-virtual method M and you say foo.M(); then the compiler generates this as "do a virtual call to M with receiver foo". Why? Because we want to throw if foo is null, and a virtual call always does a null check on the receiver. A non-virtual call does not; we'd have to generate it as "check foo for null, and then do a non-virtual call to M", which is longer, slower, more irritating code.

Now, if we can get away without doing the null check, we do. If you say this.M() or (new Foo()).M() then we do NOT generate a virtual call. We generate the non-virtual call without the null check because we know that it cannot be null.

So the compiler has excellent data on whether a particular comparison to null will sometimes, always, or never succeed.

The question then is "if the compiler knows that a particular comparison will never succeed, why not generate a warning for it?"

And the answer is "sometimes we do, and sometimes we don't".

We do in this situation:

int x = 123; if (x == null) ... 

There is an equality operator defined on two nullable ints. x is convertible to nullable int. null is convertible to nullable int. So that equality operator is valid, and therefore is used, and is of course always false. The compiler gives a warning that the expression is always false.

However, due to a bug we accidentally introduced in C# 3, this code does NOT produce that warning:

Guid x = whatever; if (x == null) ... 

Same deal. The nullable guid equality operator is valid, but the warning is suppressed. I'm not sure if we fixed this bug for C# 4 or not. If not, hopefully we'll get it into a service pack.

As for "if (this == null)" I don't know why we don't warn for that. It certainly seems like a good candidate for a warning. The most likely explanation is to follow this logical syllogism:

  • warnings are compiler features
  • compiler features must be (1) thought of, (2) designed, (3) implemented, (4) tested, (5) documented and (6) shipped to you before you can take advantage of the feature.
  • No one has done any of those six necessary things for this feature; we can't ship features that we never thought of in the first place.
  • therefore, no such feature.

There are infinitely many compiler features that we haven't thought of yet; we've implemented none of them.

Another reason to not give a warning here is that we try to give warnings on code which is both likely to be typed by accident and almost certainly wrong for a non-obvious reason. "this == null" is not likely to be typed in by accident, and though it is almost certainly wrong, as you note in the statement of your question, it is obviously wrong.

Compare that to our "guid == null" -- that is likely to happen by accident, because a developer might accidentally think that Guid is a reference type. Guids are usually passed around by reference in C++, so this is an easy mistake to make. The code is almost certainly wrong, but it is wrong in a non-obvious way. So this is a good candidate for a warning. (Which is why it is so unfortunate that this is a warning in C# 2 but not C# 3, due to a bug we introduced.)

like image 44
Eric Lippert Avatar answered Nov 17 '22 14:11

Eric Lippert