SI-5353 unsound Array lubs.

[paulp]

mergePrefixAndArgs tried too hard to finesse the Array lub. When
there is more than one distinct Array element type being lubbed,
abandon the attempt to erase to any common Array type, because the
storage must match all the element types and Nothing will always
break this. Simply erase to AnyRef.

[paulp]

Opened against master because I'm not sure about the implications for compatibility. Inferring different lubs can lead to different method descriptors.

[paulp]

Review by @xeno-by, @retronym

[xeno-by]

Could you please comment on why the fix should target lub, but not Erasure (in unboundedGenericArrayLevel)? It seems useful to have lubs as precise as possible and downgrade to Objects only when necessary. This probably has some implications wrt type inference, which know almost nothing about, so I'm sorry if the question is being stupid.

[paulp]

It's possible you're right. I was already writing a mail asking about why Types knows anything about erasure at all. But the primary reason I pursued that location is that mergePrefixAndArgs is already intervening attempting to avoid exactly this problem.

[paulp]

But what is the case you have in mind where this commit leads to a less precise lub than is warranted?

[xeno-by]

No particular case, just trying to figure out how this should work. I don't know much about advanced type machinery.

[xeno-by]

Well, it could matter for macros of all sorts, including type tag materialization, but since the change only affects erasure, it doesn't matter.

[paulp]

You're right at least that this is insufficient; I can still induce CCEs.

[xeno-by]

Yeah, same here. E.g. see files/run/t5353.scala

[paulp]

How about now.

[xeno-by]

object Test extends App {
  def f(x: Boolean) = if (x) Array("abc") else Array()
  println(f(false).length)
}

uncaught exception during compilation: RuntimeException("scala.Array.length : Object, Object, Any") @ scala.sys.package$.error(package.scala:27)
error: java.lang.RuntimeException: scala.Array.length : Object, Object, Any
    at scala.sys.package$.error(package.scala:27)
    at scala.tools.nsc.backend.ScalaPrimitives$$anonfun$elementType$1$1$$anonfun$apply$1.apply(ScalaPrimitives.scala:571)
    at scala.tools.nsc.backend.ScalaPrimitives$$anonfun$elementType$1$1$$anonfun$apply$1.apply(ScalaPrimitives.scala:571)
    at scala.Option.getOrElse(Option.scala:120)
    at scala.tools.nsc.backend.ScalaPrimitives$$anonfun$elementType$1$1.apply(ScalaPrimitives.scala:571)
    at scala.tools.nsc.backend.ScalaPrimitives$$anonfun$elementType$1$1.apply(ScalaPrimitives.scala:567)
    at scala.reflect.internal.SymbolTable.enteringPhase(SymbolTable.scala:214)
    at scala.tools.nsc.Global.enteringTyper(Global.scala:1056)
    at scala.tools.nsc.backend.ScalaPrimitives.elementType$1(ScalaPrimitives.scala:567)
    at scala.tools.nsc.backend.ScalaPrimitives.getPrimitive(ScalaPrimitives.scala:607)
    at scala.tools.nsc.backend.icode.GenICode$ICodePhase.genPrimitiveOp(GenICode.scala:428)

[xeno-by]

Looks like the change has to be applied to unboundedGenericArrayLevel as that function is used elsewhere.

[paulp]

So I am discovering.

[paulp]

Now it's definitely overly broad, but at least maybe it will work. I'll refine it.

[xeno-by]

Looks like this change can be dropped now?

[paulp]

Well this is interestingly brutal to get right. Closing for now, will have to revisit.