Skip to content

Commit

Permalink
Refine criterion when to widen types (#17180)
Browse files Browse the repository at this point in the history
  • Loading branch information
dwijnand authored Jun 6, 2023
2 parents 46e28b8 + 09f5e4c commit 5d2812a
Show file tree
Hide file tree
Showing 15 changed files with 192 additions and 45 deletions.
8 changes: 8 additions & 0 deletions compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ trait ConstraintHandling {
*/
protected var canWidenAbstract: Boolean = true

/**
* Used for match type reduction.
* When an abstract type may not be widened, according to `widenAbstractOKFor`,
* we record it in this set, so that we can ultimately fail the reduction, but
* with all the information that comes out from continuing to widen the abstract type.
*/
protected var poisoned: Set[TypeParamRef] = Set.empty

protected var myNecessaryConstraintsOnly = false
/** When collecting the constraints needed for a particular subtyping
* judgment to be true, we sometimes need to approximate the constraint
Expand Down
57 changes: 50 additions & 7 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import typer.Applications.productSelectorTypes
import reporting.trace
import annotation.constructorOnly
import cc.{CapturingType, derivedCapturingType, CaptureSet, stripCapturing, isBoxedCapturing, boxed, boxedUnlessFun, boxedIfTypeParam, isAlwaysPure}
import NameKinds.WildcardParamName

/** Provides methods to compare types.
*/
Expand Down Expand Up @@ -865,10 +866,36 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
fourthTry
}

/** Can we widen an abstract type when comparing with `tp`?
* This is the case with the following cases:
* - if `canWidenAbstract` is true.
*
* Secondly, if `tp` is a type parameter, we can widen if:
* - if `tp` is not a type parameter of the matched-against case lambda
* - if `tp` is an invariant or wildcard type parameter
* - finally, allow widening, but record the type parameter in `poisoned`,
* so that can be accounted for during the reduction step
*/
def widenAbstractOKFor(tp: Type): Boolean =
val acc = new TypeAccumulator[Boolean]:
override def apply(x: Boolean, t: Type) =
x && t.match
case t: TypeParamRef =>
variance == 0
|| (t.binder ne caseLambda)
|| t.paramName.is(WildcardParamName)
|| { poisoned += t; true }
case _ =>
foldOver(x, t)

canWidenAbstract && acc(true, tp)

def tryBaseType(cls2: Symbol) = {
val base = nonExprBaseType(tp1, cls2).boxedIfTypeParam(tp1.typeSymbol)
if base.exists && (base ne tp1)
&& (!caseLambda.exists || canWidenAbstract || tp1.widen.underlyingClassRef(refinementOK = true).exists)
&& (!caseLambda.exists
|| widenAbstractOKFor(tp2)
|| tp1.widen.underlyingClassRef(refinementOK = true).exists)
then
isSubType(base, tp2, if (tp1.isRef(cls2)) approx else approx.addLow)
&& recordGadtUsageIf { MatchType.thatReducesUsingGadt(tp1) }
Expand All @@ -889,8 +916,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
|| narrowGADTBounds(tp1, tp2, approx, isUpper = true))
&& (tp2.isAny || GADTusage(tp1.symbol))

(!caseLambda.exists || canWidenAbstract)
&& isSubType(hi1.boxedIfTypeParam(tp1.symbol), tp2, approx.addLow) && (trustBounds || isSubType(lo1, tp2, approx.addLow))
(!caseLambda.exists || widenAbstractOKFor(tp2))
&& isSubType(hi1.boxedIfTypeParam(tp1.symbol), tp2, approx.addLow) && (trustBounds || isSubType(lo1, tp2, approx.addLow))
|| compareGADT
|| tryLiftedToThis1
case _ =>
Expand Down Expand Up @@ -984,7 +1011,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
tp1.cases.corresponds(tp2.cases)(isSubType)
case _ => false
}
recur(tp1.underlying, tp2) || compareMatch
(!caseLambda.exists || canWidenAbstract) && recur(tp1.underlying, tp2) || compareMatch
case tp1: AnnotatedType if tp1.isRefining =>
isNewSubType(tp1.parent)
case JavaArrayType(elem1) =>
Expand Down Expand Up @@ -3091,6 +3118,9 @@ class TrackingTypeComparer(initctx: Context) extends TypeComparer(initctx) {
}

def matchCases(scrut: Type, cases: List[Type])(using Context): Type = {
// a reference for the type parameters poisoned during matching
// for use during the reduction step
var poisoned: Set[TypeParamRef] = Set.empty

def paramInstances(canApprox: Boolean) = new TypeAccumulator[Array[Type]]:
def apply(insts: Array[Type], t: Type) = t match
Expand All @@ -3102,16 +3132,24 @@ class TrackingTypeComparer(initctx: Context) extends TypeComparer(initctx) {
case entry: TypeBounds =>
val lo = fullLowerBound(param)
val hi = fullUpperBound(param)
if isSubType(hi, lo) then lo.simplified else Range(lo, hi)
if !poisoned(param) && isSubType(hi, lo) then lo.simplified else Range(lo, hi)
case inst =>
assert(inst.exists, i"param = $param\nconstraint = $constraint")
inst.simplified
if !poisoned(param) then inst.simplified else Range(inst, inst)
insts
case _ =>
foldOver(insts, t)

def instantiateParams(insts: Array[Type]) = new ApproximatingTypeMap {
variance = 0

override def range(lo: Type, hi: Type): Type =
if variance == 0 && (lo eq hi) then
// override the default `lo eq hi` test, which removes the Range
// which leads to a Reduced result, instead of NoInstance
Range(lower(lo), upper(hi))
else super.range(lo, hi)

def apply(t: Type) = t match {
case t @ TypeParamRef(b, n) if b `eq` caseLambda => insts(n)
case t: LazyRef => apply(t.ref)
Expand All @@ -3133,9 +3171,14 @@ class TrackingTypeComparer(initctx: Context) extends TypeComparer(initctx) {

def matches(canWidenAbstract: Boolean): Boolean =
val saved = this.canWidenAbstract
val savedPoisoned = this.poisoned
this.canWidenAbstract = canWidenAbstract
this.poisoned = Set.empty
try necessarySubType(scrut, pat)
finally this.canWidenAbstract = saved
finally
poisoned = this.poisoned
this.poisoned = savedPoisoned
this.canWidenAbstract = saved

def redux(canApprox: Boolean): MatchResult =
caseLambda match
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotc/pos-test-pickling.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ i6505.scala
i15158.scala
i15155.scala
i15827.scala
i17149.scala

# Opaque type
i5720.scala
Expand Down
2 changes: 2 additions & 0 deletions docs/_docs/reference/new-types/match-types.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ An instantiation `Is` is _minimal_ for `Xs` if all type variables in `Xs` that
appear covariantly and nonvariantly in `Is` are as small as possible and all
type variables in `Xs` that appear contravariantly in `Is` are as large as
possible. Here, "small" and "large" are understood with respect to `<:`.
But, type parameter will not be "large" if a pattern containing it is matched
against lambda case in co- or contra-variant position.

For simplicity, we have omitted constraint handling so far. The full formulation
of subtyping tests describes them as a function from a constraint and a pair of
Expand Down
12 changes: 6 additions & 6 deletions tests/neg/11982.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ type Head[X] = X match {
}

object Unpair {
def unpair[X <: Tuple2[Any, Any]]: Head[X] = 1
unpair[Tuple2["msg", 42]]: "msg" // error
def unpair[X <: Tuple2[Any, Any]]: Head[X] = 1 // error
unpair[Tuple2["msg", 42]]: "msg"
}


Expand All @@ -14,8 +14,8 @@ type Head2[X] = X match {
}

object Unpair2 {
def unpair[X <: Tuple2[Tuple2[Any, Any], Tuple2[Any, Any]]]: Head2[X] = 1
unpair[Tuple2[Tuple2["msg", 42], Tuple2[41, 40]]]: "msg" // error
def unpair[X <: Tuple2[Tuple2[Any, Any], Tuple2[Any, Any]]]: Head2[X] = 1 // error
unpair[Tuple2[Tuple2["msg", 42], Tuple2[41, 40]]]: "msg"
}


Expand All @@ -35,6 +35,6 @@ type Head4[X] = X match {
}

object Unpair4 {
def unpair[X <: Foo[Any, Any]]: Head4[Foo[X, X]] = 1
unpair[Foo["msg", 42]]: "msg" // error
def unpair[X <: Foo[Any, Any]]: Head4[Foo[X, X]] = 1 // error
unpair[Foo["msg", 42]]: "msg"
}
4 changes: 0 additions & 4 deletions tests/neg/i11982.check

This file was deleted.

27 changes: 0 additions & 27 deletions tests/neg/i11982.scala

This file was deleted.

20 changes: 20 additions & 0 deletions tests/neg/i13780.check
Original file line number Diff line number Diff line change
@@ -1,3 +1,23 @@
-- [E007] Type Mismatch Error: tests/neg/i13780.scala:12:32 ------------------------------------------------------------
12 | def unpair[X <: Y]: Head[X] = "" // error
| ^^
| Found: ("" : String)
| Required: Head[X]
|
| where: X is a type in method unpair with bounds <: A.this.Y
|
|
| Note: a match type could not be fully reduced:
|
| trying to reduce Head[X]
| failed since selector X
| does not uniquely determine parameters a, b in
| case (a, b) => a
| The computed bounds for the parameters are:
| a >: Any
| b >: Any
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/neg/i13780.scala:18:31 ------------------------------------------------------------
18 | def int[X <: Y]: Int = unpair[X] // error
| ^^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion tests/neg/i13780.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ trait Z {

class A extends Z {
type Y <: Tuple2[Any, Any]
def unpair[X <: Y]: Head[X] = ""
def unpair[X <: Y]: Head[X] = "" // error
def any[X <: Y]: Any = unpair[X]
}

Expand Down
17 changes: 17 additions & 0 deletions tests/neg/i17149.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
type Ext1[S] = S match {
case Seq[t] => t
}
type Ext2[S] = S match {
case Seq[_] => Int
}
type Ext3[S] = S match {
case Array[t] => t
}
type Ext4[S] = S match {
case Array[_] => Int
}
def foo[T <: Seq[Any], A <: Array[B], B] =
summon[Ext1[T] =:= T] // error
summon[Ext2[T] =:= Int] // ok
summon[Ext3[A] =:= B] // ok
summon[Ext4[A] =:= Int] // ok
7 changes: 7 additions & 0 deletions tests/pos/i15926.contra.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
trait Show[-A >: Nothing]

type MT1[I <: Show[Nothing], N] = I match
case Show[a] => N match
case Int => a

val a = summon[MT1[Show[String], Int] =:= String]
24 changes: 24 additions & 0 deletions tests/pos/i15926.extract.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// like pos/i15926.scala
// but with the nested match type extracted
// which is a workaround that fixed the problem
sealed trait Nat
final case class Zero() extends Nat
final case class Succ[+N <: Nat]() extends Nat

final case class Neg[+N <: Succ[Nat]]()

type Sum[X, Y] = Y match
case Zero => X
case Succ[y] => Sum[Succ[X], y]

type IntSum[A, B] = B match
case Neg[b] => IntSumNeg[A, b]

type IntSumNeg[A, B] = A match
case Neg[a] => Neg[Sum[a, B]]

type One = Succ[Zero]
type Two = Succ[One]

class Test:
def test() = summon[IntSum[Neg[One], Neg[One]] =:= Neg[Two]]
22 changes: 22 additions & 0 deletions tests/pos/i15926.min.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// like pos/i15926.scala
// but minimised to the subset of paths needed
// to fail the specific test case
sealed trait Nat
final case class Zero() extends Nat
final case class Succ[+N <: Nat]() extends Nat

final case class Neg[+N <: Succ[Nat]]()

type Sum[X, Y] = Y match
case Zero => X
case Succ[y] => Sum[Succ[X], y]

type IntSum[A, B] = B match
case Neg[b] => A match
case Neg[a] => Neg[Sum[a, b]]

type One = Succ[Zero]
type Two = Succ[One]

class Test:
def test() = summon[IntSum[Neg[One], Neg[One]] =:= Neg[Two]]
28 changes: 28 additions & 0 deletions tests/pos/i15926.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@

@main def main(): Unit =
println(summon[Sum[Minus[Succ[Zero]], Minus[Succ[Zero]]] =:= Minus[Succ[Succ[Zero]]]])

sealed trait IntT
sealed trait NatT extends IntT
final case class Zero() extends NatT
final case class Succ[+N <: NatT](n: N) extends NatT
final case class Minus[+N <: Succ[NatT]](n: N) extends IntT

type NatSum[X <: NatT, Y <: NatT] <: NatT = Y match
case Zero => X
case Succ[y] => NatSum[Succ[X], y]

type NatDif[X <: NatT, Y <: NatT] <: IntT = Y match
case Zero => X
case Succ[y] => X match
case Zero => Minus[Y]
case Succ[x] => NatDif[x, y]

type Sum[X <: IntT, Y <: IntT] <: IntT = Y match
case Zero => X
case Minus[y] => X match
case Minus[x] => Minus[NatSum[x, y]]
case _ => NatDif[X, y]
case _ => X match
case Minus[x] => NatDif[Y, x]
case _ => NatSum[X, Y]
6 changes: 6 additions & 0 deletions tests/pos/i17149.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type Ext[S <: Seq[_]] = S match {
case Seq[t] => t
}

val _ = implicitly[Ext[Seq[Int]] =:= Int] // e.scala: Cannot prove that e.Ext[Seq[Int]] =:= Int
val _ = summon[Ext[Seq[Int]] =:= Int]

0 comments on commit 5d2812a

Please sign in to comment.