Merge pull request #4016 from lrytz/t8731

SI-8731 warning if @switch is ignored

12 files changed, 93 insertions, 19 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala
index 1974befb45..3abec521df 100644
--- a/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala
+++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala
@@ -550,8 +550,24 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
                 case _ => false
               }
               val suppression = Suppression(suppressExhaustive, supressUnreachable)
+              val hasSwitchAnnotation = treeInfo.isSwitchAnnotation(tpt.tpe)
               // matches with two or fewer cases need not apply for switchiness (if-then-else will do)
-              val requireSwitch = treeInfo.isSwitchAnnotation(tpt.tpe) && casesNoSubstOnly.lengthCompare(2) > 0
+              // `case 1 | 2` is considered as two cases.
+              def exceedsTwoCasesOrAlts = {
+                // avoids traversing the entire list if there are more than 3 elements
+                def lengthMax3[T](l: List[T]): Int = l match {
+                  case a :: b :: c :: _ => 3
+                  case cases =>
+                    cases.map({
+                      case AlternativesTreeMaker(_, alts, _) :: _ => lengthMax3(alts)
+                      case c => 1
+                    }).sum
+                }
+                lengthMax3(casesNoSubstOnly) > 2
+              }
+              val requireSwitch = hasSwitchAnnotation && exceedsTwoCasesOrAlts
+              if (hasSwitchAnnotation && !requireSwitch)
+                reporter.warning(scrut.pos, "matches with two cases or fewer are emitted using if-then-else instead of switch")
               (suppression, requireSwitch)
             case _ =>
               (Suppression.NoSuppression, false)
