Skip to content

Commit aa0b9b4

Browse files
griggtmichelou
authored andcommitted
Fix scala#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 scala#5070 for an example.
1 parent 3776afb commit aa0b9b4

File tree

12 files changed

+109
-39
lines changed

12 files changed

+109
-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
@@ -1998,10 +1998,9 @@ import transform.SymUtils._
19981998
|whose behavior may have changed since version change."""
19991999
}
20002000

2001-
class UnableToEmitSwitch(tooFewCases: Boolean)(using Context)
2001+
class UnableToEmitSwitch()(using Context)
20022002
extends SyntaxMsg(UnableToEmitSwitchID) {
2003-
def tooFewStr: String = if (tooFewCases) " since there are not enough cases" else ""
2004-
def msg = em"Could not emit switch for ${hl("@switch")} annotated match$tooFewStr"
2003+
def msg = em"Could not emit switch for ${hl("@switch")} annotated match"
20052004
def explain = {
20062005
val codeExample =
20072006
"""val ConstantB = 'B'

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -977,30 +977,28 @@ object PatternMatcher {
977977
/** If match is switch annotated, check that it translates to a switch
978978
* with at least as many cases as the original match.
979979
*/
980-
private def checkSwitch(original: Match, result: Tree) = original.selector match {
980+
private def checkSwitch(original: Match, result: Tree) = original.selector match
981981
case Typed(_, tpt) if tpt.tpe.hasAnnotation(defn.SwitchAnnot) =>
982-
val resultCases = result match {
982+
val resultCases = result match
983983
case Match(_, cases) => cases
984984
case Block(_, Match(_, cases)) => cases
985985
case _ => Nil
986-
}
987-
def typesInPattern(pat: Tree): List[Type] = pat match {
986+
def typesInPattern(pat: Tree): List[Type] = pat match
988987
case Alternative(pats) => pats.flatMap(typesInPattern)
989988
case _ => pat.tpe :: Nil
990-
}
991989
def typesInCases(cdefs: List[CaseDef]): List[Type] =
992990
cdefs.flatMap(cdef => typesInPattern(cdef.pat))
993991
def numTypes(cdefs: List[CaseDef]): Int =
994992
typesInCases(cdefs).toSet.size: Int // without the type ascription, testPickling fails because of #2840.
995-
if (numTypes(resultCases) < numTypes(original.cases)) {
993+
val numTypesInOriginal = numTypes(original.cases)
994+
if numTypesInOriginal >= MinSwitchCases && numTypes(resultCases) < numTypesInOriginal then
996995
patmatch.println(i"switch warning for ${ctx.compilationUnit}")
997996
patmatch.println(i"original types: ${typesInCases(original.cases)}%, %")
998997
patmatch.println(i"switch types : ${typesInCases(resultCases)}%, %")
999998
patmatch.println(i"tree = $result")
1000-
report.warning(UnableToEmitSwitch(numTypes(original.cases) < MinSwitchCases), original.srcPos)
1001-
}
999+
report.warning(UnableToEmitSwitch(), original.srcPos)
10021000
case _ =>
1003-
}
1001+
end checkSwitch
10041002

10051003
val optimizations: List[(String, Plan => Plan)] = List(
10061004
"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)