Fix #854: Optimize matches on primitive constants as switches. #1061
Conversation
|
This is Work-in-Progress, but close enough to reality for a request for comments. There is one failing test on my machine, t3833.scala, which looks like this: package hello
object world {
def mkArray[T <: A](atype: Int) :T#AType = {
(atype match {
case 1 =>
new Array[Int](10)
// Decompiled code: return (Object[])new int[10];
case 2 =>
new Array[Float](10)
}).asInstanceOf[T#AType]
}
def main(args: Array[String]): Unit = {
println(mkArray[I](1))
//java.lang.ClassCastException: [I cannot be cast to [Ljava.lang.Object;
}
}
trait A {
type AType <: AnyRef
}
trait I extends A {
type AType = Array[Int]
}
trait F extends A {
type AType = Array[Float]
}The tree emitted after [error] def mkArray[T <: hello.A](atype: Int): T#AType = {
[error] (atype match {
[error] case 1 =>
[error] dotty.runtime.Arrays.newIntArray(10)
[error] case 2 =>
[error] dotty.runtime.Arrays.newFloatArray(10)
[error] }).asInstanceOf[T#AType]
[error] }however, it causes a crash in GenBCode: I fear there might be a problem in Erasure; maybe it does not recognize Does anyone have an idea, off the top of your head? |
| CaseDef(pat, EmptyTree, genBody(body)) | ||
| } | ||
|
|
||
| val wildcard = ctx.typeAssigner.assignType(untpd.Ident(nme.WILDCARD), defn.IntType) |
There was a problem hiding this comment.
This is most probably a code smell. I haven't found a better way to generate a typed Ident(WILDCARD), which is apparently what the back-end wants. Should there be a better way?
|
Further progress on this is blocked by #1065. |
|
Rebased on top of master, with the fix for #1065. |
| // case 5 | 6 => | ||
| case List(AlternativesTreeMaker(_, alts, _), body: BodyTreeMaker) => | ||
| val intValues = alts.map { | ||
| case List(IntEqualityTestTreeMaker(intValue)) => intValue |
There was a problem hiding this comment.
This line causes a bootstrapping test (I think) to fail with
[error] java.lang.AssertionError: assertion failed: error at /home/jenkins/workspace/dotty-master-validate-partest/tests/partest-generated/pos/transform/PatternMatcher.scala:340
[error] type mismatch:
[error] found : $this.EqualityTestTreeMaker
[error] required: TreeMakers.this.EqualityTestTreeMaker
[error] tree = TypeApply(Select(Ident(p38),asInstanceOf),List(TypeTree[TypeRef(TermRef(NoPrefix,$this),EqualityTestTreeMaker)]))
[error] at scala.Predef$.assert(Predef.scala:165)
[error] at dotty.tools.dotc.transform.TreeChecker$Checker.adapt(TreeChecker.scala:333)
[error] at dotty.tools.dotc.typer.ProtoTypes$FunProto.typedArg(ProtoTypes.scala:205)
It seems to be a Ycheck error. Apparently it cannot relate the synthetic $this thing (a capture?) to the enclosing TreeMakers.this.
Not sure what to make of this, atm.
There was a problem hiding this comment.
AFAIK only two transformations introduce $this: ExtensionMethods and TailRec, and extractSwitchCases looks tail-recursive. So my guess is that this method makes pattern-matching and tail-recursiveness interact in an interesting way we haven't tested before (probably because IntEqualityTestTreeMaker is a local class). It'd be great if you could try to reduce this to a simple test case.
TailRec methods remain members of enclosing class, it means that they can refer to methods that require this.type. It means that tailrec, unlike value classes is not allowed to widen type of $this to it's full self type. Fixes scala#1089
If the method that recurses over a different type arguments, if this method is specialised, it would lead to method not being tail-rec anymore. Eg: def foo[@specialized A, @specialized B]: Unit = foo[B, A]
This is a simpler fix than the previous one. Local methods, cannot change `this` and do not need to go through FullParameterization.
This does not yet unable the checks that `@switch` verifies that the compiler was indeed able to perform the optimization. This implementation also does not support guards. A match with guards will never be optimized as a switch.
|
Looks like this works, now :) |
|
LGTM |
This does not yet unable the checks that
@switchverifies that the compiler was indeed able to perform the optimization.This implementation also does not support guards. A match with guards will never be optimized as a switch.