From dea54341590fb68c84efe15d8ecc667bc0cf4c00 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Wed, 8 Nov 2017 17:25:53 +0100 Subject: [PATCH 1/3] fix #3443: typeparam should expose according to variance --- .../tools/dotc/transform/patmat/Space.scala | 40 ++++++++++--------- tests/patmat/i3443.scala | 19 +++++++++ 2 files changed, 40 insertions(+), 19 deletions(-) create mode 100644 tests/patmat/i3443.scala diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index c25216e1c7a4..bf264d97b77f 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -606,20 +606,19 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { } } - val tvars = tp1.typeParams.map { tparam => newTypeVar(tparam.paramInfo.bounds) } - val protoTp1 = thisTypeMap(tp1.appliedTo(tvars)) - - // replace type parameter references with fresh type vars or bounds + // replace type parameter references with bounds val typeParamMap = new TypeMap { def apply(t: Type): Type = t match { case tp: TypeRef if tp.symbol.is(TypeParam) && tp.underlying.isInstanceOf[TypeBounds] => // See tests/patmat/gadt.scala tests/patmat/exhausting.scala - val bound = - if (variance == 0) tp.underlying.bounds // non-variant case is not well-founded - else if (variance == 1) TypeBounds.upper(tp) - else TypeBounds.lower(tp) - newTypeVar(bound) + val exposed = + if (variance == 0) newTypeVar(tp.underlying.bounds) + else if (variance == 1) expose(tp, true) + else expose(tp, false) + + debug.println(s"$tp exposed to =====> " + exposed) + exposed case tp: RefinedType if tp.refinedInfo.isInstanceOf[TypeBounds] => // Ideally, we would expect type inference to do the job // Check tests/patmat/t9657.scala @@ -629,6 +628,9 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { } } + val tvars = tp1.typeParams.map { tparam => newTypeVar(tparam.paramInfo.bounds) } + val protoTp1 = thisTypeMap(tp1.appliedTo(tvars)) + if (protoTp1 <:< tp2 && isFullyDefined(protoTp1, ForceDegree.noBottom)) protoTp1 else { val protoTp2 = typeParamMap(tp2) @@ -778,13 +780,13 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { * * A <: X :> Y B <: U :> V M { type T <: A :> B } ~~> M { type T <: X :> V } */ - def expose(tp: Type, refineCtx: Boolean = false, up: Boolean = true): Type = tp match { + def expose(tp: Type, up: Boolean = true): Type = tp match { case tp: AppliedType => - tp.derivedAppliedType(expose(tp.tycon, refineCtx, up), tp.args.map(expose(_, refineCtx, up))) + tp.derivedAppliedType(expose(tp.tycon, up), tp.args.map(expose(_, up))) case tp: TypeAlias => - val hi = expose(tp.alias, refineCtx, up) - val lo = expose(tp.alias, refineCtx, up) + val hi = expose(tp.alias, up) + val lo = expose(tp.alias, up) if (hi =:= lo) tp.derivedTypeAlias(hi) @@ -792,27 +794,27 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { tp.derivedTypeBounds(lo, hi) case tp @ TypeBounds(lo, hi) => - tp.derivedTypeBounds(expose(lo, refineCtx, false), expose(hi, refineCtx, true)) + tp.derivedTypeBounds(expose(lo, false), expose(hi, true)) case tp: RefinedType => tp.derivedRefinedType( expose(tp.parent), tp.refinedName, - expose(tp.refinedInfo, true, up) + expose(tp.refinedInfo, up) ) - case tp: TypeProxy if refineCtx => + case tp: TypeProxy => tp.underlying match { case TypeBounds(lo, hi) => - expose(if (up) hi else lo, refineCtx, up) + expose(if (up) hi else lo, up) case _ => tp } case OrType(tp1, tp2) => - OrType(expose(tp1, refineCtx, up), expose(tp2, refineCtx, up)) + OrType(expose(tp1, up), expose(tp2, up)) case AndType(tp1, tp2) => - AndType(expose(tp1, refineCtx, up), expose(tp2, refineCtx, up)) + AndType(expose(tp1, up), expose(tp2, up)) case _ => tp } diff --git a/tests/patmat/i3443.scala b/tests/patmat/i3443.scala new file mode 100644 index 000000000000..e5cc884cb2a2 --- /dev/null +++ b/tests/patmat/i3443.scala @@ -0,0 +1,19 @@ +object Test { + // shapeless.Coproduct + sealed trait Coproduct extends Product with Serializable + sealed trait :+:[+H, +T <: Coproduct] extends Coproduct + final case class Inl[+H, +T <: Coproduct](head : H) extends :+:[H, T] + final case class Inr[+H, +T <: Coproduct](tail : T) extends :+:[H, T] + sealed trait CNil extends Coproduct + + // Note that this only appears when T is a type parameter. Replaying T with + // a concrete type (CNil or another :+:) leads to accurate warnnings + def f[T <: Coproduct](fa: Int :+: T) = + fa match { + case Inl(x) => 1 + case Inr(x) => 2 // Dotty thinks this unreachable, but it is! + } + + def main(args: Array[String]): Unit = + assert(f(Inr(Inl(-1))) == 2) // Example of reachability +} \ No newline at end of file From 74f0c8f613ec0b9fd8d5e48f01c51577fe9996bf Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Wed, 8 Nov 2017 17:30:37 +0100 Subject: [PATCH 2/3] remove @unchecked from exhaustivity check --- .../tools/dotc/transform/patmat/Space.scala | 19 +++---------------- tests/patmat/3144.check | 2 -- tests/patmat/3144b.check | 2 -- tests/patmat/enum-HList.check | 1 - tests/patmat/enum-Tree.check | 1 - tests/patmat/t3683.check | 1 - tests/patmat/t3683a.check | 1 - 7 files changed, 3 insertions(+), 24 deletions(-) delete mode 100644 tests/patmat/3144.check delete mode 100644 tests/patmat/3144b.check delete mode 100644 tests/patmat/enum-HList.check delete mode 100644 tests/patmat/enum-Tree.check delete mode 100644 tests/patmat/t3683.check diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index bf264d97b77f..c8a9152b9a46 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -405,35 +405,22 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { Prod(pat.tpe.stripAnnots, fun.tpe.widen, fun.symbol, pats.map(project), irrefutable(fun)) case Typed(pat @ UnApply(_, _, _), _) => project(pat) case Typed(expr, tpt) => - val unchecked = expr.tpe.hasAnnotation(ctx.definitions.UncheckedAnnot) - def warn(msg: String): Unit = if (!unchecked) ctx.warning(UncheckedTypePattern(msg), tpt.pos) - Typ(erase(expr.tpe.stripAnnots)(warn), true) + Typ(erase(expr.tpe.stripAnnots), true) case _ => debug.println(s"unknown pattern: $pat") Empty } /* Erase a type binding according to erasure semantics in pattern matching */ - def erase(tp: Type)(implicit warn: String => Unit): Type = tp match { + def erase(tp: Type): Type = tp match { case tp @ AppliedType(tycon, args) => if (tycon.isRef(defn.ArrayClass)) tp.derivedAppliedType(tycon, args.map(erase)) - else { - val ignoreWarning = args.forall { p => - p.typeSymbol.is(BindDefinedType) || - p.hasAnnotation(defn.UncheckedAnnot) || - p.isInstanceOf[TypeBounds] - } - if (!ignoreWarning) - warn("type arguments are not checked since they are eliminated by erasure") - - tp.derivedAppliedType(tycon, args.map(t => WildcardType)) - } + 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 => - warn("type refinement is not checked since it is eliminated by erasure") tp.derivedRefinedType(erase(tp.parent), tp.refinedName, WildcardType) case _ => tp } diff --git a/tests/patmat/3144.check b/tests/patmat/3144.check deleted file mode 100644 index 207dd36e1d9a..000000000000 --- a/tests/patmat/3144.check +++ /dev/null @@ -1,2 +0,0 @@ -2: Pattern Match Exhaustivity -7: Pattern Match Exhaustivity diff --git a/tests/patmat/3144b.check b/tests/patmat/3144b.check deleted file mode 100644 index d4bd280466da..000000000000 --- a/tests/patmat/3144b.check +++ /dev/null @@ -1,2 +0,0 @@ -4: Pattern Match Exhaustivity -10: Pattern Match Exhaustivity diff --git a/tests/patmat/enum-HList.check b/tests/patmat/enum-HList.check deleted file mode 100644 index 414335be5ea7..000000000000 --- a/tests/patmat/enum-HList.check +++ /dev/null @@ -1 +0,0 @@ -2: Pattern Match Exhaustivity diff --git a/tests/patmat/enum-Tree.check b/tests/patmat/enum-Tree.check deleted file mode 100644 index 84053e20646d..000000000000 --- a/tests/patmat/enum-Tree.check +++ /dev/null @@ -1 +0,0 @@ -8: Pattern Match Exhaustivity \ No newline at end of file diff --git a/tests/patmat/t3683.check b/tests/patmat/t3683.check deleted file mode 100644 index 2d090c590d8c..000000000000 --- a/tests/patmat/t3683.check +++ /dev/null @@ -1 +0,0 @@ -7: Pattern Match Exhaustivity \ No newline at end of file diff --git a/tests/patmat/t3683a.check b/tests/patmat/t3683a.check index c39820a1a0be..fdcafc5961b9 100644 --- a/tests/patmat/t3683a.check +++ b/tests/patmat/t3683a.check @@ -1,2 +1 @@ -8: Pattern Match Exhaustivity 14: Pattern Match Exhaustivity: XX() From 253f5059b22d090e883d2fd4d86b7c1ddaaf2fe5 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Wed, 8 Nov 2017 18:17:35 +0100 Subject: [PATCH 3/3] address review and remove expose completely --- .../tools/dotc/transform/patmat/Space.scala | 57 ++----------------- 1 file changed, 4 insertions(+), 53 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index c8a9152b9a46..ee17927a2598 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -598,18 +598,14 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { def apply(t: Type): Type = t match { case tp: TypeRef if tp.symbol.is(TypeParam) && tp.underlying.isInstanceOf[TypeBounds] => - // See tests/patmat/gadt.scala tests/patmat/exhausting.scala + // See tests/patmat/gadt.scala tests/patmat/exhausting.scala tests/patmat/t9657.scala val exposed = if (variance == 0) newTypeVar(tp.underlying.bounds) - else if (variance == 1) expose(tp, true) - else expose(tp, false) + else if (variance == 1) mapOver(tp.underlying.hiBound) + else mapOver(tp.underlying.loBound) - debug.println(s"$tp exposed to =====> " + exposed) + debug.println(s"$tp exposed to =====> $exposed") exposed - case tp: RefinedType if tp.refinedInfo.isInstanceOf[TypeBounds] => - // Ideally, we would expect type inference to do the job - // Check tests/patmat/t9657.scala - expose(tp) case _ => mapOver(t) } @@ -762,51 +758,6 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { res } - - /** Eliminate reference to type parameters in refinements - * - * A <: X :> Y B <: U :> V M { type T <: A :> B } ~~> M { type T <: X :> V } - */ - def expose(tp: Type, up: Boolean = true): Type = tp match { - case tp: AppliedType => - tp.derivedAppliedType(expose(tp.tycon, up), tp.args.map(expose(_, up))) - - case tp: TypeAlias => - val hi = expose(tp.alias, up) - val lo = expose(tp.alias, up) - - if (hi =:= lo) - tp.derivedTypeAlias(hi) - else - tp.derivedTypeBounds(lo, hi) - - case tp @ TypeBounds(lo, hi) => - tp.derivedTypeBounds(expose(lo, false), expose(hi, true)) - - case tp: RefinedType => - tp.derivedRefinedType( - expose(tp.parent), - tp.refinedName, - expose(tp.refinedInfo, up) - ) - case tp: TypeProxy => - tp.underlying match { - case TypeBounds(lo, hi) => - expose(if (up) hi else lo, up) - case _ => - tp - } - - case OrType(tp1, tp2) => - OrType(expose(tp1, up), expose(tp2, up)) - - case AndType(tp1, tp2) => - AndType(expose(tp1, up), expose(tp2, up)) - - case _ => tp - } - - def checkExhaustivity(_match: Match): Unit = { val Match(sel, cases) = _match val selTyp = sel.tpe.widen.dealias