diff options
author | Lukas Rytz <lukas.rytz@gmail.com> | 2014-09-29 17:52:40 +0200 |
---|---|---|
committer | Lukas Rytz <lukas.rytz@gmail.com> | 2014-10-06 20:56:32 +0200 |
commit | 0c25979244877b4431066700a6e945f145771c3c (patch) | |
tree | 92d631fc91e90c55508eda2b4aa460c2ba29658c | |
parent | a52db7f1639c6d48eaa64ae609385a60467fd566 (diff) | |
download | scala-0c25979244877b4431066700a6e945f145771c3c.tar.gz scala-0c25979244877b4431066700a6e945f145771c3c.tar.bz2 scala-0c25979244877b4431066700a6e945f145771c3c.zip |
SI-8731 warning if @switch is ignored
For matches with two or fewer cases, @switch is ignored. This should
not happen silently.
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala | 18 | ||||
-rw-r--r-- | test/files/neg/t6902.scala | 2 | ||||
-rw-r--r-- | test/files/neg/t8731.check | 9 | ||||
-rw-r--r-- | test/files/neg/t8731.flags | 1 | ||||
-rw-r--r-- | test/files/neg/t8731.scala | 15 | ||||
-rw-r--r-- | test/files/pos/switch-small.flags | 1 | ||||
-rw-r--r-- | test/files/pos/switch-small.scala | 8 | ||||
-rw-r--r-- | test/files/run/t5830.check | 1 | ||||
-rw-r--r-- | test/files/run/t5830.scala | 13 | ||||
-rw-r--r-- | test/files/run/t6011c.scala | 2 | ||||
-rw-r--r-- | test/junit/scala/issues/BytecodeTests.scala | 39 | ||||
-rw-r--r-- | test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala | 3 |
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) } |