diff --git a/test/files/neg/t6902.scala b/test/files/neg/t6902.scala
index ce5ff8b6fb..627c324279 100644
--- a/test/files/neg/t6902.scala
+++ b/test/files/neg/t6902.scala
@@ -16,7 +16,7 @@ object Test {
   // 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 {
+  ((1: Byte): @unchecked) match {
     case 1 => 2
     case 1 => 3 // crash
   }
diff --git a/test/files/neg/t8731.check b/test/files/neg/t8731.check
new file mode 100644
index 0000000000..2a9af475fc
--- /dev/null
+++ b/test/files/neg/t8731.check
@@ -0,0 +1,9 @@
+t8731.scala:5: warning: matches with two cases or fewer are emitted using if-then-else instead of switch
+  def f(x: Int) = (x: @annotation.switch) match {
+                       ^
+t8731.scala:10: warning: could not emit switch for @switch annotated match
+  def g(x: Int) = (x: @annotation.switch) match {
+                       ^
+error: No warnings can be incurred under -Xfatal-warnings.
+two warnings found
+one error found
diff --git a/test/files/neg/t8731.flags b/test/files/neg/t8731.flags
new file mode 100644
index 0000000000..e8fb65d50c
--- /dev/null
+++ b/test/files/neg/t8731.flags
@@ -0,0 +1 @@
+-Xfatal-warnings
\ No newline at end of file
diff --git a/test/files/neg/t8731.scala b/test/files/neg/t8731.scala
new file mode 100644
index 0000000000..d93fe706ad
--- /dev/null
+++ b/test/files/neg/t8731.scala
@@ -0,0 +1,15 @@
+class C {
+  // not a compile-time constant due to return type
+  final val K: Int = 20
+
+  def f(x: Int) = (x: @annotation.switch) match {
+    case K => 0
+    case 2 => 1
+  }
+
+  def g(x: Int) = (x: @annotation.switch) match {
+    case K => 0
+    case 2 => 1
+    case 3 => 2
+  }
+}
diff --git a/test/files/pos/switch-small.flags b/test/files/pos/switch-small.flags
deleted file mode 100644
index 85d8eb2ba2..0000000000
--- a/test/files/pos/switch-small.flags
+++ /dev/null
@@ -1 +0,0 @@
--Xfatal-warnings
diff --git a/test/files/pos/switch-small.scala b/test/files/pos/switch-small.scala
deleted file mode 100644
index 9de9ca028e..0000000000
--- a/test/files/pos/switch-small.scala
+++ /dev/null
@@ -1,8 +0,0 @@
-import annotation._
-
-object Test {
-  def f(x: Int) = (x: @switch) match {
-    case 1 => 1
-    case _ => 2
-  }
-}
diff --git a/test/files/run/t5830.check b/test/files/run/t5830.check
index 675387eb8e..9260854676 100644
--- a/test/files/run/t5830.check
+++ b/test/files/run/t5830.check
@@ -1,5 +1,6 @@
 a with oef
 a with oef
+a with oef
 a
 def with oef
 def
diff --git a/test/files/run/t5830.scala b/test/files/run/t5830.scala
index 5d808bfa28..03b9c540e0 100644
--- a/test/files/run/t5830.scala
+++ b/test/files/run/t5830.scala
@@ -1,12 +1,11 @@
 import scala.annotation.switch
 
 object Test extends App {
-  // TODO: should not emit a switch
-  // def noSwitch(ch: Char, eof: Boolean) = (ch: @switch) match {
-  //   case 'a' if eof => println("a with oef") // then branch
-  // }
+  def noSwitch(ch: Char, eof: Boolean) = ch match {
+    case 'a' if eof => println("a with oef") // then branch
+  }
 
-  def onlyThen(ch: Char, eof: Boolean) = (ch: @switch) match {
+  def onlyThen(ch: Char, eof: Boolean) = ch match {
     case 'a' if eof => println("a with oef") // then branch
     case 'c' =>
   }
@@ -18,7 +17,7 @@ object Test extends App {
     case 'c' =>
   }
 
-  def defaultUnguarded(ch: Char, eof: Boolean) = (ch: @switch) match {
+  def defaultUnguarded(ch: Char, eof: Boolean) = ch match {
     case ' ' if eof => println("spacey oef")
     case _ => println("default")
   }
@@ -44,7 +43,7 @@ object Test extends App {
   //   case 'c' =>
   // }
 
-  // noSwitch('a', true)
+  noSwitch('a', true)
   onlyThen('a', true)       // 'a with oef'
   ifThenElse('a', true)     // 'a with oef'
   ifThenElse('a', false)    // 'a'
diff --git a/test/files/run/t6011c.scala b/test/files/run/t6011c.scala
index 0647e3f81a..96a685b9cf 100644
--- a/test/files/run/t6011c.scala
+++ b/test/files/run/t6011c.scala
@@ -6,7 +6,7 @@ object Test extends App {
   // 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 {
+  ((1: Byte): @unchecked) match {
     case 1 => 2
     case 1 => 3 // crash
   }
diff --git a/test/junit/scala/issues/BytecodeTests.scala b/test/junit/scala/issues/BytecodeTests.scala
new file mode 100644
index 0000000000..7a05472277
--- /dev/null
+++ b/test/junit/scala/issues/BytecodeTests.scala
@@ -0,0 +1,39 @@
+package scala.issues
+
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.junit.Test
+import scala.tools.asm.Opcodes
+import scala.tools.nsc.backend.jvm.CodeGenTools._
+import org.junit.Assert._
+import scala.collection.JavaConverters._
+import scala.tools.partest.ASMConverters._
+
+@RunWith(classOf[JUnit4])
+class BytecodeTests {
+  val compiler = newCompiler()
+
+  @Test
+  def t8731(): Unit = {
+    val code =
+      """class C {
+        |  def f(x: Int) = (x: @annotation.switch) match {
+        |    case 1 => 0
+        |    case 2 => 1
+        |    case 3 => 2
+        |  }
+        |  final val K = 10
+        |  def g(x: Int) = (x: @annotation.switch) match {
+        |    case K => 0
+        |    case 1 => 10
+        |    case 2 => 20
+        |  }
+        |}
+      """.stripMargin
+
+    val List(c) = compileClasses(compiler)(code)
+
+    assertTrue(getSingleMethod(c, "f").instructions.count(_.isInstanceOf[TableSwitch]) == 1)
+    assertTrue(getSingleMethod(c, "g").instructions.count(_.isInstanceOf[LookupSwitch]) == 1)
+  }
+}
diff --git a/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala b/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
index 15bc1f427d..b892eb36cf 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
@@ -76,4 +76,7 @@ object CodeGenTools {
   def assertSameCode(actual: List[Instruction], expected: List[Instruction]): Unit = {
     assertTrue(s"\nExpected: $expected\nActual  : $actual", actual === expected)
   }
+
+  def getSingleMethod(classNode: ClassNode, name: String): Method =
+    convertMethod(classNode.methods.asScala.toList.find(_.name == name).get)
 }