Merge pull request #3724 from dotty-staging/fix-#3561

Fix #3561: Correctly handle switches followed by other tests in matches

Author: odersky

compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala

@@ -2030,10 +2030,11 @@ object messages {
     }
   }
 
-  case class UnableToEmitSwitch()(implicit ctx: Context)
+  case class UnableToEmitSwitch(tooFewCases: Boolean)(implicit ctx: Context)
   extends Message(UnableToEmitSwitchID) {
     val kind = "Syntax"
-    val msg = hl"Could not emit switch for ${"@switch"} annotated match"
+    val tooFewStr = if (tooFewCases) " since there are not enough cases" else ""
+    val msg = hl"Could not emit switch for ${"@switch"} annotated match$tooFewStr"
     val explanation = {
       val codeExample =
         """val ConstantB = 'B'

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

@@ -48,6 +48,9 @@ object PatternMatcher {
 
   final val selfCheck = false // debug option, if on we check that no case gets generated twice
 
+  /** Minimal number of cases to emit a switch */
+  final val MinSwitchCases = 4
+
   /** Was symbol generated by pattern matcher? */
   def isPatmatGenerated(sym: Symbol)(implicit ctx: Context): Boolean =
     sym.is(Synthetic) &&
@@ -845,10 +848,10 @@ object PatternMatcher {
 
     /** Emit cases of a switch */
     private def emitSwitchCases(cases: List[Plan]): List[CaseDef] = (cases: @unchecked) match {
-      case TestPlan(EqualTest(tree), _, _, ons, _) :: cases1 =>
-        CaseDef(tree, EmptyTree, emit(ons)) :: emitSwitchCases(cases1)
       case (default: Plan) :: Nil =>
         CaseDef(Underscore(defn.IntType), EmptyTree, emit(default)) :: Nil
+      case TestPlan(EqualTest(tree), _, _, ons, _) :: cases1 =>
+        CaseDef(tree, EmptyTree, emit(ons)) :: emitSwitchCases(cases1)
     }
 
     /** If selfCheck is `true`, used to check whether a tree gets generated twice */
@@ -863,7 +866,7 @@ object PatternMatcher {
       plan match {
         case plan: TestPlan =>
           val switchCases = collectSwitchCases(plan)
-          if (switchCases.lengthCompare(4) >= 0) // at least 3 cases + default
+          if (switchCases.lengthCompare(MinSwitchCases) >= 0) // at least 3 cases + default
             Match(plan.scrutinee, emitSwitchCases(switchCases))
           else {
             /** Merge nested `if`s that have the same `else` branch into a single `if`.
@@ -969,12 +972,21 @@ object PatternMatcher {
           case Block(_, Match(_, cases)) => cases
           case _ => Nil
         }
-        def numConsts(cdefs: List[CaseDef]): Int = {
-          val tpes = cdefs.map(_.pat.tpe)
-          tpes.toSet.size
+        def typesInPattern(pat: Tree): List[Type] = pat match {
+          case Alternative(pats) => pats.flatMap(typesInPattern)
+          case _ => pat.tpe :: Nil
+        }
+        def typesInCases(cdefs: List[CaseDef]): List[Type] =
+          cdefs.flatMap(cdef => typesInPattern(cdef.pat))
+        def numTypes(cdefs: List[CaseDef]): Int =
+          typesInCases(cdefs).toSet.size: Int // without the type ascription, testPickling fails because of #2840.
+        if (numTypes(resultCases) < numTypes(original.cases)) {
+          patmatch.println(i"switch warning for ${ctx.compilationUnit}")
+          patmatch.println(i"original types: ${typesInCases(original.cases)}%, %")
+          patmatch.println(i"switch types  : ${typesInCases(resultCases)}%, %")
+          patmatch.println(i"tree = $result")
+          ctx.warning(UnableToEmitSwitch(numTypes(original.cases) < MinSwitchCases), original.pos)
         }
-        if (numConsts(resultCases) < numConsts(original.cases))
-          ctx.warning(UnableToEmitSwitch(), original.pos)
       case _ =>
     }
 

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -184,6 +184,7 @@ class CompilationTests extends ParallelTesting {
     compileFile("../tests/neg-custom-args/noimports2.scala", defaultOptions.and("-Yno-imports")) +
     compileFile("../tests/neg-custom-args/overloadsOnAbstractTypes.scala", allowDoubleBindings) +
     compileFile("../tests/neg-custom-args/xfatalWarnings.scala", defaultOptions.and("-Xfatal-warnings")) +
+    compileFile("../tests/neg-custom-args/i3561.scala", defaultOptions.and("-Xfatal-warnings")) +
     compileFile("../tests/neg-custom-args/pureStatement.scala", defaultOptions.and("-Xfatal-warnings")) +
     compileFile("../tests/neg-custom-args/i3589-a.scala", defaultOptions.and("-Xfatal-warnings")) +
     compileFile("../tests/neg-custom-args/i2333.scala", defaultOptions.and("-Xfatal-warnings")) +

tests/neg-custom-args/i3561.scala

@@ -0,0 +1,22 @@
+class Test {
+  val Constant = 'Q' // OK if final
+  def tokenMe(ch: Char) = (ch: @annotation.switch) match { // error: could not emit switch
+    case ' ' => 1
+    case 'A' => 2
+    case '5' | Constant => 3
+    case '4' => 4
+  }
+
+  def test2(x: Any) = (x: @annotation.switch) match { // error: could not emit switch
+    case ' ' => 1
+    case 'A' => 2
+    case '5' | Constant => 3
+    case '4' => 4
+  }
+
+  def test3(x: Any) = (x: @annotation.switch) match { // error: could not emit switch - too few cases
+    case 1 => 1
+    case 2 => 2
+    case x: String => 4
+  }
+}

tests/pos/i3561.scala

@@ -0,0 +1,15 @@
+class Test {
+  val Constant = 'Q' // OK if final
+  def tokenMe(ch: Char) = (ch: @annotation.switch) match {
+    case ' ' => 1
+    case 'A' => 2
+    case '5' | Constant => 3
+  }
+
+  def test2(x: Any) = x match {
+    case 1 => 1
+    case 2 => 2
+    case 3 => 3
+    case x: String => 4
+  }
+}

tests/pos/typers.scala

@@ -77,7 +77,7 @@ object typers {
 
   class C {
 
-    @tailrec final def factorial(acc: Int, n: Int): Int = (n: @switch) match {
+    @tailrec final def factorial(acc: Int, n: Int): Int = n match {
       case 0 => acc
       case _ => factorial(acc * n, n - 1)
     }

tests/run/switches.scala

@@ -18,7 +18,7 @@ object Test extends App {
     case _ => 4
   }
 
-  val x3 = (x: @switch) match {
+  val x3 = x match {
     case '0' if x > 0 => 0
     case '1' => 1
     case '2' => 2

tests/run/t5830.scala

@@ -10,7 +10,7 @@ object Test extends dotty.runtime.LegacyApp {
     case 'c' =>
   }
 
-  def ifThenElse(ch: Char, eof: Boolean) = (ch: @switch) match {
+  def ifThenElse(ch: Char, eof: Boolean) = ch match {
     case 'a' if eof => println("a with oef") // then branch
     case 'a' if eof => println("a with oef2") // unreachable, but the analysis is not that sophisticated
     case 'a' => println("a") // else-branch
@@ -22,7 +22,7 @@ object Test extends dotty.runtime.LegacyApp {
     case _ => println("default")
   }
 
-  def defaults(ch: Char, eof: Boolean) = (ch: @switch) match {
+  def defaults(ch: Char, eof: Boolean) = ch match {
     case _ if eof => println("def with oef") // then branch
     case _ if eof => println("def with oef2") // unreachable, but the analysis is not that sophisticated
     case _ => println("def") // else-branch