Merge pull request #2291 from retronym/ticket/7290

SI-7290 Discard duplicates in switchable alternative patterns.

5 files changed, 45 insertions, 5 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala
index dcf2413b15..c570dd8572 100644
--- a/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala
+++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala
@@ -16,13 +16,13 @@ import scala.reflect.internal.util.NoPosition
 /** Optimize and analyze matches based on their TreeMaker-representation.
  *
  * The patmat translation doesn't rely on this, so it could be disabled in principle.
- *
- * TODO: split out match analysis
+ *  - well, not quite: the backend crashes if we emit duplicates in switches (e.g. SI-7290)
  */
+// TODO: split out match analysis
 trait MatchOptimization extends MatchTreeMaking with MatchAnalysis {
   import PatternMatchingStats._
   import global.{Tree, Type, Symbol, NoSymbol, CaseDef, atPos,
-    ConstantType, Literal, Constant, gen, EmptyTree,
+    ConstantType, Literal, Constant, gen, EmptyTree, distinctBy,
     Typed, treeInfo, nme, Ident,
     Apply, If, Bind, lub, Alternative, deriveCaseDef, Match, MethodType, LabelDef, TypeTree, Throw}
 
@@ -442,7 +442,7 @@ trait MatchOptimization extends MatchTreeMaking with MatchAnalysis {
               case SwitchableTreeMaker(pattern) :: GuardAndBodyTreeMakers(guard, body) =>
                 Some(CaseDef(pattern, guard, body))
               // alternatives
-              case AlternativesTreeMaker(_, altss, _) :: GuardAndBodyTreeMakers(guard, body) if alternativesSupported =>
+              case AlternativesTreeMaker(_, altss, pos) :: GuardAndBodyTreeMakers(guard, body) if alternativesSupported =>
                 val switchableAlts = altss map {
                   case SwitchableTreeMaker(pattern) :: Nil =>
                     Some(pattern)
@@ -452,7 +452,17 @@ trait MatchOptimization extends MatchTreeMaking with MatchAnalysis {
 
                 // succeed if they were all switchable
                 sequence(switchableAlts) map { switchableAlts =>
-                  CaseDef(Alternative(switchableAlts), guard, body)
+                  def extractConst(t: Tree) = t match {
+                    case Literal(const) => const
+                    case _              => t
+                  }
+                  // SI-7290 Discard duplicate alternatives that would crash the backend
+                  val distinctAlts = distinctBy(switchableAlts)(extractConst)
+                  if (distinctAlts.size < switchableAlts.size) {
+                    val duplicated = switchableAlts.groupBy(extractConst).flatMap(_._2.drop(1).take(1)) // report the first duplicated
+                    global.currentUnit.warning(pos, s"Pattern contains duplicate alternatives: ${duplicated.mkString(", ")}")
+                  }
+                  CaseDef(Alternative(distinctAlts), guard, body)
                 }
               case _ =>
                 // debug.patmat("can't emit switch for "+ makers)
diff --git a/test/files/neg/t7290.check b/test/files/neg/t7290.check
new file mode 100644
index 0000000000..85bedbac95
--- /dev/null
+++ b/test/files/neg/t7290.check
@@ -0,0 +1,10 @@
+t7290.scala:4: error: Pattern contains duplicate alternatives: 0
+    case 0 | 0 => 0
+         ^
+t7290.scala:5: error: Pattern contains duplicate alternatives: 2, 3
+    case 2 | 2 | 2 | 3 | 2 | 3 => 0
+         ^
+t7290.scala:6: error: Pattern contains duplicate alternatives: 4
+    case 4 | (_ @ 4) => 0
+         ^
+three errors found
diff --git a/test/files/neg/t7290.flags b/test/files/neg/t7290.flags
new file mode 100644
index 0000000000..e8fb65d50c
--- /dev/null
+++ b/test/files/neg/t7290.flags
@@ -0,0 +1 @@
+-Xfatal-warnings
\ No newline at end of file
diff --git a/test/files/neg/t7290.scala b/test/files/neg/t7290.scala
new file mode 100644
index 0000000000..b9db7f7e8a
--- /dev/null
+++ b/test/files/neg/t7290.scala
@@ -0,0 +1,10 @@
+object Test extends App {
+  val y = (0: Int) match {
+    case 1 => 1
+    case 0 | 0 => 0
+    case 2 | 2 | 2 | 3 | 2 | 3 => 0
+    case 4 | (_ @ 4) => 0
+    case _ => -1
+  }
+  assert(y == 0, y)
+}
diff --git a/test/files/run/t7290.scala b/test/files/run/t7290.scala
new file mode 100644
index 0000000000..01f7e8f68e
--- /dev/null
+++ b/test/files/run/t7290.scala
@@ -0,0 +1,9 @@
+object Test extends App {
+  val y = (0: Int) match {
+    case 1 => 1
+    case 0 | 0 => 0
+    case 2 | 2 | 2 | 3 | 2 | 3 => 0
+    case _ => -1
+  }
+  assert(y == 0, y)
+}