Merge pull request #6850 from dotty-staging/fix-6490

nicolasstucki · web-flow · commit 376f5b0d8320 · 2020-01-16T15:53:22.000+01:00
Fix #6849: support irrefutable sequence match
diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala
@@ -715,7 +715,8 @@ object desugar {
           val methName = if (hasRepeatedParam) nme.unapplySeq else nme.unapply
           val unapplyParam = makeSyntheticParameter(tpt = classTypeRef)
           val unapplyRHS = if (arity == 0) Literal(Constant(true)) else Ident(unapplyParam.name)
-          DefDef(methName, derivedTparams, (unapplyParam :: Nil) :: Nil, TypeTree(), unapplyRHS)
+          val unapplyResTp = if (arity == 0) Literal(Constant(true)) else TypeTree()
+          DefDef(methName, derivedTparams, (unapplyParam :: Nil) :: Nil, unapplyResTp, unapplyRHS)
             .withMods(synthetic)
         }
         companionDefs(companionParent, applyMeths ::: unapplyMeth :: companionMembers)
diff --git a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala
@@ -8,7 +8,7 @@ import collection.mutable
 import Symbols._, Contexts._, Types._, StdNames._, NameOps._
 import ast.Trees._
 import util.Spans._
-import typer.Applications.{isProductMatch, isGetMatch, isProductSeqMatch, productSelectors, productArity}
+import typer.Applications.{isProductMatch, isGetMatch, isProductSeqMatch, productSelectors, productArity, unapplySeqTypeElemTp}
 import SymUtils._
 import Flags._, Constants._
 import Decorators._
@@ -329,10 +329,13 @@ object PatternMatcher {
                 .map(ref(unappResult).select(_))
               matchArgsPlan(selectors, args, onSuccess)
             }
