Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect compiler warning about null comparisons with primitive types #602

Open
scabug opened this issue Mar 5, 2008 · 11 comments
Open

Incorrect compiler warning about null comparisons with primitive types #602

scabug opened this issue Mar 5, 2008 · 11 comments
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Mar 5, 2008

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
Copy link
Author

@scabug scabug commented Mar 5, 2008

@scabug
Copy link
Author

@scabug scabug commented Mar 5, 2008

@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
Copy link
Author

@scabug scabug commented Mar 5, 2008

@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
Copy link
Author

@scabug scabug commented Mar 6, 2008

@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
Copy link
Author

@scabug scabug commented Mar 6, 2008

@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
Copy link
Author

@scabug scabug commented Mar 18, 2008

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

@scabug
Copy link
Author

@scabug scabug commented Mar 20, 2008

@dubochet said:
Fixed in r14428.

@scabug
Copy link
Author

@scabug scabug commented Mar 21, 2008

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

@scabug
Copy link
Author

@scabug scabug commented Jan 14, 2009

@odersky said:
Milestone next_bugfix deleted

@scabug
Copy link
Author

@scabug scabug commented Nov 18, 2009

@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
Copy link
Author

@scabug scabug commented Apr 15, 2016

@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants