scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

Incorrect compiler warning about null comparisons with primitive types #602

Open scabug opened 16 years ago

scabug commented 16 years ago

The following code warns that the expression will always evaluate to false. The expression promptly proceeds to evaluate to true.

scala> val foo = new java.util.HashMap[Int, Int]
foo: java.util.HashMap[Int,Int] = {}

scala> foo.get(0) == null
<console>:6: warning: comparing values of types Int and Null using `==' will always yield false
       foo.get(0) == null
                  ^
res0: Boolean = true

This needs either fixed semantics or a fixed warning. I'd prefer the latter, as "fixing" the semantics would make Java collections very hard to use correctly on Scala primitives (I was very annoyed by the fact that I couldn't usefully use them on Scala primitives until I discovered the compiler was lying to me)

scabug commented 16 years ago

Imported From: https://issues.scala-lang.org/browse/SI-602?orig=1 Reporter: @DRMacIver

scabug commented 16 years ago

@mcdirmid said: Why not use scala.collection.jcl.HashMap, which is backed by a java.util.HashMap and contains lots of useful Scala functions. More to the point, get will return an option, and null is never used to indicate lack of a map binding, avoiding this problem altogether. Longer term, I think we are supposed to throw a runtime exception if null is unboxed to a primitive type, right?

scabug commented 16 years ago

@blair said: I disagree. I would rather fix the semantics.

The HashMap has to be parameterized with a reference type, so in the hash are java.lang.Integer's, but on the way back, I'm pretty sure the null is being passed through to unboxToInt in scala.runtime.BoxesRunTime:

public static int unboxToInt(Object i) {
    return i == null ? 0 : ((Integer)i).intValue();
}

I tested checking out trunk and changing that 0 to 123 and getting a null returned 123 :)

I think in this case you would want to Scala be smart enough not to take the Integer back to a primitive int, especially since it's being compared to a null.

The workaround is to explicitly use java.lang.Integer's, at least for the hash value:

scala> val foo = new java.util.HashMap[Int, Integer]
foo: java.util.HashMap[Int,Integer] = {}

scala> foo.get(0) == null
res0: Boolean = true

As an aside, I don't like the idea of null Int's, nor any of the other integer, boolean, floating poing types magically being converted to 0's. I think you just have to be aware of those conversions with nulls. I like the idea of having unboxToInt throwing an exception.

scabug commented 16 years ago

@DRMacIver said: Replying to [comment:1 mcdirmid]:

Why not use scala.collection.jcl.HashMap, which is backed by a java.util.HashMap and contains lots of useful Scala functions. More to the point, get will return an option, and null is never used to indicate lack of a map binding, avoiding this problem altogether. Longer term, I think we are supposed to throw a runtime exception if null is unboxed to a primitive type, right?

Because the actual example I need this for isn't a HashMap, it's an IdentityHashMap which doesn't have a wrapper. It just happened to be easier to use a HashMap for the example.

scabug commented 16 years ago

@DRMacIver said: Replying to [comment:2 blair]:

I disagree. I would rather fix the semantics.

I'm fine with fixing the semantics as long as doing so doesn't block the use case. But fixing the semantics and making a significant number of Java types unusable as a result is a Really Bad Idea.

The HashMap has to be parameterized with a reference type, so in the hash are java.lang.Integer's, but on the way back, I'm pretty sure the null is being passed through to unboxToInt in scala.runtime.BoxesRunTime:

public static int unboxToInt(Object i) {
    return i == null ? 0 : ((Integer)i).intValue();
}

I tested checking out trunk and changing that 0 to 123 and getting a null returned 123 :)

Sure.

I think in this case you would want to Scala be smart enough not to take the Integer back to a primitive int, especially since it's being compared to a null.

The workaround is to explicitly use java.lang.Integer's, at least for the hash value:

I tried exactly this. It generates a deprecation warning in the compiler.

scabug commented 16 years ago

@odersky said: I think this is for Gilles (to change the behavior of unboxToInt).

scabug commented 16 years ago

@dubochet said: Fixed in r14428.

scabug commented 16 years ago

@dubochet said: Fix caused #668 and is removed in r14432. Will be reapplied when #668 is fixed.

scabug commented 15 years ago

@odersky said: Milestone next_bugfix deleted

scabug commented 14 years ago

@paulp said: For those who may care: this patch could be reapplied, but it doesn't do any good anymore, and doesn't change the behavior of the reported example. Now that == involves all kinds of interesting logic possibilities there's no guarantee at all that unboxToWhatever is even called (and indeed in the above example, it isn't.)

scabug commented 8 years ago

@lrytz said: Also

scala> def f(x: Any) = "" + x
f: (x: Any)String

scala> f(null.asInstanceOf[Int])
res0: String = null

scala> f({ val a = null.asInstanceOf[Int]; a })
res1: String = 0