From 911f05dfce06e83b9bb76184bd24b9d6fa1490e0 Mon Sep 17 00:00:00 2001
From: Liu Fengyun <liu@fengy.me>
Date: Mon, 15 Apr 2019 17:38:15 +0200
Subject: [PATCH 1/2] Fix #4935: make desugaring of PatDef sound

We can only optimize `val pat = if (...) e1 else e2` if:

- `e1` and `e2` are both tuples of arity N
- `pat` is a tuple of N variables or wildcard patterns like `(x1, x2, ..., xN)`

Otherwise, we need to generate pattern matches.
---
 .../src/dotty/tools/dotc/ast/Desugar.scala    | 28 +++++++++++++++----
 .../dotty/tools/dotc/parsing/Parsers.scala    |  2 +-
 tests/neg/i4935.scala                         |  3 ++
 tests/run/i4935.scala                         |  5 ++++
 tests/run/i4935b.scala                        |  5 ++++
 5 files changed, 37 insertions(+), 6 deletions(-)
 create mode 100644 tests/neg/i4935.scala
 create mode 100644 tests/run/i4935.scala
 create mode 100644 tests/run/i4935b.scala

diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala
index c3682329cd68..c746cfd171d0 100644
--- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala
+++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala
@@ -913,16 +913,34 @@ object desugar {
     case IdPattern(named, tpt) =>
       derivedValDef(original, named, tpt, rhs, mods)
     case _ =>
-      val rhsUnchecked = makeAnnotated("scala.unchecked", rhs)
-      val vars = getVariables(pat)
+      def isTuplePattern(arity: Int): Boolean = pat match {
+        case Tuple(pats) if pats.size == arity =>
+          pats.forall(id => id.isInstanceOf[Ident] && isVarPattern(id))
+        case _ => false
+      }
       val isMatchingTuple: Tree => Boolean = {
-        case Tuple(es) => es.length == vars.length
+        case Tuple(es) => isTuplePattern(es.length)
         case _ => false
       }
+
+      // We can only optimize `val pat = if (...) e1 else e2` if:
+      // - `e1` and `e2` are both tuples of arity N
+      // - `pat` is a tuple of N variables or wildcard patterns like `(x1, x2, ..., xN)`
+      val tupleOptimizable = forallResults(rhs, isMatchingTuple)
+
+      val rhsUnchecked = makeAnnotated("scala.unchecked", rhs)
+      val vars =
+        if (tupleOptimizable) // include `_`
+          pat match {
+            case Tuple(pats) =>
+            pats.map { case id: Ident => id -> TypeTree() }
+          }
+        else getVariables(pat)  // no `_`
+
       val ids = for ((named, _) <- vars) yield Ident(named.name)
       val caseDef = CaseDef(pat, EmptyTree, makeTuple(ids))
       val matchExpr =
-        if (forallResults(rhs, isMatchingTuple)) rhs
+        if (tupleOptimizable) rhs
         else Match(rhsUnchecked, caseDef :: Nil)
       vars match {
         case Nil =>
@@ -938,7 +956,7 @@ object desugar {
               .withSpan(pat.span.union(rhs.span)).withMods(patMods)
           def selector(n: Int) = Select(Ident(tmpName), nme.selectorName(n))
           val restDefs =
-            for (((named, tpt), n) <- vars.zipWithIndex)
+            for (((named, tpt), n) <- vars.zipWithIndex if named.name != nme.WILDCARD)
             yield
               if (mods is Lazy) derivedDefDef(original, named, tpt, selector(n), mods &~ Lazy)
               else derivedValDef(original, named, tpt, selector(n), mods)
diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
index 8985088ff2fb..f988ab7b7cd1 100644
--- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
+++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
@@ -2405,7 +2405,7 @@ object Parsers {
         }
         makeConstructor(Nil, vparamss, rhs).withMods(mods).setComment(in.getDocComment(start))
       } else {
-        val (leadingParamss: List[List[ValDef]], flags: FlagSet) =
+        val (leadingParamss, flags) =
           if (in.token == LPAREN)
             (paramClause(prefix = true) :: Nil, Method | Extension)
           else
diff --git a/tests/neg/i4935.scala b/tests/neg/i4935.scala
new file mode 100644
index 000000000000..29ccd3cb8ec5
--- /dev/null
+++ b/tests/neg/i4935.scala
@@ -0,0 +1,3 @@
+object Foo {
+  val (A, B) = () // error // error
+}
diff --git a/tests/run/i4935.scala b/tests/run/i4935.scala
new file mode 100644
index 000000000000..6b3b1292c2ec
--- /dev/null
+++ b/tests/run/i4935.scala
@@ -0,0 +1,5 @@
+object Test {
+  val (Some(a), b) = (Some(3), 6)
+
+  def main(args: Array[String]) = assert(a == 3)
+}
diff --git a/tests/run/i4935b.scala b/tests/run/i4935b.scala
new file mode 100644
index 000000000000..b1260107292f
--- /dev/null
+++ b/tests/run/i4935b.scala
@@ -0,0 +1,5 @@
+object Test {
+  val (a, _, b) = (Some(3), 6, Some(9))
+
+  def main(args: Array[String]) = assert(a == Some(3) && b == Some(9))
+}

From 1e921b2941481caeb4c60788107cdcf35d0a0a1e Mon Sep 17 00:00:00 2001
From: Liu Fengyun <liu@fengy.me>
Date: Tue, 16 Apr 2019 14:48:00 +0200
Subject: [PATCH 2/2] Address review

---
 compiler/src/dotty/tools/dotc/ast/Desugar.scala | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala
index c746cfd171d0..3b4fc8a97d13 100644
--- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala
+++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala
@@ -915,7 +915,7 @@ object desugar {
     case _ =>
       def isTuplePattern(arity: Int): Boolean = pat match {
         case Tuple(pats) if pats.size == arity =>
-          pats.forall(id => id.isInstanceOf[Ident] && isVarPattern(id))
+          pats.forall(isVarPattern)
         case _ => false
       }
       val isMatchingTuple: Tree => Boolean = {
@@ -928,7 +928,7 @@ object desugar {
       // - `pat` is a tuple of N variables or wildcard patterns like `(x1, x2, ..., xN)`
       val tupleOptimizable = forallResults(rhs, isMatchingTuple)
 
-      val rhsUnchecked = makeAnnotated("scala.unchecked", rhs)
+      def rhsUnchecked = makeAnnotated("scala.unchecked", rhs)
       val vars =
         if (tupleOptimizable) // include `_`
           pat match {