Skip to content

Fix #6849: support irrefutable sequence match #6850

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jan 16, 2020
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 5 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand Down Expand Up @@ -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 = {
Expand Down
60 changes: 41 additions & 19 deletions compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand All @@ -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 = {
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,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)
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/test/dotc/run-test-pickling.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ macros-in-same-project1
i5257.scala
i7868.scala
enum-java
zero-arity-case-class.scala
tuple-ops.scala
13 changes: 13 additions & 0 deletions docs/docs/reference/changed-features/pattern-matching.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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`
Expand Down
1 change: 1 addition & 0 deletions tests/patmat/i6490.check
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
8: Pattern Match Exhaustivity: Foo()
10 changes: 10 additions & 0 deletions tests/patmat/i6490.scala
Original file line number Diff line number Diff line change
@@ -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)
}
2 changes: 2 additions & 0 deletions tests/patmat/irrefutable.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
22: Pattern Match Exhaustivity: _: Base
65: Pattern Match Exhaustivity: _: M
85 changes: 85 additions & 0 deletions tests/patmat/irrefutable.scala
Original file line number Diff line number Diff line change
@@ -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) =>
}
}
4 changes: 2 additions & 2 deletions tests/patmat/t9232.check
Original file line number Diff line number Diff line change
@@ -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()
14 changes: 14 additions & 0 deletions tests/pos/i6849a.scala
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
5 changes: 5 additions & 0 deletions tests/pos/i6849b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object A {
def unapplySeq(a: Any): Seq[_] = ""
}

def unapply(x: Any) = x match { case A() => }
2 changes: 1 addition & 1 deletion tests/run-macros/tasty-extractors-2.check
Original file line number Diff line number Diff line change
Expand Up @@ -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(()))))
Expand Down
14 changes: 14 additions & 0 deletions tests/run/i6490.scala
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
14 changes: 14 additions & 0 deletions tests/run/i6490b.scala
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
11 changes: 6 additions & 5 deletions tests/run/string-extractor.scala
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should add extra tests rather than modify the old ones

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We added extra tests in other files. This test has to be adapted, as it does not conform to the protocol: the result type conforms to sequence match, thus it is irrefutable.

}

object Test {
Expand Down