Permalink
Browse files

SI-6902 Check unreachability under @unchecked

Only exhaustiveness checking should be disabled if the
scrutinee of a match as annotated as `: @unchecked`.
This was the pre-2.10.x behaviour.

This also fixes a variation of the closed ticket,
SI-6011. The exhaustiveness check is needed to
safely fallback from emitting a table switch if
duplicate cases are detected.
  • Loading branch information...
retronym committed Jan 19, 2013
1 parent 766bb97 commit cbd0205999d19e378f9f7ac8ca685a134862cf47
@@ -3225,6 +3225,7 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
// TODO: make more fine-grained, as we don't always need to jump
def canJump: Boolean
+ /** Should exhaustivity analysis be skipped? */
def unchecked: Boolean
@@ -3458,12 +3459,10 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
case Some(cds) => cds
}
- val allReachable = unchecked || {
- // a switch with duplicate cases yields a verify error,
- // and a switch with duplicate cases and guards cannot soundly be rewritten to an unguarded switch
- // (even though the verify error would disappear, the behaviour would change)
- unreachableCase(caseDefsWithGuards) map (cd => reportUnreachable(cd.body.pos)) isEmpty
- }
+ // a switch with duplicate cases yields a verify error,
+ // and a switch with duplicate cases and guards cannot soundly be rewritten to an unguarded switch
+ // (even though the verify error would disappear, the behaviour would change)
+ val allReachable = unreachableCase(caseDefsWithGuards) map (cd => reportUnreachable(cd.body.pos)) isEmpty
if (!allReachable) Nil
else if (noGuards(caseDefsWithGuards)) {
@@ -3710,10 +3709,10 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
with SymbolicMatchAnalysis
with DPLLSolver { self: TreeMakers =>
override def optimizeCases(prevBinder: Symbol, cases: List[List[TreeMaker]], pt: Type, unchecked: Boolean): (List[List[TreeMaker]], List[Tree]) = {
+ unreachableCase(prevBinder, cases, pt) foreach { caseIndex =>
+ reportUnreachable(cases(caseIndex).last.pos)
+ }
if (!unchecked) {
- unreachableCase(prevBinder, cases, pt) foreach { caseIndex =>
- reportUnreachable(cases(caseIndex).last.pos)
- }
val counterExamples = exhaustive(prevBinder, cases, pt)
if (counterExamples.nonEmpty)
reportMissingCases(prevBinder.pos, counterExamples)
View
@@ -0,0 +1,10 @@
+t6902.scala:4: error: unreachable code
+ case Some(b) => 3 // no warning was emitted
+ ^
+t6902.scala:9: error: unreachable code
+ case Some(b) => 3 // no warning was emitted
+ ^
+t6902.scala:21: error: unreachable code
+ case 1 => 3 // crash
+ ^
+three errors found
@@ -0,0 +1 @@
+-Xfatal-warnings
View
@@ -0,0 +1,23 @@
+object Test {
+ Some(Some(1)) collect {
+ case Some(a) => 2
+ case Some(b) => 3 // no warning was emitted
+ }
+
+ (Some(1): @ unchecked) match {
+ case Some(a) => 2
+ case Some(b) => 3 // no warning was emitted
+ }
+
+ // A variation of SI-6011, which eluded the fix
+ // in 2.10.0.
+ //
+ // duplicate keys in SWITCH, can't pick arbitrarily one of them to evict, see SI-6011.
+ // at scala.reflect.internal.SymbolTable.abort(SymbolTable.scala:50)
+ // at scala.tools.nsc.Global.abort(Global.scala:249)
+ // at scala.tools.nsc.backend.jvm.GenASM$JPlainBuilder$jcode$.emitSWITCH(GenASM.scala:1850)
+ ((1: Byte): @unchecked @annotation.switch) match {
+ case 1 => 2
+ case 1 => 3 // crash
+ }
+}
@@ -0,0 +1,13 @@
+object Test extends App {
+ // A variation of SI-6011, which eluded the fix
+ // in 2.10.0.
+ //
+ // duplicate keys in SWITCH, can't pick arbitrarily one of them to evict, see SI-6011.
+ // at scala.reflect.internal.SymbolTable.abort(SymbolTable.scala:50)
+ // at scala.tools.nsc.Global.abort(Global.scala:249)
+ // at scala.tools.nsc.backend.jvm.GenASM$JPlainBuilder$jcode$.emitSWITCH(GenASM.scala:1850)
+ ((1: Byte): @unchecked @annotation.switch) match {
+ case 1 => 2
+ case 1 => 3 // crash
+ }
+}

0 comments on commit cbd0205

Please sign in to comment.