Skip to content

Commit

Permalink
Fix scala#4314: revert changes in scala#4299
Browse files Browse the repository at this point in the history
1. ApproximatingTypeMap produces Nothing, which is not what is needed.
2. Avoid blind erasure which lose information about the pattern, see tests/patmat/i4314b.scala
  • Loading branch information
liufengyun committed Apr 17, 2018
1 parent ac84047 commit 84cf261
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 34 deletions.
85 changes: 51 additions & 34 deletions compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ trait SpaceLogic {
case Prod(tp, fun, sym, spaces, full) =>
val sp = Prod(tp, fun, sym, spaces.map(simplify(_)), full)
if (sp.params.contains(Empty)) Empty
else if (canDecompose(tp) && decompose(tp).isEmpty) Empty
else sp
case Or(spaces) =>
val set = spaces.map(simplify(_)).flatMap {
Expand Down Expand Up @@ -349,18 +350,18 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
Empty
}

/* Erase a type binding according to erasure semantics in pattern matching */
def erase(tp: Type): Type = tp match {
case tp @ AppliedType(tycon, args) =>
if (tycon.isRef(defn.ArrayClass)) tp.derivedAppliedType(tycon, args.map(erase))
else tp.derivedAppliedType(tycon, args.map(t => WildcardType))
case OrType(tp1, tp2) =>
OrType(erase(tp1), erase(tp2))
case AndType(tp1, tp2) =>
AndType(erase(tp1), erase(tp2))
case tp @ RefinedType(parent, refinedName, _) if refinedName.isTermName => // see pos/dependent-extractors.scala
tp.derivedRefinedType(erase(parent), refinedName, WildcardType)
case _ => tp
/* Erase pattern bound types with WildcardType */
def erase(tp: Type) = {
def isPatternTypeSymbol(sym: Symbol) = !sym.isClass && sym.is(Case)

val map = new TypeMap {
def apply(tp: Type) = tp match {
case tref: TypeRef if isPatternTypeSymbol(tref.typeSymbol) => WildcardType(tref.underlying.bounds)
case _ => mapOver(tp)
}
}

map(tp)
}

/** Space of the pattern: unapplySeq(a, b, c: _*)
Expand All @@ -384,7 +385,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
/** Is `tp1` a subtype of `tp2`? */
def isSubType(tp1: Type, tp2: Type): Boolean = {
val res = (tp1 != nullType || tp2 == nullType) && tp1 <:< tp2
debug.println(s"${tp1.show} <:< ${tp2.show} = $res")
debug.println(s"${tp1} <:< ${tp2} = $res")
res
}

Expand Down Expand Up @@ -576,8 +577,8 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
noClassConflict &&
(!isSingleton(tp1) || tp1 <:< tp2) &&
(!isSingleton(tp2) || tp2 <:< tp1) &&
(!bases1.exists(_ is Final) || tp1 <:< tp2) &&
(!bases2.exists(_ is Final) || tp2 <:< tp1)
(!bases1.exists(_ is Final) || tp1 <:< maxTypeMap.apply(tp2)) &&
(!bases2.exists(_ is Final) || tp2 <:< maxTypeMap.apply(tp1))
}
case OrType(tp1, tp2) =>
recur(tp1) || recur(tp2)
Expand All @@ -596,6 +597,41 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
res
}

/** expose abstract type references to their bounds or tvars according to variance */
private class AbstractTypeMap(maximize: Boolean)(implicit ctx: Context) extends TypeMap {
def expose(lo: Type, hi: Type): Type =
if (variance == 0)
newTypeVar(TypeBounds(lo, hi))
else if (variance == 1)
if (maximize) hi else lo
else
if (maximize) lo else hi

def apply(tp: Type): Type = tp match {
case tp: TypeRef if tp.underlying.isInstanceOf[TypeBounds] =>
val lo = this(tp.info.loBound)
val hi = this(tp.info.hiBound)
// See tests/patmat/gadt.scala tests/patmat/exhausting.scala tests/patmat/t9657.scala
val exposed = expose(lo, hi)
debug.println(s"$tp exposed to =====> $exposed")
exposed

case AppliedType(tycon: TypeRef, args) if tycon.underlying.isInstanceOf[TypeBounds] =>
val args2 = args.map(this)
val lo = this(tycon.info.loBound).applyIfParameterized(args2)
val hi = this(tycon.info.hiBound).applyIfParameterized(args2)
val exposed = expose(lo, hi)
debug.println(s"$tp exposed to =====> $exposed")
exposed

case _ =>
mapOver(tp)
}
}

private def minTypeMap(implicit ctx: Context) = new AbstractTypeMap(maximize = false)
private def maxTypeMap(implicit ctx: Context) = new AbstractTypeMap(maximize = true)

/** Instantiate type `tp1` to be a subtype of `tp2`
*
* Return the instantiated type if type parameters and this type
Expand All @@ -605,25 +641,6 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
*
*/
def instantiate(tp1: NamedType, tp2: Type)(implicit ctx: Context): Type = {
// expose type param references to their bounds according to variance
class AbstractTypeMap(maximize: Boolean)(implicit ctx: Context) extends ApproximatingTypeMap {
variance = if (maximize) 1 else -1

def apply(tp: Type): Type = tp match {
case tp: TypeRef if tp.underlying.isInstanceOf[TypeBounds] =>
val lo = this(tp.info.loBound)
val hi = this(tp.info.hiBound)
// See tests/patmat/gadt.scala tests/patmat/exhausting.scala tests/patmat/t9657.scala
range(lo, hi)

case _ =>
mapOver(tp)
}
}

def minTypeMap(implicit ctx: Context) = new AbstractTypeMap(maximize = false)
def maxTypeMap(implicit ctx: Context) = new AbstractTypeMap(maximize = true)

// Fix subtype checking for child instantiation,
// such that `Foo(Test.this.foo) <:< Foo(Foo.this)`
// See tests/patmat/i3938.scala
Expand Down
10 changes: 10 additions & 0 deletions tests/patmat/i4314.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
sealed trait Foo[A]
case class One[A]() extends Foo[A]
sealed abstract case class Bar[A]() extends Foo[A]
class Two() extends Bar[String]

object Test {
def test(x: Foo[Int]) = x match {
case One() =>
}
}
1 change: 1 addition & 0 deletions tests/patmat/i4314b.check
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
9: Match case Unreachable
11 changes: 11 additions & 0 deletions tests/patmat/i4314b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
sealed trait Foo[A]
case class One[A]() extends Foo[A]
sealed abstract case class Bar[A]() extends Foo[A]
class Two() extends Bar[String]

object Test {
def test(x: Foo[Int]) = x match {
case One() =>
case Bar() =>
}
}

0 comments on commit 84cf261

Please sign in to comment.