From 84cf26190e518fe6b01ec41403ddfe6967bed091 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Tue, 17 Apr 2018 21:33:50 +0200 Subject: [PATCH 1/5] Fix #4314: revert changes in #4299 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 --- .../tools/dotc/transform/patmat/Space.scala | 85 +++++++++++-------- tests/patmat/i4314.scala | 10 +++ tests/patmat/i4314b.check | 1 + tests/patmat/i4314b.scala | 11 +++ 4 files changed, 73 insertions(+), 34 deletions(-) create mode 100644 tests/patmat/i4314.scala create mode 100644 tests/patmat/i4314b.check create mode 100644 tests/patmat/i4314b.scala diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 2cb690805a9c..c1b670ae4a33 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -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 { @@ -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: _*) @@ -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 } @@ -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) @@ -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 @@ -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 diff --git a/tests/patmat/i4314.scala b/tests/patmat/i4314.scala new file mode 100644 index 000000000000..d44304b3a916 --- /dev/null +++ b/tests/patmat/i4314.scala @@ -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() => + } +} diff --git a/tests/patmat/i4314b.check b/tests/patmat/i4314b.check new file mode 100644 index 000000000000..50890d03434b --- /dev/null +++ b/tests/patmat/i4314b.check @@ -0,0 +1 @@ +9: Match case Unreachable diff --git a/tests/patmat/i4314b.scala b/tests/patmat/i4314b.scala new file mode 100644 index 000000000000..f74e7d763e15 --- /dev/null +++ b/tests/patmat/i4314b.scala @@ -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() => + } +} From 54a7df8628990dc1814255b1e69f531e34470bc7 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Tue, 17 Apr 2018 22:05:32 +0200 Subject: [PATCH 2/5] fix test case t2425.scala Blind substitution of pattern bound type symbol with WildcardType will cause t2425.scala fail, as the following holds: Array[_] <:< Array[Array[_]] To walkaround the problem, we don't substitute pattern bound symbols that are immediate parameters to `Array`. --- compiler/src/dotty/tools/dotc/transform/patmat/Space.scala | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index c1b670ae4a33..3a80eb13ef50 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -356,7 +356,11 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { val map = new TypeMap { def apply(tp: Type) = tp match { - case tref: TypeRef if isPatternTypeSymbol(tref.typeSymbol) => WildcardType(tref.underlying.bounds) + case tp @ AppliedType(tycon, args) if tycon.isRef(defn.ArrayClass) => + // walkaround `Array[_] <:< Array[Array[_]]`, see tests/patmat/t2425.scala + tp.derivedAppliedType(tycon, args.map(mapOver)) + case tref: TypeRef if isPatternTypeSymbol(tref.typeSymbol) => + WildcardType(tref.underlying.bounds) case _ => mapOver(tp) } } From f9700b13b9f5b2378c00d8179b9bc8ef1738fb9a Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Tue, 17 Apr 2018 22:46:53 +0200 Subject: [PATCH 3/5] catch two more exhaustive issue in compiler --- compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 2 +- compiler/src/dotty/tools/dotc/core/Types.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 3064078ed403..76ee96c136fd 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1678,7 +1678,7 @@ object SymDenotations { case tp @ AppliedType(tycon, args) => val subsym = tycon.typeSymbol if (subsym eq symbol) tp - else tycon.typeParams match { + else (tycon.typeParams: @unchecked) match { case LambdaParam(_, _) :: _ => baseTypeOf(tp.superType) case tparams: List[Symbol @unchecked] => diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 01144c3ddbfd..acadbf8c6f52 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2674,7 +2674,7 @@ object Types { * either a list of type parameter symbols or a list of lambda parameters */ def integrate(tparams: List[ParamInfo], tp: Type)(implicit ctx: Context): Type = - tparams match { + (tparams: @unchecked) match { case LambdaParam(lam, _) :: _ => tp.subst(lam, this) case params: List[Symbol @unchecked] => tp.subst(params, paramRefs) } From d8139fc3a796fcadd057eae10d5c312017f73ec4 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Thu, 19 Apr 2018 22:23:48 +0200 Subject: [PATCH 4/5] address review Use TypeBounds instead of WildcardType. WildcardType has the following unexpected behavior: Array[?] <:< Array[Array[?]] --- compiler/src/dotty/tools/dotc/transform/patmat/Space.scala | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 3a80eb13ef50..662ba2e6a04f 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -356,11 +356,8 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { val map = new TypeMap { def apply(tp: Type) = tp match { - case tp @ AppliedType(tycon, args) if tycon.isRef(defn.ArrayClass) => - // walkaround `Array[_] <:< Array[Array[_]]`, see tests/patmat/t2425.scala - tp.derivedAppliedType(tycon, args.map(mapOver)) case tref: TypeRef if isPatternTypeSymbol(tref.typeSymbol) => - WildcardType(tref.underlying.bounds) + tref.underlying.bounds case _ => mapOver(tp) } } From 0dc0502208255144005b68db42bac7befc56f492 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Thu, 19 Apr 2018 23:02:24 +0200 Subject: [PATCH 5/5] fix CI --- .../{pos-special => neg-custom-args}/isInstanceOf/t2755.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename tests/{pos-special => neg-custom-args}/isInstanceOf/t2755.scala (93%) diff --git a/tests/pos-special/isInstanceOf/t2755.scala b/tests/neg-custom-args/isInstanceOf/t2755.scala similarity index 93% rename from tests/pos-special/isInstanceOf/t2755.scala rename to tests/neg-custom-args/isInstanceOf/t2755.scala index 8d10b567346b..9073e9253098 100644 --- a/tests/pos-special/isInstanceOf/t2755.scala +++ b/tests/neg-custom-args/isInstanceOf/t2755.scala @@ -17,7 +17,7 @@ object Test { case x: Array[String] => x.size case x: Array[AnyRef] => 5 case x: Array[_] => 6 - case _ => 7 + case _ => 7 // error: only null is matched } def f3[T](a: Array[T]) = a match { case x: Array[Int] => x(0) @@ -26,7 +26,7 @@ object Test { case x: Array[String] => x.size case x: Array[AnyRef] => 5 case x: Array[_] => 6 - case _ => 7 + case _ => 7 // error: only null is matched }