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

Added a puzzler on extracting nulls as values in Maps #55

Closed
wants to merge 2 commits into from

Conversation

vdichev
Copy link

@vdichev vdichev commented Sep 10, 2012

No description provided.

@demobox
Copy link
Collaborator

demobox commented Sep 12, 2012

Hi Vassil!

Thanks for submitting! Apologies in advance if this review takes a little longer than usual - busy times.

If I am getting this correctly the puzzler essentially reduces to:

def intId(n: Int) = n
def anyId(a: Any) = a
val m = Map("key" -> null)
intId(m.asInstanceOf[Map[String, Int]]("key"))
res22: Int = 0
anyId(m.asInstanceOf[Map[String, Int]]("key")) // asInstanceOf not actually required here
res23: Any = null

In your example, the flatMap application is responsible for the cast. Are you sure autoboxing is required, by the way? In this reduced sample, simply casting to Int converts null to 0, as in e.g.

val n = null.asInstanceOf[Int]
n: Int = 0

@vdichev
Copy link
Author

vdichev commented Sep 12, 2012

Hi Andrew,

I see that you want to simplify the puzzle, so that readers can more easily understand it. You've also made me delve down deeper to understand the underlying issue. I think you've narrowed it down to the essential cause of the problem, although this does leave out some of the mystery of the puzzle.

The intent of the original code is to have values, which can be null or int, be converted to None or Some[Int], respectively. As you understand, it's common to have situations like this when you're interfacing with e.g. Java libraries. I also want to combine this with checking if the key is in the map at all- i.e. if there's no key, or the key is null, I want to get None, and if there's a key, I want Some[Int] instead of Some[Some[Int]].

Thus, converting null to 0 is not intended. Yes, null.asInstanceOf[Int] is 0, but this is explicit, whereas autoboxing is not. In this sense, I don't know what you mean by saying that autoboxing is required- it's unintended, which is the essence of the catch. Essentially, flatMap(Option.apply) compiles down to bytecode, which is also generated from the following Java code:

flatMap(new AbstractFunction1() {
    public final Option<Object> apply(Object paramObject) {
        return this.apply(BoxesRunTime.unboxToInt(paramObject));
    }
    public final Option<Object> apply(int arg1) {
        return Option$.apply(BoxesRunTime.boxToInteger(arg1));
    }
})

scala.runtime.BoxesRunTime.unboxToInt is defined as:

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

If we change the original example just slightly to cast to Map[String,Integer], then it works as intended, because Integer is allowed to be null. Int, on the other hand, is not. Or is it?

If we want to just get the Option indicating if the key is there or not:

map match { case m: Map[String,Int] => m.get("key") }
res2: Option[Int with Null] = Some(null)

So there's a weird unification type Int with Null, which you can't get explicitly.

If we just get the Option indicating if the value is null or not (without checking if the key is there):

map match { case m: Map[String,Int] => Option.apply(m("key")) }
res3: Option[Int with Null] = None

So how come Option.apply on the Int type, which cannot be null, result in None? Maybe that's the real puzzle? Because if we combine these two Options using flatMap, it behaves differently. Maybe I should put these two side by side?

map match { case m: Map[String,Int] => Option(m("key")) }
map match { case m: Map[String,Int] => m.get("key") flatMap Option.apply }

Do you think this explanation belongs in the puzzle? Do you think it qualifies as a puzzle, considering that there's a warning? Maybe code which results in warnings, should be the compiler's way to say, "expect weird corner-case behaviour". I'm not sure, but it was definitely surprising to me.

@demobox
Copy link
Collaborator

demobox commented Sep 12, 2012

Hi Vassil!

Many thanks for the detailed comment and explanation. I'll try to respond in more detail later. And yes, I certainly think there's a puzzler in there ;-)

@demobox
Copy link
Collaborator

demobox commented Sep 12, 2012

Thanks again for the additional research and comments. I certainly agree with you that the conversion/boxing of null to an Int is puzzling here, because it somehow seems to be about "when you access it". (In answer to your question, I would say Int is not "allowed to be null", but null can be converted/boxed to an Int.)

I think this is boxing at the "Scala level" (in the classes you pointed out) and not at the JVM level, though, so the explanation might have to be amended a bit. Would be curious for more expert input on this and the previous point.

Getting back to the puzzler, what do you think about the following sample:

val map = Map("key" -> null)
map match { case m: Map[String,Int] => m.get("key") }
map match { case m: Map[String,Int] => Option(m("key")) }
map match { case m: Map[String,Int] => val v = m("key"); Option(v) }

Or alternatively

