Pattern matching creates array types that end up with needless indirection through ScalaRuntime

[retronym]

scala> class ArrayLength { def test[T](a: Array[T]) = a match { case x: Array[AnyRef] => x.length }}
defined class ArrayLength

scala> :javap -c ArrayLength#test
  public <T> int test(java.lang.Object);
    Code:
       0: aload_1
       1: astore_3
       2: aload_3
       3: instanceof    #18                 // class "[Ljava/lang/Object;"
       6: ifeq          27
       9: aload_3
      10: checkcast     #18                 // class "[Ljava/lang/Object;"
      13: astore        4
      15: getstatic     #24                 // Field scala/runtime/ScalaRunTime$.MODULE$:Lscala/runtime/ScalaRunTime$;
      18: aload         4
      20: invokevirtual #27                 // Method scala/runtime/ScalaRunTime$.array_length:(Ljava/lang/Object;)I
      23: istore_2
      24: goto          39
      27: goto          30
      30: new           #29                 // class scala/MatchError
      33: dup
      34: aload_3
      35: invokespecial #33                 // Method scala/MatchError."<init>":(Ljava/lang/Object;)V
      38: athrow
      39: iload_2
      40: ireturn

Workaround is to upcast:

scala> class ArrayLength1 { def test[T](a: Array[T]) = a match { case x: Array[AnyRef] => (x: Array[AnyRef]).length }}
defined class ArrayLength1

scala> :javap -c ArrayLength1#test
  public <T> int test(java.lang.Object);
    Code:
       0: aload_1
       1: astore_3
       2: aload_3
       3: instanceof    #18                 // class "[Ljava/lang/Object;"
       6: ifeq          22
       9: aload_3
      10: checkcast     #18                 // class "[Ljava/lang/Object;"
      13: astore        4
      15: aload         4
      17: arraylength
      18: istore_2
      19: goto          34
      22: goto          25
      25: new           #20                 // class scala/MatchError
      28: dup
      29: aload_3
      30: invokespecial #24                 // Method scala/MatchError."<init>":(Ljava/lang/Object;)V
      33: athrow
      34: iload_2
      35: ireturn

The proper fix would be in https://github.com/scala/scala/blob/179f9a97f29319b29f74a61e02d0a6e87114bac0/src/reflect/scala/reflect/internal/transform/Erasure.scala#L61

[dragos]

Unrelated, but since I noticed: the instruction at line 27 is also useless and should not be emitted.

[retronym]

@dragos That's the target of the conditional jump earlier on. It gets cleaned up with local optimizations:

$ scala -opt:l:method
Welcome to Scala 2.12.4 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_172).
Type in expressions for evaluation. Or try :help.

scala> class ArrayLength1 { def test[T](a: Array[T]) = a match { case x: Array[AnyRef] => (x: Array[AnyRef]).length }}
defined class ArrayLength1

scala> :javap -c ArrayLength1#test
  public <T> int test(java.lang.Object);
    Code:
       0: aload_1
       1: instanceof    #18                 // class "[Ljava/lang/Object;"
       4: ifeq          16
       7: aload_1
       8: checkcast     #18                 // class "[Ljava/lang/Object;"
      11: arraylength
      12: istore_2
      13: goto          25
      16: new           #20                 // class scala/MatchError
      19: dup
      20: aload_1
      21: invokespecial #24                 // Method scala/MatchError."<init>":(Ljava/lang/Object;)V
      24: athrow
      25: iload_2
      26: ireturn

But I think we can avoid creating the mess in the first place:

    def genBlock(tree: Block, expectedType: BType) {
      val Block(stats, expr) = tree
      val savedScope = varsInScope
      varsInScope = Nil
      stats foreach genStat
      genLoad(expr, expectedType)
      val end = currProgramPoint()
      if (emitVars) { // add entries to LocalVariableTable JVM attribute
        for ((sym, start) <- varsInScope.reverse) { emitLocalVarScope(sym, start, end) }
      }
      varsInScope = savedScope
    }

e.g. if the end Label was passed into genLoad(expr, expectedType), could we use that instead of generating a new label in, e.g.

    def genLoadIf(tree: If, expectedType: BType): BType = {
      val If(condp, thenp, elsep) = tree

      val success = new asm.Label
      val failure = new asm.Label

      val hasElse = !elsep.isEmpty
      val postIf  = if (hasElse) new asm.Label else failure

[dragos]

I agree it's nice to do it in an optimization phase (and not sure that it has an impact on performance), but it would be nice to avoid if not too hard. If I remember correctly, in the old code generator/optimizer this was done by a peephole optimization phase that ran even without -opt. It would clean up a few quirks that were hard to avoid while generating code (like push/pop and handling of Nothing types).