This page is no longer maintained — Please continue to the home page at www.scala-lang.org

Re: [scala-reports] Re: [scala-bts] #1718: Matching on null for generic Array parameters is broken

No replies
DRMacIver
Joined: 2008-09-02,
User offline. Last seen 42 years 45 weeks ago.
So, this is easy enough to fix. The question is if it's worth fixing if arrays are changing in 2.8 (sorry my SIP for that has stalled. Between surprise performance issues, waiting on seeing what happens with @specialise and time spent on other things I've not been working on it much). A related question is whether 2.7.4 is at all likely to happen.

Essentially the problem is that the array boxing code looks like this:

              val elemClass = tree.tpe.typeArgs.head.typeSymbol;
              val boxedClass = if (isValueClass(elemClass)) boxedArrayClass(elemClass)
                               else BoxedObjectArrayClass;
              Apply(Select(New(TypeTree(boxedClass.tpe)), nme.CONSTRUCTOR), List(tree))

i.e. we're explicitly creating a new BoxedArray of the appropriate type to wrap the array. But this means we end up boxing null to something non-null. This is, in my opinion, clearly wrong.

The easiest solution is to simply replace this with a call to ScalaRuntime.boxArray (which should be updated to handle the null case anyway). This might come with a slight runtime cost as we're doing a bit more runtime type testing than we otherwise would, but isInstanceOf is pretty fast, it's of a type that's rather easy for hotspot to inline and we're doing an allocation at tha tpoint anyway so this is already not a super cheap operation, so I'd expect this to be a nearly unnoticeable cost. It would be strongly my inclination to go for this rather than to add a box method to each array class and select the appropriate one.

So, what's the thinking? Is this worth fixing, and if so any objections if I go ahead and do it?

Copyright © 2012 École Polytechnique Fédérale de Lausanne (EPFL), Lausanne, Switzerland