val map = Map("key" -> null).asInstanceOf[Map[String, Int]]
map.get("key")
Option(map("key"))
val v = map("key")
Option(v)

? The former variant is probably more "pure" in that it avoids a cast, but it also generates compiler warnings and is more verbose. The latter version produces the same results without compiler warnings and is shorter.

Thoughts? @nermin ?

@vdichev
Copy link
Author

vdichev commented Sep 12, 2012

Of course, autoboxing is not a JVM feature, but one of the compiler. When Java 5 introduced boxing and unboxing, only the compiler was modified, but not the JVM- that would have probably broken backward compatibility. Today I checked the generated bytecode and the Java code I've pasted (which corresponds to the bytecode) shows clearly that this is something that the Scala compiler is doing. I wanted to update the description, but didn't get around to it today.

I'm now leaning towards the example using casting- it seems to me that warnings somehow show it's your fault. Besides, shorter code is cleaner- something which is especially important in figuring out the essence of the puzzler.

if what you expected was <code>None</code>.
</p>
<p>
So what is the explanation of this? Scala creates a function object as an argument to <code>flatMap</code> and it needs to unbox and box the parameter from <code>Integer</code> to <code>int</code> and back again. However, on the JVM unboxing <code>null</code> to an integral type(such as <code>byte</code>, <code>short</code>, <code>int</code> or <code>long</code>), results in 0, and Scala is also affected by this behaviour.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, Java implements unboxing of null by throwing a NullPointerException. I'm not sure which is more surprising.

@vdichev
Copy link
Author

vdichev commented Sep 13, 2012

