Skip to content
Prev Previous commit
Next Next commit
fix false warning about Nil
scala.Nil refines scala.collection.immutable.Nil, this commit
follows redefinition of case objects.
  • Loading branch information
liufengyun committed Jul 5, 2016
commit fd1b28689b14e6ec3627324c1247184841f47e98
4 changes: 1 addition & 3 deletions src/dotty/tools/dotc/transform/PatternMatcher.scala
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,11 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
private var _id = 0 // left for debuging

override def transformMatch(tree: Match)(implicit ctx: Context, info: TransformerInfo): Tree = {
val Match(sel, cases) = tree

val translated = new Translator()(ctx).translator.translateMatch(tree)

// check exhaustivity and unreachability
val engine = new SpaceEngine
Copy link
Contributor

Choose a reason for hiding this comment

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

PatternMatcher now has a 🌠SpaceEngine🌠 and can do interstellar travels. Looking forward to battle space invaders.
👾👾👾👾

if (engine.checkable(sel.tpe.widen.elimAnonymousClass)) {
if (engine.checkable(tree)) {
engine.checkExhaustivity(tree)
engine.checkRedundancy(tree)
}
Expand Down
50 changes: 29 additions & 21 deletions src/dotty/tools/dotc/transform/patmat/Space.scala
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ trait SpaceLogic {
ss.exists(subspace(a, _)) ||
(canDecompose(tp1) && subspace(Or(partitions(tp1)), b))
case (Typ(tp1, _), Kon(tp2, ss)) =>
isSubType(tp1, tp2) && subspace(Kon(tp2, signature(tp2).map(tp => Typ(tp, false))), b)
isSubType(tp1, tp2) && subspace(Kon(tp2, signature(tp2).map(Typ(_, false))), b)
case (Kon(tp1, ss), Typ(tp2, _)) =>
isSubType(tp1, tp2) ||
simplify(a) == Empty ||
Expand Down Expand Up @@ -188,7 +188,7 @@ trait SpaceLogic {
minus(Or(partitions(tp1)), b)
else a
Copy link
Contributor

@DarkDimius DarkDimius Jul 1, 2016

Choose a reason for hiding this comment

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

See counterexample above.
Maybe it may make sence to try to decompose a here and see if some of results of decomposition after minus b do change.

case (Typ(tp1, _), Kon(tp2, ss)) =>
if (isSubType(tp1, tp2)) minus(Kon(tp2, signature(tp2).map(tp => Typ(tp, false))), b)
if (isSubType(tp1, tp2)) minus(Kon(tp2, signature(tp2).map(Typ(_, false))), b)
Copy link
Contributor

Choose a reason for hiding this comment

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

this case seems to be wrong: a - b == fun(b) - fun(b), the implementation does not take into account a.
offtopic: https://www.kirupa.com/forum/showthread.php?328247-Are-there-names-for-x-and-y-in-x-y
but maybe this is a corner case. I propose to at least document it.

else if (isSubType(tp2, tp1) && canDecompose(tp1))
minus(Or(partitions(tp1)), b)
else a
Expand Down Expand Up @@ -242,15 +242,17 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
def project(pat: Tree, roundUp: Boolean = true)(implicit ctx: Context): Space = pat match {
case Literal(c) => Const(c, c.tpe)
case _: BackquotedIdent => Var(pat.symbol, pat.tpe)
case Ident(_) =>
Typ(pat.tpe.stripAnnots, false)
case Select(_, _) =>
if (pat.symbol.is(Module))
Typ(pat.tpe.stripAnnots, false)
else if (pat.symbol.is(Enum))
Const(Constant(pat.symbol), pat.tpe)
else
Var(pat.symbol, pat.tpe)
case Ident(_) | Select(_, _) =>
pat.tpe.stripAnnots match {
case tp: TermRef =>
if (pat.symbol.is(Enum))
Const(Constant(pat.symbol), tp)
else if (tp.underlyingIterator.exists(_.classSymbol.is(Module)))
Typ(tp.widenTermRefExpr.stripAnnots, false)
else
Var(pat.symbol, tp)
case tp => Typ(tp, false)
}
case Alternative(trees) => Or(trees.map(project(_, roundUp)))
case Bind(_, pat) => project(pat)
case UnApply(_, _, pats) =>
Expand Down Expand Up @@ -295,6 +297,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
ktor
}

// refine path-dependent type in params. refer to t9672
meth.firstParamTypes.map(_.stripTypeVar).map(refine(tp, _))
}

Expand Down Expand Up @@ -415,37 +418,42 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
if (mergeList) "_" else "List(_)"
else if (tp.classSymbol.is(CaseClass))
// use constructor syntax for case class
showType(tp) + signature(tp).map(_ => "_").mkString("(", ",", ")")
showType(tp) + signature(tp).map(_ => "_").mkString("(", ", ", ")")
else if (signature(tp).nonEmpty)
tp.classSymbol.name + signature(tp).map(_ => "_").mkString("(", ",", ")")
tp.classSymbol.name + signature(tp).map(_ => "_").mkString("(", ", ", ")")
else if (decomposed) "_: " + showType(tp)
else "_"
case Kon(tp, params) =>
if (ctx.definitions.isTupleType(tp))
"(" + params.map(p => doShow(p)).mkString(", ") + ")"
"(" + params.map(doShow(_)).mkString(", ") + ")"
else if (tp.widen.classSymbol.showFullName == "scala.collection.immutable.::")
if (mergeList) params.map(p => doShow(p, mergeList)).mkString(", ")
else params.map(p => doShow(p, true)).mkString("List(", ", ", ")")
if (mergeList) params.map(doShow(_, mergeList)).mkString(", ")
else params.map(doShow(_, true)).filter(_ != "Nil").mkString("List(", ", ", ")")
else
showType(tp) + params.map(p => doShow(p)).mkString("(", ", ", ")")
showType(tp) + params.map(doShow(_)).mkString("(", ", ", ")")
case Or(_) =>
throw new Exception("incorrect flatten result " + s)
}

flatten(s).map(doShow(_, false)).distinct.mkString(", ")
}

def checkable(tp: Type): Boolean = tp match {
case AnnotatedType(tp, annot) =>
(ctx.definitions.UncheckedAnnot != annot.symbol) && checkable(tp)
case _ => true // actually everything is checkable unless @unchecked
def checkable(tree: Match): Boolean = {
def isCheckable(tp: Type): Boolean = tp match {
case AnnotatedType(tp, annot) =>
(ctx.definitions.UncheckedAnnot != annot.symbol) && isCheckable(tp)
case _ => true // actually everything is checkable unless @unchecked

Copy link
Contributor

Choose a reason for hiding this comment

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

scalac only checks sealed classes. We should consider if we indeed want to change this.
I'll create an issue about this.

// tp.classSymbol.is(Sealed) ||
// tp.isInstanceOf[OrType] ||
// tp.classSymbol.is(Enum) ||
// Boolean
// Int
// ...
}

val Match(sel, cases) = tree
isCheckable(sel.tpe.widen.elimAnonymousClass)
}

def checkExhaustivity(_match: Match): Unit = {
Expand Down
6 changes: 5 additions & 1 deletion tests/patmat/enum/expected.check
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@
It would fail on the following input: SATURDAY, FRIDAY, THURSDAY, SUNDAY
day match {
^
one warning found
./tests/patmat/enum/patmat-enum.scala:15: warning: match may not be exhaustive.
It would fail on the following input: SATURDAY, FRIDAY, THURSDAY
day match {
^
two warnings found
14 changes: 13 additions & 1 deletion tests/patmat/enum/patmat-enum.scala
Original file line number Diff line number Diff line change
@@ -1,9 +1,21 @@
object Test {
object Test1 {
val day: Day = ???

day match {
case Day.MONDAY => true
case Day.TUESDAY => true
case Day.WEDNESDAY => true
}
}

object Test2 {
import Day._
val day: Day = ???

day match {
case MONDAY => true
case TUESDAY => true
case WEDNESDAY => true
case SUNDAY => true
}
}
5 changes: 5 additions & 0 deletions tests/patmat/patmat-adt.scala
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,9 @@ object PatmatADT {
def foo5(tree: Tree) : Any = tree match {
case LetL(CharLit) =>
}

def foo6[T](l: List[T]): Boolean = l match {
case x::xs => true
case Nil => false
}
}
13 changes: 13 additions & 0 deletions tests/patmat/patmat-indent.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
./tests/patmat/patmat-indent.scala:9: warning: match may not be exhaustive.
It would fail on the following input: Nil
def foo1a[T](l: List[T]) = l match {
^
./tests/patmat/patmat-indent.scala:23: warning: match may not be exhaustive.
It would fail on the following input: _: Boolean
def foo2(b: Boolean) = b match {
^
./tests/patmat/patmat-indent.scala:27: warning: match may not be exhaustive.
It would fail on the following input: _: Int
def foo3(x: Int) = x match {
^
three warnings found
30 changes: 30 additions & 0 deletions tests/patmat/patmat-indent.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
object Test {
val Nil = scala.Nil
val X = 5

object Inner {
val Y = false
}

def foo1a[T](l: List[T]) = l match {
case x::xs => false
}

def foo1b[T](l: List[T]) = l match {
case Nil => true
case x::xs => false
}

def foo1c[T](l: List[T]) = l match {
case Test.Nil => true
case x::xs => false
}

def foo2(b: Boolean) = b match {
case Inner.Y => false
}

def foo3(x: Int) = x match {
case X => 0
}
}
2 changes: 1 addition & 1 deletion tests/patmat/t7466.check
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
./tests/patmat/t7466.scala:8: warning: match may not be exhaustive.
It would fail on the following input: (true, _), (false, _), (_, true), (_, false)
It would fail on the following input: (_, _)
(b1, b2) match {
^
one warning found
27 changes: 27 additions & 0 deletions tests/patmat/t9411a.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
object OhNoes {

sealed trait F
sealed abstract class FA extends F
sealed abstract class FB extends F

case object FA1 extends FA
case object FB1 extends FB
case object FB2 extends FB

sealed trait G
case object G1 extends G
case object G2 extends G

sealed trait H
case class H1(a: FB, b: G) extends H
case class H2(a: F) extends H

val demo: H => Unit = {
case H1(FB1, G1) =>
case H1(FB2, G2) =>
case H2(_: FB) =>
case H2(_: FA) =>
case H1(FB1, G2) =>
case H1(FB2, G1) =>
}
}
36 changes: 36 additions & 0 deletions tests/patmat/t9411b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
object OhNoes {

sealed trait F
sealed abstract class FA extends F
sealed abstract class FB extends F

case object FA1 extends FA
case object FB1 extends FB
case object FB2 extends FB

sealed trait G
case object G1 extends G
case object G2 extends G

sealed trait H
case class H1(a: FB, b: G) extends H
case class H2(b: F) extends H

val demo: H => Unit = {
case H1(FB1, G1) =>
case H1(FB2, G2) =>
case H2(_: FB) =>
case H2(_: FA) =>
case H1(FB1, G2) =>
case H1(FB2, G1) =>
}

val demo2: H => Unit = {
case H2(_: FA) =>
case H2(_: FB) =>
case H1(FB1, G1) =>
case H1(FB2, G1) =>
case H1(FB1, G2) =>
case H1(FB2, G2) =>
}
}
5 changes: 5 additions & 0 deletions tests/patmat/virtpatmat_apply.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
./tests/patmat/virtpatmat_apply.scala:2: warning: match may not be exhaustive.
It would fail on the following input: List(_)
List(1, 2, 3) match {
^
one warning found
7 changes: 7 additions & 0 deletions tests/patmat/virtpatmat_apply.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
object Test {
List(1, 2, 3) match {
case Nil => println("FAIL")
case x :: y :: xs if xs.length == 2 => println("FAIL")
case x :: y :: xs if xs.length == 1 => println("OK "+ y)
}
}