Should the "unbox" method for AnyVals match runtime unboxing behaviour?

[apwashere]

Apologies in advance if this should have gone to scala-internals instead

The AnyVal subclasses Byte, Int, Long etc.[1] define an "unbox" method as follows:

def unbox(x: java.lang.Object): _type_ = x.asInstanceOf[_java_type_]._type_Value()

This is generated by AnyVals:232 [2]. However, the runtime unboxing behaviour of these classes does not match (and is not implemented by) these unbox methods. The actual behaviour comes from BoxesRunTime [3] which, unlike the above pattern, does not throw a NullPointerException for null input. Its behaviour is instead something like

def unbox(x: java.lang.Object): _type_ = if (x == null) _type_zero_ else x.asInstanceOf[_java_type_]._type_Value()

SI-6710 already refers to the specific case of scala.Int.unbox, but before returning to that discussion I would like to ask whether the difference between the "indicated" behaviour of scala.X.unbox and runtime is intentional, in which case this wouldn't be an issue.

Thanks!

Andrew

[1] https://github.com/scala/scala/blob/master/src/library/scala/Byte.scala

https://github.com/scala/scala/blob/master/src/library/scala/Int.scala

...

[2] https://github.com/scala/scala/blob/master/src/compiler/scala/tools/cmd/gen/AnyVals.scala#L232

[3] https://github.com/scala/scala/blob/master/src/library/scala/runtime/BoxesRunTime.java#L87

[4] https://issues.scala-lang.org/browse/SI-6710 

[Paul Phillips]

On Tue, Nov 27, 2012 at 1:07 AM, apwashere <share...@gmail.com> wrote:

SI-6710 already refers to the specific case of scala.Int.unbox, but before returning to that discussion I would like to ask whether the difference between the "indicated" behaviour of scala.X.unbox and runtime is intentional, in which case this wouldn't be an issue.

It's never going to be intentional (if I'm involved anyway, which I am here) that a method is defined with source code but the compiler secretly injects an implementation which is inconsistent with the apparent source. It's fun to make things hard to follow and I know everyone enjoys hearing screams of anguish originating at other terminals, but in all things there are limits.

[apwashere]

It's fun to make things hard to follow and I know everyone enjoys hearing screams of anguish originating at other terminals, but in all things there are limits.

*grin*. Do you have any thoughts about whether this is something worth addressing at the present time, e.g. via an improvement or pull request? Or is this perhaps just not important enough now to merit changing a class with a potentially wide impact?

Regards

ap

[Paul Phillips]

On Thursday, November 29, 2012, apwashere wrote:

*grin*. Do you have any thoughts about whether this is something worth addressing at the present time, e.g. via an improvement or pull request? Or is this perhaps just not important enough now to merit changing a class with a potentially wide impact?

I may not have taken in the whole what-is-done-where matrix, but it was my impression the implementation generated as Int.unbox isn't ever called by generated code. It is a public method in the library so we do have to assume it's in use, but I doubt changing it would have a wide impact. That said, we would be far more likely to create trouble by changing it than we would by leaving it alone; both because "change" is more dangerous than "no change", and because the change in question takes a presently unexceptional case and makes it exceptional.

If we're not using it in generated code and don't intend to, the ideal change is probably to deprecate it and then remove it. Next best (and to do regardless) is to document the behavior and the fact that it isn't actually used by the compiler. There's no way which doesn't risk breakage to change the semantics of the method while leaving it in place.

[apwashere]

> Next best (and to do regardless) is to document the behavior and the fact that it isn't actually used by the compiler. 

See https://github.com/demobox/scala/commit/dd177a73bee86f86bdd66adda49baa26988c80a6 for one possible suggestion.

ap

[Andrew Phillips]

See https://issues.scala-lang.org/browse/SI-6898. It took more changes [1] than anticipated to make a seemingly small adjustment, but there's doubtless room for improvement.

Andrew

[1] https://github.com/demobox/scala/commit/719b86a3869f222bb37790faa4b6e9105cde4b1d