Maybe we can make it even simpler (to the point where it doesn't look like the original one):

val someNull = Some(null).asInstanceOf[Option[Int]]
someNull: Option[Int] = Some(null)
someNull.get
res10: Int = 0
someNull == Some(null)
res11: Boolean = true
someNull == Some(0)
res12: Boolean = false
someNull filter (null!=)
res13: Option[Int] = None

This warning made me laugh:

someNull.get == null

warning: comparing values of types Int and Null using `==' will always yield false
       someNull.get == null
                    ^
res20: Boolean = true

OK, this has to stop somewhere :)

@demobox
Copy link
Collaborator

demobox commented Sep 13, 2012

Thanks for the many updates. I like the new example you suggest but I think using a Map is perhaps a little more "natural" than using an Option. As you pointed out in the original motivation for the puzzler, maps are a common type you'll get back from things like Java libraries, And trying to get something from a map and applying operations to it is more common than explicitly creating a null Option[Int].

So how about (a subset of) the following?

val map = Map("key" -> null).asInstanceOf[Map[String, Int]]
println(map.get("key") == Some(null)) // prints true
println(map("key") == null) // prints true
println(map("key") == 0) // prints true (puzzling?!)
println(map.get("key") == Some(0)) // prints false (puzzling?!)
println(map.get("key") filter (null==)) // prints Some(null)
println(map.get("key") filter (0==)) // prints Some(null) (puzzling?!)

And I agree, this has to stop somewhere (although I'm really enjoying playing with this one ;-) ). So last suggestion from me...

@vdichev
Copy link
Author

vdichev commented Sep 14, 2012

What I like about the existing set of puzzlers is that they demonstrate behaviour which might be surprising at first, but has reasons for it cited e.g. in the SLS. This behaviour is known and deliberately chosen. It is also stable, the lesson from the puzzler has lasting value.

It would be great if all examples follow the same guideline.

This means that bugs, for instance, might not be good candidates for puzzlers. What I'm unsure of is whether this is a bug or intended behaviour, or maybe just unspecified accidental behaviour, which might change in the future.

@demobox
Copy link
Collaborator

demobox commented Sep 14, 2012

Good point. From what I know this isn't a bug, although I'd like someone with a bit more inside knowledge to comment on that. And - sad as it is - I think we will indeed see some of the puzzlers (like puzzler 2) disappear as Scala changes. But then I guess the language is getting less puzzling, which must be a good thing ;-)

@dgruntz @dcsobral : any comments from you guys about whether this behaviour is in accordance with the SLS?

@dcsobral
Copy link
Contributor

Whenever you cast, you lie, and anything can come out of it.

Int cannot be null, and you'd never be able to put a null in a Map[X,Int] without resorting to casting.

If you do asInstanceOf, or if you do case x: T[A] (and ignore the warning), nothing from there on can be considered a puzzler.

@vdichev
Copy link
Author

vdichev commented Sep 14, 2012

Thanks for the insight Daniel. Yeah, that was my thinking as well- you're telling the compiler, "I know better, I take the consequences".

However, asInstanceOf is well defined in at least some cases in the spec- for instance, when you're casting a null value itself. So I was wondering- why stop there? For instance, the compiler manages to successfully box null to 0 with some of the methods.

Anyway, I think this is turning more into a discussion about defining the behaviour of the spec rather than explaining a known language feature :)

@demobox
Copy link
Collaborator

demobox commented Sep 14, 2012

Thanks for the comment, Daniel. I agree that casting is "evil" in that sense (and perhaps should trigger an automatic warning in any code analysis?), but seeing how common it is - e.g. when interacting with Java libraries, where I think Vassil's original example came from - I personally feel there is still value in pointing out some of the strange things that can happen.

If only as a way to say: THAT IS WHY YOU SHOULD NEVER CAST ;-) Which perhaps should go in the explanation - or the title.

@nermin
Copy link
Collaborator

nermin commented Sep 25, 2012

How about a little bit more realistic and somewhat simplified scenario:

def fromJava: java.lang.Integer = null

def double(m: Map[String, Int]) = m.get("key").map(_ * 2)

val map = Map("key" -> fromJava)

val result = double(map.asInstanceOf[Map[String, Int]])

result is Some[0] and I would expect either a null pointer exception or None.

@demobox
Copy link
Collaborator

demobox commented Sep 25, 2012

Hm. The tricky one here is that the map is a "regular" Scala map, not a Java map that is more likely to be returned in this scenario. So I wonder if there is some way to get fromJava to actually return a java.util.HashMap and somehoe use that in double. Because now what you're doing really is evil - casting a Scala map with a null to a type that doesn't allow nulls.

@nermin
Copy link
Collaborator

nermin commented Sep 27, 2012

Yes, it would look like this:

import scala.collection.JavaConverters._
def fromJava: java.util.Map[String, java.lang.Integer] = new java.util.HashMap[String, java.lang.Integer]() {
  {
    put("key", null)
  }
}
def double(m: scala.collection.Map[String, Int]) = m.get("key").map(_ * 2)
val result = double(fromJava.asScala.asInstanceOf[scala.collection.Map[String, Int]])

@demobox
Copy link
Collaborator

demobox commented Sep 29, 2012

So how about (sorry, being boring and ignoring the magic map "put" in the constructor there ;-) ):

import scala.collection.JavaConverters._
def fromJava: java.util.Map[String, java.lang.Integer] = { 
  val map = new java.util.HashMap[String, java.lang.Integer]()
  map.put("key", null)
  map
}
def incrNonnegatives(m: scala.collection.Map[String, Int]) = m.filter(_._2 >= 0).mapValues(_ + 1)
incrNonnegatives(fromJava.asScala.asInstanceOf[scala.collection.Map[String, Int]])

with options:

  1. throws NPE
  2. Map()
  3. throws ClassCastException
  4. Map(key -> 1) // correct

@nermin
Copy link
Collaborator

nermin commented Oct 1, 2012

Okay, let's finally close this one with that version. Option 4., though would have to be Map("key" -> 1).

@demobox
Copy link
Collaborator

demobox commented Oct 1, 2012

though would have to be Map("key" -> 1)

You would have thought so, eh? But the REPL really does omit the quotation marks:

scala> val m = Map("key" -> 1)
m: scala.collection.immutable.Map[java.lang.String,Int] = Map(key -> 1)

@vdichev
Copy link
Author

vdichev commented Oct 2, 2012

@demobox So in your answers you meant what it prints then (the result of toString).

In general (and to reply to Daniel), I would expect that there's some transitivity to asInstanceOf with type parameters. If the spec explicitly states that null.asInstanceOf[T] should be 0 for numeric types, then it would make sense that asInstanceOf[S[T]] should follow the same policy for numeric Ts. I understand this might be difficult to do in practice. So @nermin, given the spec, I'd say that getting null in some methods is the surprising behaviour, not the Some(0).

Anyway, I know this would further delay publishing of this puzzling puzzler, but shouldn't we just ask Paul Phillips on the mailing lists?

@demobox
Copy link
Collaborator

demobox commented Oct 2, 2012

@vdichev "So in your answers you meant what it prints then" Exactly. Simply because the intro question for each puzzler is "What is the result of executing the following code?"

And by all means please ask Paul to comment! After all, the puzzler won't "go mouldy" or anything like that ;-)

@paulp Care to comment, Paul? ;-) Any thoughts much appreciated!

@paulp
Copy link

paulp commented Oct 3, 2012

Can you phrase your question in the form of a question? I can't tell what is being asked.

@demobox
Copy link
Collaborator

demobox commented Oct 3, 2012

@paulp Sorry, Paul, I was jumping the gun a bit there - I was expecting Vassil to phrase the relevant question(s) first but you beat us to it ;-)

@vdichev all yours!

@vdichev
Copy link
Author

vdichev commented Oct 3, 2012

@paulp to summarize: since null.asInstanceOf[Int] is expected to be 0, how would you characterize this behavior:

val someNull = Some(null).asInstanceOf[Option[Int]]
someNull: Option[Int] = Some(null)
someNull.get
res10: Int = 0
someNull == Some(null)
res11: Boolean = true
someNull == Some(0)
res12: Boolean = false
someNull filter (null!=)
res13: Option[Int] = None

which is to say, sometimes it behaves as 0, sometimes as null.

The question is, is this a bug? If yes, is it a WONTFIX? If not, is it undefined & "don't care"? I mean, can we expect that this will behave the same way in the future (let's say, for a couple of major versions of Scala)?

@paulp
Copy link

paulp commented Oct 3, 2012

All of that looks "correct" to me. There is a Some holding null. You have lied to scala and claimed it is an Int in there. Everything which proceeds from that point is either the normal behavior of Some(null) or a consequence of the lie.

@vdichev
Copy link
Author

vdichev commented Oct 3, 2012

OK, "correct" means it's a real puzzler, not a bug in my book :) So which should be the final code? I would prefer to have both behavior as 0 and as null (which is more, ahem, puzzling), but shortness and clarity of the code is of highest priority.
Moral of the story: don't lie if you don't want to be lied to.

@demobox
Copy link
Collaborator

demobox commented Oct 3, 2012

@paulp Thanks, Paul!

@demobox
Copy link
Collaborator

demobox commented Oct 12, 2012

I like this variant. There really seems to be some shape-shifting here, but the Int with Null type does make it a little more complex...

import scala.collection.JavaConverters._
def fromJava: java.util.Map[String, java.lang.Integer] = {
  val map = new java.util.HashMap[String, java.lang.Integer]()
  map.put("key1", -1)
  map.put("key2", null)
  map.put("key3", 0)
  map
}
def incrNullAndNonneg(m: scala.collection.Map[String, Int with Null]) = m.filter(_._2 == 
null).filter(_._2 >= 0).mapValues(_ + 1)
incrNullAndNonneg(fromJava.asScala.asInstanceOf[scala.collection.Map[String, Int with 
Null]]) // prints Map(key2 -> 1)

@demobox
Copy link
Collaborator

demobox commented Oct 12, 2012

@dcsobral @paulp Curious to hear whether you feel this is the same case, because the type is now Int with Null. Or is java.lang.Integer -> Int with Null still considered a "lie"? ;-)

@demobox
Copy link
Collaborator

demobox commented Oct 12, 2012

@dgruntz Also curious to see what you think about this last example...

@dcsobral
Copy link
Contributor

Any use of asInstanceOf is a lie. The thing about a type such as Int with
Null is that it isn't about what is allowed, but what is required. It says
the type must be both Int and Null, which doesn't exist. We say such a type
is uninhabited.
Em 12/10/2012 06:55, "Andrew Phillips" notifications@github.com escreveu:

@dcsobral https://github.com/dcsobral @paulp https://github.com/paulpCurious to hear whether you feel this is the same case, because the type is
now Int with Null. Or is java.lang.Integer -> Int with Null still
considered a "lie"? ;-)


Reply to this email directly or view it on GitHubhttps://github.com//pull/55#issuecomment-9371608.

@demobox
Copy link
Collaborator

demobox commented Oct 12, 2012

It says the type must be both Int and Null, which doesn't exist

Err...of course. Doh! Thanks for pointing that out!

@vdichev
Copy link
Author

vdichev commented Oct 12, 2012

Yeah, I remember reading something like this: http://www.scala-lang.org/node/11165

You can get Int with String or String with Int e.g. for the parameter of a function when you unify the types of two functions:

List(_: Int => Unit,
     _: String => Unit)

BTW in the initial examples the type was also Int with Null (search in the comments above). Also, we've agreed that the type received using asInstanceOf is a lie, but this doesn't change the nature of the puzzle- it's unexpected, it's not a bug, and it's educational to developers.

@demobox
Copy link
Collaborator

demobox commented Nov 12, 2012

Puzzler #28: "Cast Away" added ;-) See the updated PR.

@demobox demobox closed this Nov 12, 2012
dgruntz pushed a commit to dgruntz/scalapuzzlers.github.com that referenced this pull request Jun 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants