From 4fa9e5a6ef18b40eca3b41b8f8aea9ecb21e5d10 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Fri, 29 May 2020 11:13:09 +0200 Subject: [PATCH 1/5] Code refactoring --- .../src/dotty/tools/dotc/core/TypeOps.scala | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeOps.scala b/compiler/src/dotty/tools/dotc/core/TypeOps.scala index e8e59252ed7d..1e8251372b50 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeOps.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeOps.scala @@ -665,21 +665,21 @@ object TypeOps: 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)` + // Prefix inference, replace `p.C.this.Child` with `X.Child` where `X <: p.C` + // Note: we need to handle `p` recursively. + // // See tests/patmat/i3938.scala - class RemoveThisMap extends TypeMap { - var prefixTVar: Type = null + class InferPrefixMap extends TypeMap { + var tvars = Set.empty[TypeVar] def apply(tp: Type): Type = tp match { case ThisType(tref: TypeRef) if !tref.symbol.isStaticOwner => if (tref.symbol.is(Module)) TermRef(this(tref.prefix), tref.symbol.sourceModule) - else if (prefixTVar != null) - this(tref) else { - prefixTVar = WildcardType // prevent recursive call from assigning it - prefixTVar = newTypeVar(TypeBounds.upper(this(tref))) - prefixTVar + val bound = TypeRef(this(tref.prefix), tref.symbol) + val tvar = newTypeVar(TypeBounds.upper(bound)) + tvars = tvars + tvar + tvar } case tp => mapOver(tp) } @@ -693,14 +693,14 @@ object TypeOps: } } - val removeThisType = new RemoveThisMap + val inferThisMap = new InferPrefixMap val tvars = tp1.typeParams.map { tparam => newTypeVar(tparam.paramInfo.bounds) } - val protoTp1 = removeThisType.apply(tp1).appliedTo(tvars) + val protoTp1 = inferThisMap.apply(tp1).appliedTo(tvars) val force = new ForceDegree.Value( tvar => !(ctx.typerState.constraint.entry(tvar.origin) `eq` tvar.origin.underlying) || - (tvar `eq` removeThisType.prefixTVar), + inferThisMap.tvars.contains(tvar), // always instantiate prefix IfBottom.flip ) From 4841ecb684eea49caeca04da1c067be251a7d4e5 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Fri, 29 May 2020 11:55:41 +0200 Subject: [PATCH 2/5] Fix #9067: avoid instantiate child parameters to Wildcard if possible In tests/patmat/i9067.scala, for Foo[X1, X2] <: Base[Any, Any], X1 and X2 can both be `Any` instead of `Wildcard`. Inferring `Wildcard` will lead to the signature of `Foo.unapply` be inferred as `Nothing => Nothing`, which causes the checker to issue a false unexhaustive warning. --- .../src/dotty/tools/dotc/core/TypeOps.scala | 21 ++++++++++++------- tests/patmat/i9067.scala | 4 ++++ 2 files changed, 18 insertions(+), 7 deletions(-) create mode 100644 tests/patmat/i9067.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeOps.scala b/compiler/src/dotty/tools/dotc/core/TypeOps.scala index 1e8251372b50..96965d932f28 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeOps.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeOps.scala @@ -18,7 +18,7 @@ import config.Feature import typer.Applications._ import typer.ProtoTypes._ import typer.ForceDegree -import typer.Inferencing.isFullyDefined +import typer.Inferencing._ import typer.IfBottom import scala.annotation.internal.sharable @@ -618,8 +618,9 @@ object TypeOps: val childTp = if (child.isTerm) child.termRef else child.typeRef - instantiateToSubType(childTp, parent)(using ctx.fresh.setNewTyperState()) - .dealias + inContext(ctx.fresh.setExploreTyperState().setFreshGADTBounds) { + instantiateToSubType(childTp, parent).dealias + } } /** Instantiate type `tp1` to be a subtype of `tp2` @@ -685,10 +686,15 @@ object TypeOps: } } + def isPatternTypeSymbol(sym: Symbol) = !sym.isClass && sym.is(Case) + // replace uninstantiated type vars with WildcardType, check tests/patmat/3333.scala def instUndetMap(implicit ctx: Context) = new TypeMap { def apply(t: Type): Type = t match { - case tvar: TypeVar if !tvar.isInstantiated => WildcardType(tvar.origin.underlying.bounds) + case tvar: TypeVar if !tvar.isInstantiated => + WildcardType(tvar.origin.underlying.bounds) + case tref: TypeRef if isPatternTypeSymbol(tref.typeSymbol) => + WildcardType(tref.underlying.bounds) case _ => mapOver(t) } } @@ -715,9 +721,10 @@ object TypeOps: } } - if (protoTp1 <:< tp2) - if (isFullyDefined(protoTp1, force)) protoTp1 - else instUndetMap.apply(protoTp1) + if (protoTp1 <:< tp2) { + maximizeType(protoTp1, NoSpan, fromScala2x = false) + instUndetMap.apply(protoTp1) + } else { val protoTp2 = maxTypeMap.apply(tp2) if (protoTp1 <:< protoTp2 || parentQualify) diff --git a/tests/patmat/i9067.scala b/tests/patmat/i9067.scala new file mode 100644 index 000000000000..3ef8271e16df --- /dev/null +++ b/tests/patmat/i9067.scala @@ -0,0 +1,4 @@ +sealed trait Base[A, +B] +case class Foo[A, +B](f: A => B) extends Base[A, B] + +def bar(x: Base[Any, Any]): Unit = x match { case Foo(_) => } From 236486cf58ca02010c32e1cbbe50241db92db178 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Fri, 29 May 2020 12:02:42 +0200 Subject: [PATCH 3/5] Code refactoring --- compiler/src/dotty/tools/dotc/core/TypeOps.scala | 4 +--- .../dotty/tools/dotc/transform/TypeTestsCasts.scala | 12 +++++------- .../dotty/tools/dotc/transform/patmat/Space.scala | 3 +-- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeOps.scala b/compiler/src/dotty/tools/dotc/core/TypeOps.scala index 96965d932f28..c2d2363207d1 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeOps.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeOps.scala @@ -686,14 +686,12 @@ object TypeOps: } } - def isPatternTypeSymbol(sym: Symbol) = !sym.isClass && sym.is(Case) - // replace uninstantiated type vars with WildcardType, check tests/patmat/3333.scala def instUndetMap(implicit ctx: Context) = new TypeMap { def apply(t: Type): Type = t match { case tvar: TypeVar if !tvar.isInstantiated => WildcardType(tvar.origin.underlying.bounds) - case tref: TypeRef if isPatternTypeSymbol(tref.typeSymbol) => + case tref: TypeRef if tref.typeSymbol.isPatternBound => WildcardType(tref.underlying.bounds) case _ => mapOver(t) } diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index 70026a6b60d9..49ad01275685 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -55,22 +55,20 @@ object TypeTestsCasts { */ def checkable(X: Type, P: Type, span: Span)(using Context): Boolean = { def isAbstract(P: Type) = !P.dealias.typeSymbol.isClass - def isPatternTypeSymbol(sym: Symbol) = !sym.isClass && sym.is(Case) def replaceP(tp: Type)(implicit ctx: Context) = new TypeMap { def apply(tp: Type) = tp match { - case tref: TypeRef - if isPatternTypeSymbol(tref.typeSymbol) => WildcardType - case AnnotatedType(_, annot) - if annot.symbol == defn.UncheckedAnnot => WildcardType + case tref: TypeRef if tref.typeSymbol.isPatternBound => + WildcardType + case AnnotatedType(_, annot) if annot.symbol == defn.UncheckedAnnot => + WildcardType case _ => mapOver(tp) } }.apply(tp) def replaceX(tp: Type)(implicit ctx: Context) = new TypeMap { def apply(tp: Type) = tp match { - case tref: TypeRef - if isPatternTypeSymbol(tref.typeSymbol) => + case tref: TypeRef if tref.typeSymbol.isPatternBound => if (variance == 1) tref.info.hiBound else if (variance == -1) tref.info.loBound else OrType(defn.AnyType, defn.NothingType) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 9d20725a2590..98ba98b4f313 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -464,7 +464,6 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { * } */ private def erase(tp: Type, inArray: Boolean = false): Type = trace(i"$tp erased to", debug) { - def isPatternTypeSymbol(sym: Symbol) = !sym.isClass && sym.is(Case) tp match { case tp @ AppliedType(tycon, args) => @@ -476,7 +475,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { AndType(erase(tp1, inArray), erase(tp2, inArray)) case tp @ RefinedType(parent, _, _) => erase(parent) - case tref: TypeRef if isPatternTypeSymbol(tref.typeSymbol) => + case tref: TypeRef if tref.typeSymbol.isPatternBound => if (inArray) tref.underlying else WildcardType case _ => tp } From f0383597fa0a679507104a5621819cdfeaa63042 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Fri, 29 May 2020 23:46:44 +0200 Subject: [PATCH 4/5] Address review: use wildApprox --- .../src/dotty/tools/dotc/core/TypeOps.scala | 30 +++++++------------ tests/patmat/i3938.scala | 7 ++++- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeOps.scala b/compiler/src/dotty/tools/dotc/core/TypeOps.scala index c2d2363207d1..fd318e59bf9d 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeOps.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeOps.scala @@ -667,36 +667,26 @@ object TypeOps: def maxTypeMap(implicit ctx: Context) = new AbstractTypeMap(maximize = true) // Prefix inference, replace `p.C.this.Child` with `X.Child` where `X <: p.C` - // Note: we need to handle `p` recursively. + // Note: we need to strip ThisType in `p` recursively. // // See tests/patmat/i3938.scala class InferPrefixMap extends TypeMap { - var tvars = Set.empty[TypeVar] + var prefixTVar: Type = null def apply(tp: Type): Type = tp match { case ThisType(tref: TypeRef) if !tref.symbol.isStaticOwner => if (tref.symbol.is(Module)) TermRef(this(tref.prefix), tref.symbol.sourceModule) + else if (prefixTVar != null) + this(tref) else { - val bound = TypeRef(this(tref.prefix), tref.symbol) - val tvar = newTypeVar(TypeBounds.upper(bound)) - tvars = tvars + tvar - tvar + prefixTVar = WildcardType // prevent recursive call from assigning it + prefixTVar = newTypeVar(TypeBounds.upper(this(tref))) + prefixTVar } case tp => mapOver(tp) } } - // replace uninstantiated type vars with WildcardType, check tests/patmat/3333.scala - def instUndetMap(implicit ctx: Context) = new TypeMap { - def apply(t: Type): Type = t match { - case tvar: TypeVar if !tvar.isInstantiated => - WildcardType(tvar.origin.underlying.bounds) - case tref: TypeRef if tref.typeSymbol.isPatternBound => - WildcardType(tref.underlying.bounds) - case _ => mapOver(t) - } - } - val inferThisMap = new InferPrefixMap val tvars = tp1.typeParams.map { tparam => newTypeVar(tparam.paramInfo.bounds) } val protoTp1 = inferThisMap.apply(tp1).appliedTo(tvars) @@ -704,7 +694,7 @@ object TypeOps: val force = new ForceDegree.Value( tvar => !(ctx.typerState.constraint.entry(tvar.origin) `eq` tvar.origin.underlying) || - inferThisMap.tvars.contains(tvar), // always instantiate prefix + (tvar `eq` inferThisMap.prefixTVar), // always instantiate prefix IfBottom.flip ) @@ -721,13 +711,13 @@ object TypeOps: if (protoTp1 <:< tp2) { maximizeType(protoTp1, NoSpan, fromScala2x = false) - instUndetMap.apply(protoTp1) + wildApprox(protoTp1) } else { val protoTp2 = maxTypeMap.apply(tp2) if (protoTp1 <:< protoTp2 || parentQualify) if (isFullyDefined(AndType(protoTp1, protoTp2), force)) protoTp1 - else instUndetMap.apply(protoTp1) + else wildApprox(protoTp1) else { typr.println(s"$protoTp1 <:< $protoTp2 = false") NoType diff --git a/tests/patmat/i3938.scala b/tests/patmat/i3938.scala index d5c40b6490e5..0a6395b0a727 100644 --- a/tests/patmat/i3938.scala +++ b/tests/patmat/i3938.scala @@ -23,10 +23,15 @@ class Test { val foo = new Foo import foo.bar._ - def test(a: A) = { + def h(a: A) = { a match { case B() => 1 case _ => 2 // unreachable code } } + + def f(a: A) = + a match { + case B() => 1 + } } From a6ab570c3fdfd1e2f95c1c7b792e4a0da33bd392 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Fri, 29 May 2020 23:48:19 +0200 Subject: [PATCH 5/5] Add missing check file --- tests/patmat/i3938.check | 1 + 1 file changed, 1 insertion(+) create mode 100644 tests/patmat/i3938.check diff --git a/tests/patmat/i3938.check b/tests/patmat/i3938.check new file mode 100644 index 000000000000..5aaefe2263e1 --- /dev/null +++ b/tests/patmat/i3938.check @@ -0,0 +1 @@ +34: Pattern Match Exhaustivity: bar.C()