-            else if (isProductSeqMatch(unapp.tpe.widen, args.length, unapp.sourcePos) && isUnapplySeq) {
+            else if (isUnapplySeq && isProductSeqMatch(unapp.tpe.widen, args.length, unapp.sourcePos)) {
               val arity = productArity(unapp.tpe.widen, unapp.sourcePos)
               unapplyProductSeqPlan(unappResult, args, arity)
             }
+            else if (isUnapplySeq && unapplySeqTypeElemTp(unapp.tpe.widen.finalResultType).exists) {
+              unapplySeqPlan(unappResult, args)
+            }
             else {
               assert(isGetMatch(unapp.tpe))
               val argsPlan = {
diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
@@ -275,12 +275,30 @@ object SpaceEngine {
   /** Is the unapply irrefutable?
    *  @param  unapp   The unapply function reference
    */
-  def isIrrefutableUnapply(unapp: tpd.Tree)(implicit ctx: Context): Boolean = {
+  def isIrrefutableUnapply(unapp: tpd.Tree, patSize: Int)(implicit ctx: Context): Boolean = {
     val unappResult = unapp.tpe.widen.finalResultType
     unappResult.isRef(defn.SomeClass) ||
-    unappResult =:= ConstantType(Constant(true)) ||
-    (unapp.symbol.is(Synthetic) && unapp.symbol.owner.linkedClass.is(Case)) ||
-    productArity(unappResult) > 0
+    unappResult <:< ConstantType(Constant(true)) ||
+    (unapp.symbol.is(Synthetic) && unapp.symbol.owner.linkedClass.is(Case)) ||  // scala2 compatibility
+    (patSize != -1 && productArity(unappResult) == patSize) || {
+      val isEmptyTp = extractorMemberType(unappResult, nme.isEmpty, unapp.sourcePos)
+      isEmptyTp <:< ConstantType(Constant(false))
+    }
+  }
+
+  /** Is the unapplySeq irrefutable?
+   *  @param  unapp   The unapplySeq function reference
+   */
+  def isIrrefutableUnapplySeq(unapp: tpd.Tree, patSize: Int)(implicit ctx: Context): Boolean = {
+    val unappResult = unapp.tpe.widen.finalResultType
+    unappResult.isRef(defn.SomeClass) ||
+    (unapp.symbol.is(Synthetic) && unapp.symbol.owner.linkedClass.is(Case)) ||  // scala2 compatibility
+    unapplySeqTypeElemTp(unappResult).exists ||
+    isProductSeqMatch(unappResult, patSize) ||
+    {
+      val isEmptyTp = extractorMemberType(unappResult, nme.isEmpty, unapp.sourcePos)
+      isEmptyTp <:< ConstantType(Constant(false))
+    }
   }
 }
 
@@ -348,16 +366,15 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
         else {
           val (arity, elemTp, resultTp) = unapplySeqInfo(fun.tpe.widen.finalResultType, fun.sourcePos)
           if (elemTp.exists)
-            Prod(erase(pat.tpe.stripAnnots), fun.tpe, fun.symbol, projectSeq(pats) :: Nil, isIrrefutableUnapply(fun))
+            Prod(erase(pat.tpe.stripAnnots), fun.tpe, fun.symbol, projectSeq(pats) :: Nil, isIrrefutableUnapplySeq(fun, pats.size))
           else
-            Prod(erase(pat.tpe.stripAnnots), fun.tpe, fun.symbol, pats.take(arity - 1).map(project) :+ projectSeq(pats.drop(arity - 1)),isIrrefutableUnapply(fun))
+            Prod(erase(pat.tpe.stripAnnots), fun.tpe, fun.symbol, pats.take(arity - 1).map(project) :+ projectSeq(pats.drop(arity - 1)), isIrrefutableUnapplySeq(fun, pats.size))
         }
       else
-        Prod(erase(pat.tpe.stripAnnots), erase(fun.tpe), fun.symbol, pats.map(project), isIrrefutableUnapply(fun))
-    case Typed(expr @ Ident(nme.WILDCARD), tpt) =>
+        Prod(erase(pat.tpe.stripAnnots), erase(fun.tpe), fun.symbol, pats.map(project), isIrrefutableUnapply(fun, pats.length))
+    case Typed(pat @ UnApply(_, _, _), _) => project(pat)
+    case Typed(expr, _) =>
       Typ(erase(expr.tpe.stripAnnots), true)
-    case Typed(pat, _) =>
-      project(pat)
     case This(_) =>
       Typ(pat.tpe.stripAnnots, false)
     case EmptyTree =>         // default rethrow clause of try/catch, check tests/patmat/try2.scala
@@ -678,19 +695,21 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
         companion.findMember(nme.unapplySeq, NoPrefix, required = EmptyFlags, excluded = Synthetic).exists
     }
 
-    def doShow(s: Space, mergeList: Boolean = false): String = s match {
+    def doShow(s: Space, flattenList: Boolean = false): String = s match {
       case Empty => ""
       case Typ(c: ConstantType, _) => "" + c.value.value
-      case Typ(tp: TermRef, _) => tp.symbol.showName
+      case Typ(tp: TermRef, _) =>
+        if (flattenList && tp <:< scalaNilType) ""
+        else tp.symbol.showName
       case Typ(tp, decomposed) =>
         val sym = tp.widen.classSymbol
 
         if (ctx.definitions.isTupleType(tp))
           params(tp).map(_ => "_").mkString("(", ", ", ")")
         else if (scalaListType.isRef(sym))
-          if (mergeList) "_: _*" else "_: List"
+          if (flattenList) "_: _*" else "_: List"
         else if (scalaConsType.isRef(sym))
-          if (mergeList) "_, _: _*"  else "List(_, _: _*)"
+          if (flattenList) "_, _: _*"  else "List(_, _: _*)"
         else if (tp.classSymbol.is(Sealed) && tp.classSymbol.hasAnonymousChild)
           "_: " + showType(tp) + " (anonymous)"
         else if (tp.classSymbol.is(CaseClass) && !hasCustomUnapply(tp.classSymbol))
@@ -702,15 +721,18 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
         if (ctx.definitions.isTupleType(tp))
           "(" + params.map(doShow(_)).mkString(", ") + ")"
         else if (tp.isRef(scalaConsType.symbol))
-          if (mergeList) params.map(doShow(_, mergeList)).mkString(", ")
-          else params.map(doShow(_, true)).filter(_ != "Nil").mkString("List(", ", ", ")")
-        else
-          showType(sym.owner.typeRef) + params.map(doShow(_)).mkString("(", ", ", ")")
+          if (flattenList) params.map(doShow(_, flattenList)).mkString(", ")
+          else params.map(doShow(_, flattenList = true)).filter(!_.isEmpty).mkString("List(", ", ", ")")
+        else {
+          val isUnapplySeq = sym.name.eq(nme.unapplySeq)
+          val paramsStr = params.map(doShow(_, flattenList = isUnapplySeq)).mkString("(", ", ", ")")
+          showType(sym.owner.typeRef) + paramsStr
+        }
       case Or(_) =>
         throw new Exception("incorrect flatten result " + s)
     }
 
-    flatten(s).map(doShow(_, false)).distinct.mkString(", ")
+    flatten(s).map(doShow(_, flattenList = false)).distinct.mkString(", ")
   }
 
   private def exhaustivityCheckable(sel: Tree): Boolean = {
diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala
@@ -71,7 +71,7 @@ object Applications {
    *  parameterless `isEmpty` member of result type `Boolean`.
    */
   def isGetMatch(tp: Type, errorPos: SourcePosition = NoSourcePosition)(implicit ctx: Context): Boolean =
-    extractorMemberType(tp, nme.isEmpty, errorPos).isRef(defn.BooleanClass) &&
+    extractorMemberType(tp, nme.isEmpty, errorPos).widenSingleton.isRef(defn.BooleanClass) &&
     extractorMemberType(tp, nme.get, errorPos).exists
 
   /** If `getType` is of the form:
diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala
@@ -643,7 +643,7 @@ trait Checking {
             recur(pat1, pt)
           case UnApply(fn, _, pats) =>
             check(pat, pt) &&
-            (isIrrefutableUnapply(fn) || fail(pat, pt)) && {
+            (isIrrefutableUnapply(fn, pats.length) || fail(pat, pt)) && {
               val argPts = unapplyArgs(fn.tpe.widen.finalResultType, fn, pats, pat.sourcePos)
               pats.corresponds(argPts)(recur)
             }
diff --git a/compiler/test/dotc/run-test-pickling.blacklist b/compiler/test/dotc/run-test-pickling.blacklist
@@ -24,3 +24,5 @@ macros-in-same-project1
 i5257.scala
 i7868.scala
 enum-java
+zero-arity-case-class.scala
+tuple-ops.scala
diff --git a/docs/docs/reference/changed-features/pattern-matching.md b/docs/docs/reference/changed-features/pattern-matching.md
@@ -50,6 +50,13 @@ and `S` conforms to one of the following matches:
 The former form of `unapply` has higher precedence, and _single match_ has higher
 precedence over _name-based match_.
 
+A usage of a fixed-arity extractor is irrefutable if one of the following condition holds:
+
+- `U = true`
+- the extractor is used as a product match
+- `U = Some[T]` (for Scala2 compatibility)
+- `U <: R` and `U <: { def isEmpty: false }`
+
 ### Variadic Extractors
 
 Variadic extractors expose the following signature:
@@ -77,6 +84,12 @@ and `S` conforms to one of the two matches above.
 The former form of `unapplySeq` has higher priority, and _sequence match_ has higher
 precedence over _product-sequence match_.
 
+A usage of a variadic extractor is irrefutable if one of the following condition holds:
+
+- the extractor is used directly as a sequence match or product-sequence match
+- `U = Some[T]` (for Scala2 compatibility)
+- `U <: R` and `U <: { def isEmpty: false }`
+
 ## Boolean Match
 
 - `U =:= Boolean`
diff --git a/tests/patmat/i6490.check b/tests/patmat/i6490.check
@@ -0,0 +1 @@
+8: Pattern Match Exhaustivity: Foo()
diff --git a/tests/patmat/i6490.scala b/tests/patmat/i6490.scala
@@ -0,0 +1,10 @@
+sealed class Foo(val value: Int)
+
+object Foo {
+  def unapplySeq(foo: Foo): Seq[Int] = List(foo.value)
+}
+
+def foo(x: Foo): Unit =
+  x match {
+    case Foo(x, _: _*) => assert(x == 3)
+  }
diff --git a/tests/patmat/irrefutable.check b/tests/patmat/irrefutable.check
@@ -0,0 +1,2 @@
+22: Pattern Match Exhaustivity: _: Base
+65: Pattern Match Exhaustivity: _: M
diff --git a/tests/patmat/irrefutable.scala b/tests/patmat/irrefutable.scala
@@ -0,0 +1,85 @@
+sealed trait Base
+sealed class A(s: String) extends Base
+sealed class B(val n: Int) extends Base
+sealed case class C(val s: String, val n: Int) extends Base
+
+// boolean
+object ExTrue {
+  def unapply(b: Base): true = b match {
+    case _ => true
+  }
+
+  def test(b: Base) = b match {
+    case ExTrue() =>
+  }
+}
+
+object ExFalse {
+  def unapply(b: Base): Boolean = b match {
+    case _ => true
+  }
+
+  def test(b: Base) = b match {  // warning
+    case ExFalse() =>
+  }
+}
+
+// product
+
+object ExProduct {
+  def unapply(b: B): (Int, Int) = (b.n, b.n * b.n)
+  def test(b: B) = b match {
+    case ExProduct(x, xx) =>
+  }
+}
+
+// isEmpty/get
+
+trait Res {
+  def isEmpty: false = false
+  def get: Int
+}
+
+object ExName {
+  def unapply(b: B): Res = new Res { def get: Int = b.n }
+
+  def test(b: B) = b match {
+    case ExName(x) =>
+  }
+}
+
+sealed class M(n: Int, s: String) extends Product {
+  def _1: Int = n
+  def _2: String = s
+  def isEmpty: Boolean = s.size > n
+  def get: M = this
+
+  def canEqual(that: Any): Boolean = true
+  def productArity: Int = 2
+  def productElement(n: Int): Any = ???
+}
+
+object ExM {
+  def unapply(m: M): M = m
+
+  def test(m: M) = m match {  // warning
+    case ExM(s) =>
+  }
+}
+
+// some
+object ExSome {
+  def unapply(b: B): Some[Int] = Some(b.n)
+
+  def test(b: B) = b match {
+    case ExSome(x) =>
+  }
+}
+
+object ExSome2 {
+  def unapply(c: C): Some[C] = Some(c)
+
+  def test(c: C) = c match {
+    case ExSome2(s, n) =>
+  }
+}
diff --git a/tests/patmat/t9232.check b/tests/patmat/t9232.check
@@ -1,3 +1,3 @@
 13: Pattern Match Exhaustivity: Node2()
-17: Pattern Match Exhaustivity: Node1(Foo(List(_, _: _*))), Node1(Foo(Nil)), Node2()
-21: Pattern Match Exhaustivity: Node1(Foo(Nil)), Node2()
+17: Pattern Match Exhaustivity: Node1(Foo(_, _: _*)), Node1(Foo()), Node2()
+21: Pattern Match Exhaustivity: Node1(Foo()), Node2()
diff --git a/tests/pos/i6849a.scala b/tests/pos/i6849a.scala
@@ -0,0 +1,14 @@
+final class Foo(val value: Int)
+
+object Foo {
+  def unapplySeq(foo: Foo): Seq[Int] = List(foo.value)
+}
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    (new Foo(3)) match {
+      case Foo(x, _: _*) =>
+        assert(x == 3)
+    }
+  }
+}
diff --git a/tests/pos/i6849b.scala b/tests/pos/i6849b.scala
@@ -0,0 +1,5 @@
+object A {
+  def unapplySeq(a: Any): Seq[_] = ""
+}
+
+def unapply(x: Any) = x match { case A() => }
diff --git a/tests/run-macros/tasty-extractors-2.check b/tests/run-macros/tasty-extractors-2.check
@@ -49,7 +49,7 @@ TypeRef(ThisType(TypeRef(NoPrefix(), "scala")), "Unit")
 Inlined(None, Nil, Block(List(ClassDef("Foo", DefDef("<init>", Nil, List(Nil), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil)), Nil, None, List(DefDef("a", Nil, Nil, Inferred(), Some(Literal(Constant(0))))))), Literal(Constant(()))))
 TypeRef(ThisType(TypeRef(NoPrefix(), "scala")), "Unit")
 
-Inlined(None, Nil, Block(List(ClassDef("Foo", DefDef("<init>", Nil, List(Nil), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil), TypeSelect(Select(Ident("_root_"), "scala"), "Product"), TypeSelect(Select(Ident("_root_"), "scala"), "Serializable")), Nil, None, List(DefDef("copy", Nil, List(Nil), Inferred(), Some(Apply(Select(New(Inferred()), "<init>"), Nil))))), ValDef("Foo", TypeIdent("Foo$"), Some(Apply(Select(New(TypeIdent("Foo$")), "<init>"), Nil))), ClassDef("Foo$", DefDef("<init>", Nil, List(Nil), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil), Applied(Inferred(), List(Inferred())), TypeSelect(Select(Ident("_root_"), "scala"), "Serializable")), Nil, Some(ValDef("_", Singleton(Ident("Foo")), None)), List(DefDef("apply", Nil, List(Nil), Inferred(), Some(Apply(Select(New(Inferred()), "<init>"), Nil))), DefDef("unapply", Nil, List(List(ValDef("x$1", Inferred(), None))), Inferred(), Some(Literal(Constant(true))))))), Literal(Constant(()))))
+Inlined(None, Nil, Block(List(ClassDef("Foo", DefDef("<init>", Nil, List(Nil), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil), TypeSelect(Select(Ident("_root_"), "scala"), "Product"), TypeSelect(Select(Ident("_root_"), "scala"), "Serializable")), Nil, None, List(DefDef("copy", Nil, List(Nil), Inferred(), Some(Apply(Select(New(Inferred()), "<init>"), Nil))))), ValDef("Foo", TypeIdent("Foo$"), Some(Apply(Select(New(TypeIdent("Foo$")), "<init>"), Nil))), ClassDef("Foo$", DefDef("<init>", Nil, List(Nil), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil), Applied(Inferred(), List(Inferred())), TypeSelect(Select(Ident("_root_"), "scala"), "Serializable")), Nil, Some(ValDef("_", Singleton(Ident("Foo")), None)), List(DefDef("apply", Nil, List(Nil), Inferred(), Some(Apply(Select(New(Inferred()), "<init>"), Nil))), DefDef("unapply", Nil, List(List(ValDef("x$1", Inferred(), None))), Singleton(Literal(Constant(true))), Some(Literal(Constant(true))))))), Literal(Constant(()))))
 TypeRef(ThisType(TypeRef(NoPrefix(), "scala")), "Unit")
 
 Inlined(None, Nil, Block(List(ClassDef("Foo1", DefDef("<init>", Nil, List(List(ValDef("a", TypeIdent("Int"), None))), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil)), Nil, None, List(ValDef("a", Inferred(), None)))), Literal(Constant(()))))
diff --git a/tests/run/i6490.scala b/tests/run/i6490.scala
@@ -0,0 +1,14 @@
+final class Foo(val value: Int)
+
+object Foo {
+  def unapplySeq(foo: Foo): Seq[Int] = List(foo.value)
+}
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    (new Foo(3)) match {
+      case Foo(x, _: _*) =>
+        assert(x == 3)
+    }
+  }
+}
diff --git a/tests/run/i6490b.scala b/tests/run/i6490b.scala
@@ -0,0 +1,14 @@
+final class Foo(val value: Int)
+
+object Foo {
+  def unapplySeq(foo: Foo): (Int, Seq[Int]) = (foo.value, Nil)
+}
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    (new Foo(3)) match {
+      case Foo(x, _: _*) =>
+        assert(x == 3)
+    }
+  }
+}
diff --git a/tests/run/string-extractor.scala b/tests/run/string-extractor.scala
@@ -1,6 +1,4 @@
 final class StringExtract(val s: String) extends AnyVal {
-  def isEmpty                     = (s eq null) || (s == "")
-  def get                         = this
   def length                      = s.length
   def lengthCompare(n: Int)       = s.length compare n
   def apply(idx: Int): Char       = s charAt idx
@@ -13,7 +11,6 @@ final class StringExtract(val s: String) extends AnyVal {
 }
 
 final class ThreeStringExtract(val s: String) extends AnyVal {
-  def isEmpty                             = (s eq null) || (s == "")
   def get: (List[Int], Double, Seq[Char]) = ((s.length :: Nil, s.length.toDouble, toSeq))
   def length                              = s.length
   def lengthCompare(n: Int)               = s.length compare n
@@ -28,10 +25,14 @@ final class ThreeStringExtract(val s: String) extends AnyVal {
 
 
 object Bippy {
-  def unapplySeq(x: Any): StringExtract = new StringExtract("" + x)
+  def unapplySeq(x: Any): Option[StringExtract] =
+    if ((x == null) || (x == "")) None
+    else Some(new StringExtract("" + x))
 }
 object TripleBippy {
-  def unapplySeq(x: Any): ThreeStringExtract = new ThreeStringExtract("" + x)
+  def unapplySeq(x: Any): Option[(List[Int], Double, Seq[Char])] =
+    if ((x == null) || (x == "")) then None
+    else Some(new ThreeStringExtract("" + x).get)
 }
 
 object Test {

Original file line number	Diff line number	Diff line change
`@@ -643,7 +643,7 @@ trait Checking {`
`643`	`643`	`recur(pat1, pt)`
`644`	`644`	`case UnApply(fn, _, pats) =>`
`645`	`645`	`check(pat, pt) &&`
`646`		`- (isIrrefutableUnapply(fn) \|\| fail(pat, pt)) && {`
	`646`	`+ (isIrrefutableUnapply(fn, pats.length) \|\| fail(pat, pt)) && {`
`647`	`647`	`val argPts = unapplyArgs(fn.tpe.widen.finalResultType, fn, pats, pat.sourcePos)`
`648`	`648`	`pats.corresponds(argPts)(recur)`
`649`	`649`	`}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+22: Pattern Match Exhaustivity: _: Base`
	`2`	`+65: Pattern Match Exhaustivity: _: M`