Skip to content

Commit 691f868

Browse files
authored
Merge pull request #3724 from dotty-staging/fix-#3561
Fix #3561: Correctly handle switches followed by other tests in matches
2 parents 08e6735 + 9191288 commit 691f868

File tree

8 files changed

+65
-14
lines changed

8 files changed

+65
-14
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2030,10 +2030,11 @@ object messages {
20302030
}
20312031
}
20322032

2033-
case class UnableToEmitSwitch()(implicit ctx: Context)
2033+
case class UnableToEmitSwitch(tooFewCases: Boolean)(implicit ctx: Context)
20342034
extends Message(UnableToEmitSwitchID) {
20352035
val kind = "Syntax"
2036-
val msg = hl"Could not emit switch for ${"@switch"} annotated match"
2036+
val tooFewStr = if (tooFewCases) " since there are not enough cases" else ""
2037+
val msg = hl"Could not emit switch for ${"@switch"} annotated match$tooFewStr"
20372038
val explanation = {
20382039
val codeExample =
20392040
"""val ConstantB = 'B'

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

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ object PatternMatcher {
4848

4949
final val selfCheck = false // debug option, if on we check that no case gets generated twice
5050

51+
/** Minimal number of cases to emit a switch */
52+
final val MinSwitchCases = 4
53+
5154
/** Was symbol generated by pattern matcher? */
5255
def isPatmatGenerated(sym: Symbol)(implicit ctx: Context): Boolean =
5356
sym.is(Synthetic) &&
@@ -845,10 +848,10 @@ object PatternMatcher {
845848

846849
/** Emit cases of a switch */
847850
private def emitSwitchCases(cases: List[Plan]): List[CaseDef] = (cases: @unchecked) match {
848-
case TestPlan(EqualTest(tree), _, _, ons, _) :: cases1 =>
849-
CaseDef(tree, EmptyTree, emit(ons)) :: emitSwitchCases(cases1)
850851
case (default: Plan) :: Nil =>
851852
CaseDef(Underscore(defn.IntType), EmptyTree, emit(default)) :: Nil
853+
case TestPlan(EqualTest(tree), _, _, ons, _) :: cases1 =>
854+
CaseDef(tree, EmptyTree, emit(ons)) :: emitSwitchCases(cases1)
852855
}
853856

854857
/** If selfCheck is `true`, used to check whether a tree gets generated twice */
@@ -863,7 +866,7 @@ object PatternMatcher {
863866
plan match {
864867
case plan: TestPlan =>
865868
val switchCases = collectSwitchCases(plan)
866-
if (switchCases.lengthCompare(4) >= 0) // at least 3 cases + default
869+
if (switchCases.lengthCompare(MinSwitchCases) >= 0) // at least 3 cases + default
867870
Match(plan.scrutinee, emitSwitchCases(switchCases))
868871
else {
869872
/** Merge nested `if`s that have the same `else` branch into a single `if`.
@@ -969,12 +972,21 @@ object PatternMatcher {
969972
case Block(_, Match(_, cases)) => cases
970973
case _ => Nil
971974
}
972-
def numConsts(cdefs: List[CaseDef]): Int = {
973-
val tpes = cdefs.map(_.pat.tpe)
974-
tpes.toSet.size
975+
def typesInPattern(pat: Tree): List[Type] = pat match {
976+
case Alternative(pats) => pats.flatMap(typesInPattern)
977+
case _ => pat.tpe :: Nil
978+
}
979+
def typesInCases(cdefs: List[CaseDef]): List[Type] =
980+
cdefs.flatMap(cdef => typesInPattern(cdef.pat))
981+
def numTypes(cdefs: List[CaseDef]): Int =
982+
typesInCases(cdefs).toSet.size: Int // without the type ascription, testPickling fails because of #2840.
983+
if (numTypes(resultCases) < numTypes(original.cases)) {
984+
patmatch.println(i"switch warning for ${ctx.compilationUnit}")
985+
patmatch.println(i"original types: ${typesInCases(original.cases)}%, %")
986+
patmatch.println(i"switch types : ${typesInCases(resultCases)}%, %")
987+
patmatch.println(i"tree = $result")
988+
ctx.warning(UnableToEmitSwitch(numTypes(original.cases) < MinSwitchCases), original.pos)
975989
}
976-
if (numConsts(resultCases) < numConsts(original.cases))
977-
ctx.warning(UnableToEmitSwitch(), original.pos)
978990
case _ =>
979991
}
980992

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ class CompilationTests extends ParallelTesting {
184184
compileFile("../tests/neg-custom-args/noimports2.scala", defaultOptions.and("-Yno-imports")) +
185185
compileFile("../tests/neg-custom-args/overloadsOnAbstractTypes.scala", allowDoubleBindings) +
186186
compileFile("../tests/neg-custom-args/xfatalWarnings.scala", defaultOptions.and("-Xfatal-warnings")) +
187+
compileFile("../tests/neg-custom-args/i3561.scala", defaultOptions.and("-Xfatal-warnings")) +
187188
compileFile("../tests/neg-custom-args/pureStatement.scala", defaultOptions.and("-Xfatal-warnings")) +
188189
compileFile("../tests/neg-custom-args/i3589-a.scala", defaultOptions.and("-Xfatal-warnings")) +
189190
compileFile("../tests/neg-custom-args/i2333.scala", defaultOptions.and("-Xfatal-warnings")) +

tests/neg-custom-args/i3561.scala

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
class Test {
2+
val Constant = 'Q' // OK if final
3+
def tokenMe(ch: Char) = (ch: @annotation.switch) match { // error: could not emit switch
4+
case ' ' => 1
5+
case 'A' => 2
6+
case '5' | Constant => 3
7+
case '4' => 4
8+
}
9+
10+
def test2(x: Any) = (x: @annotation.switch) match { // error: could not emit switch
11+
case ' ' => 1
12+
case 'A' => 2
13+
case '5' | Constant => 3
14+
case '4' => 4
15+
}
16+
17+
def test3(x: Any) = (x: @annotation.switch) match { // error: could not emit switch - too few cases
18+
case 1 => 1
19+
case 2 => 2
20+
case x: String => 4
21+
}
22+
}

tests/pos/i3561.scala

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
class Test {
2+
val Constant = 'Q' // OK if final
3+
def tokenMe(ch: Char) = (ch: @annotation.switch) match {
4+
case ' ' => 1
5+
case 'A' => 2
6+
case '5' | Constant => 3
7+
}
8+
9+
def test2(x: Any) = x match {
10+
case 1 => 1
11+
case 2 => 2
12+
case 3 => 3
13+
case x: String => 4
14+
}
15+
}

tests/pos/typers.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ object typers {
7777

7878
class C {
7979

80-
@tailrec final def factorial(acc: Int, n: Int): Int = (n: @switch) match {
80+
@tailrec final def factorial(acc: Int, n: Int): Int = n match {
8181
case 0 => acc
8282
case _ => factorial(acc * n, n - 1)
8383
}

tests/run/switches.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ object Test extends App {
1818
case _ => 4
1919
}
2020

21-
val x3 = (x: @switch) match {
21+
val x3 = x match {
2222
case '0' if x > 0 => 0
2323
case '1' => 1
2424
case '2' => 2

tests/run/t5830.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ object Test extends dotty.runtime.LegacyApp {
1010
case 'c' =>
1111
}
1212

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

25-
def defaults(ch: Char, eof: Boolean) = (ch: @switch) match {
25+
def defaults(ch: Char, eof: Boolean) = ch match {
2626
case _ if eof => println("def with oef") // then branch
2727
case _ if eof => println("def with oef2") // unreachable, but the analysis is not that sophisticated
2828
case _ => println("def") // else-branch

0 commit comments

Comments
 (0)