Skip to content

Commit f2a2c04

Browse files
committed
Fix #9776: Don't issue a @switch warning when fewer than 4 cases
In the spirit of scala/scala#4418, which fixed SI-8731. The consensus seems to be that the intent of the @switch annotation is to warn when the generated bytecode may be poor, not merely if the compiler elects to not emit a tableswitch/lookupswitch. Note that the case threshold for determining whether a switch is emitted is implementation dependent, and currently varies between scalac and dotc. Also note that this implementation will not warn in instances where a switch will never be emitted (e.g. because the match is on a non-integral type) but the number of cases is below the warning threshold. This behavior is consistent with scalac, but may be surprising to the user if another case is added later and a warning suddenly appears. Not all spurious @switch warnings are addressed by this commit, see issue #5070 for an example.
1 parent f9e2428 commit f2a2c04

File tree

12 files changed

+108
-39
lines changed

12 files changed

+108
-39
lines changed

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1969,10 +1969,9 @@ import ast.tpd
19691969
|whose behavior may have changed since version change."""
19701970
}
19711971

1972-
class UnableToEmitSwitch(tooFewCases: Boolean)(using Context)
1972+
class UnableToEmitSwitch()(using Context)
19731973
extends SyntaxMsg(UnableToEmitSwitchID) {
1974-
def tooFewStr: String = if (tooFewCases) " since there are not enough cases" else ""
1975-
def msg = em"Could not emit switch for ${hl("@switch")} annotated match$tooFewStr"
1974+
def msg = em"Could not emit switch for ${hl("@switch")} annotated match"
19761975
def explain = {
19771976
val codeExample =
19781977
"""val ConstantB = 'B'

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

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -968,30 +968,27 @@ object PatternMatcher {
968968
/** If match is switch annotated, check that it translates to a switch
969969
* with at least as many cases as the original match.
970970
*/
971-
private def checkSwitch(original: Match, result: Tree) = original.selector match {
971+
private def checkSwitch(original: Match, result: Tree) = original.selector match
972972
case Typed(_, tpt) if tpt.tpe.hasAnnotation(defn.SwitchAnnot) =>
973-
val resultCases = result match {
973+
val resultCases = result match
974974
case Match(_, cases) => cases
975975
case Block(_, Match(_, cases)) => cases
976976
case _ => Nil
977-
}
978-
def typesInPattern(pat: Tree): List[Type] = pat match {
977+
def typesInPattern(pat: Tree): List[Type] = pat match
979978
case Alternative(pats) => pats.flatMap(typesInPattern)
980979
case _ => pat.tpe :: Nil
981-
}
982980
def typesInCases(cdefs: List[CaseDef]): List[Type] =
983981
cdefs.flatMap(cdef => typesInPattern(cdef.pat))
984982
def numTypes(cdefs: List[CaseDef]): Int =
985983
typesInCases(cdefs).toSet.size: Int // without the type ascription, testPickling fails because of #2840.
986-
if (numTypes(resultCases) < numTypes(original.cases)) {
984+
if numTypes(original.cases) >= MinSwitchCases && numTypes(resultCases) < numTypes(original.cases) then
987985
patmatch.println(i"switch warning for ${ctx.compilationUnit}")
988986
patmatch.println(i"original types: ${typesInCases(original.cases)}%, %")
989987
patmatch.println(i"switch types : ${typesInCases(resultCases)}%, %")
990988
patmatch.println(i"tree = $result")
991-
report.warning(UnableToEmitSwitch(numTypes(original.cases) < MinSwitchCases), original.srcPos)
992-
}
989+
report.warning(UnableToEmitSwitch(), original.srcPos)
993990
case _ =>
994-
}
991+
end checkSwitch
995992

996993
val optimizations: List[(String, Plan => Plan)] = List(
997994
"mergeTests" -> mergeTests,

tests/neg-custom-args/fatal-warnings/i3561.scala

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,4 @@ class Test {
1313
case '5' | Constant => 3
1414
case '4' => 4
1515
}
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-
}
2216
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import scala.annotation.switch
2+
3+
sealed trait Fruit
4+
5+
object Fruit {
6+
case object Apple extends Fruit
7+
case object Banana extends Fruit
8+
case object Lemon extends Fruit
9+
case object Lime extends Fruit
10+
case object Orange extends Fruit
11+
12+
def isCitrus(fruit: Fruit): Boolean =
13+
(fruit: @switch) match { // error Could not emit switch for @switch annotated match
14+
case Orange => true
15+
case Lemon => true
16+
case Lime => true
17+
case _ => false
18+
}
19+
}
20+
21+
22+
sealed trait TaggedFruit {
23+
def tag: Int
24+
}
25+
26+
object TaggedFruit {
27+
case object Apple extends TaggedFruit {
28+
val tag = 1
29+
}
30+
case object Banana extends TaggedFruit {
31+
val tag = 2
32+
}
33+
case object Orange extends TaggedFruit {
34+
val tag = 3
35+
}
36+
37+
def isCitrus(fruit: TaggedFruit): Boolean =
38+
(fruit.tag: @switch) match { // error Could not emit switch for @switch annotated match
39+
case Apple.tag => true
40+
case 2 => true
41+
case 3 => true
42+
case _ => false
43+
}
44+
45+
// fewer than four cases, so no warning
46+
def succ1(fruit: TaggedFruit): Boolean =
47+
(fruit.tag: @switch) match {
48+
case 3 => false
49+
case 2 | Apple.tag => true
50+
}
51+
52+
// fewer than four cases, so no warning
53+
def succ2(fruit: TaggedFruit): Boolean =
54+
(fruit.tag: @switch) match {
55+
case 3 => false
56+
case 2 => true
57+
case Apple.tag => true
58+
}
59+
}

tests/neg-custom-args/fatal-warnings/switches.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,15 @@ object Main {
3838
// thinks a val in an object is constant... so naive
3939
def fail1(c: Char) = (c: @switch @unchecked) match { // error: Could not emit switch for @switch annotated match
4040
case 'A' => true
41+
case 'B' => true
4142
case Other.C1 => true
4243
case _ => false
4344
}
4445

4546
// more naivete
4647
def fail2(c: Char) = (c: @unchecked @switch) match { // error: Could not emit switch for @switch annotated match
4748
case 'A' => true
49+
case 'B' => true
4850
case Other.C3 => true
4951
case _ => false
5052
}

tests/untried/neg/t5830.scala renamed to tests/neg-custom-args/fatal-warnings/t5830.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import scala.annotation.switch
33
class Test {
44
def unreachable(ch: Char) = (ch: @switch) match {
55
case 'a' => println("b") // ok
6-
case 'a' => println("b") // unreachable
6+
case 'a' => println("b") // error: unreachable case
77
case 'c' =>
88
}
99
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import scala.annotation.switch
2+
3+
sealed trait Fruit
4+
5+
object Fruit {
6+
case object Apple extends Fruit
7+
case object Banana extends Fruit
8+
case object Orange extends Fruit
9+
10+
def isCitrus(fruit: Fruit): Boolean =
11+
(fruit: @switch) match {
12+
case Orange => true
13+
case _ => false
14+
}
15+
}
16+
17+
18+
sealed trait TaggedFruit {
19+
def tag: Int
20+
}
21+
22+
object TaggedFruit {
23+
case object Apple extends TaggedFruit {
24+
val tag = 1
25+
}
26+
case object Banana extends TaggedFruit {
27+
val tag = 2
28+
}
29+
case object Orange extends TaggedFruit {
30+
val tag = 3
31+
}
32+
33+
def isCitrus(fruit: TaggedFruit): Boolean =
34+
(fruit.tag: @switch) match {
35+
case 3 => true
36+
case _ => false
37+
}
38+
}
File renamed without changes.

tests/untried/neg/switch.check

Lines changed: 0 additions & 9 deletions
This file was deleted.

tests/untried/neg/switch.flags

Lines changed: 0 additions & 1 deletion
This file was deleted.

tests/untried/neg/t5830.check

Lines changed: 0 additions & 9 deletions
This file was deleted.

tests/untried/neg/t5830.flags

Lines changed: 0 additions & 1 deletion
This file was deleted.

0 commit comments

Comments
 (0)