Stop dropping a cast, handle the missing cast in the bytecode assertion

Author: dwijnand

compiler/src/dotty/tools/dotc/transform/ArrayApply.scala

@@ -20,11 +20,6 @@ class ArrayApply extends MiniPhase {
 
   private val transformListApplyLimit = 8
 
-  override def transformTypeApply(tree: TypeApply)(using Context): Tree =
-    stripCast(tree) match
-      case app: Apply if isConsChain(app) => app
-      case _ => tree
-
   override def transformApply(tree: Apply)(using Context): Tree =
     if isArrayModuleApply(tree.symbol) then
       tree.args match
@@ -53,11 +48,6 @@ class ArrayApply extends MiniPhase {
 
     else tree
 
-  private def isConsChain(tree: Tree)(using Context): Boolean = tree match
-    case Apply(Select(New(tt), nme.CONSTRUCTOR), List(_, arg)) =>
-      tt.symbol == defn.ConsClass && (arg.symbol == defn.NilModule || isConsChain(arg))
-    case _ => false
-
   private def isArrayModuleApply(sym: Symbol)(using Context): Boolean =
     sym.name == nme.apply
     && (sym.owner == defn.ArrayModuleClass || (sym.owner == defn.IArrayModuleClass && !sym.is(Extension)))

compiler/test/dotty/tools/backend/jvm/ArrayApplyOptTest.scala

@@ -212,7 +212,24 @@ class ArrayApplyOptTest extends DottyBytecodeTest {
       val meth1   = getMethod(clsNode, "meth1")
       val meth2   = getMethod(clsNode, "meth2")
 
-      val instructions1 = instructionsFromMethod(meth1)
+      val instructions1 = instructionsFromMethod(meth1) match
+        case instr :+ TypeOp(CHECKCAST, _) :+ (ret @ Op(ARETURN)) =>
+          // List.apply[?A] doesn't, strictly, return List[?A],
+          // because it cascades to its definition on IterableFactory
+          // where it returns CC[A].  The erasure of that is Object,
+          // which is why Erasure's Typer adds a cast to compensate.
+          // If we drop that cast while optimising (because using
+          // the constructor for :: doesn't require the cast like
+          // List.apply did) then then cons construction chain will
+          // be typed as ::.
+          // Unfortunately the LUB of :: and Nil.type is Product
+          // instead of List, so a cast remains necessary,
+          // across whatever causes the lub, like `if` or `try` branches.
+          // Therefore if we dropping the cast may cause a needed cast
+          // to be necessary, we shouldn't drop the cast,
+          // which was only motivated by the assert here.
+          instr :+ ret
+        case instr => instr
       val instructions2 = instructionsFromMethod(meth2)
 
       assert(instructions1 == instructions2,

tests/run/list-apply-eval.scala

@@ -22,3 +22,7 @@ object Test:
 
     // just assert it doesn't throw CCE to List
     val queue = scala.collection.mutable.Queue[String]()
+
+  // test for the cast instruction described in checkApplyAvoidsIntermediateArray
+  def lub(b: Boolean): List[(String, String)] =
+    if b then List(("foo", "bar")) else Nil