Skip to content

Commit 362aaf2

Browse files
committed
Fix variance of approximating map in addOneBound
It turns out that the variance was flipped: when adding a constraint `P <: List[WildcardType(Int, Any)]`, we should over-constrain to `P <: List[Int]`, but previously we would under-constrain to `P <: List[Any]` which would allow us later to infer `P := List[String]` for example. However, this logic needs to be flipped when inferring necessary constraints (this explains why the previous behavior was seemingly correct). No test case but this will end up being important in later commits of this PR where we re-use the same map to do more approximations.
1 parent 454e301 commit 362aaf2

File tree

2 files changed

+23
-19
lines changed

2 files changed

+23
-19
lines changed

compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,24 @@ trait ConstraintHandling {
5656
*/
5757
protected var comparedTypeLambdas: Set[TypeLambda] = Set.empty
5858

59+
protected var myNecessaryConstraintsOnly = false
60+
/** When collecting the constraints needed for a particular subtyping
61+
* judgment to be true, we sometimes need to approximate the constraint
62+
* set (see `TypeComparer#either` for example).
63+
*
64+
* Normally, this means adding extra constraints which may not be necessary
65+
* for the subtyping judgment to be true, but if this variable is set to true
66+
* we will instead under-approximate and keep only the constraints that must
67+
* always be present for the subtyping judgment to hold.
68+
*
69+
* This is needed for GADT bounds inference to be sound, but it is also used
70+
* when constraining a method call based on its expected type to avoid adding
71+
* constraints that would later prevent us from typechecking method
72+
* arguments, see or-inf.scala and and-inf.scala for examples.
73+
*/
74+
protected def necessaryConstraintsOnly(using Context): Boolean =
75+
ctx.mode.is(Mode.GadtConstraintInference) || myNecessaryConstraintsOnly
76+
5977
def checkReset() =
6078
assert(addConstraintInvocations == 0)
6179
assert(frozenConstraint == false)
@@ -92,7 +110,11 @@ trait ConstraintHandling {
92110
false
93111
else
94112
val dropWildcards = new AvoidWildcardsMap:
95-
if !isUpper then variance = -1
113+
// Approximate the upper-bound from below and vice-versa
114+
if isUpper then variance = -1
115+
// ...unless we can only infer necessary constraints, in which case we
116+
// flip the variance to under-approximate.
117+
if necessaryConstraintsOnly then variance = -variance
96118
override def mapWild(t: WildcardType) =
97119
if approximateWildcards then super.mapWild(t)
98120
else newTypeVar(apply(t.effectiveBounds).toBounds)

compiler/src/dotty/tools/dotc/core/TypeComparer.scala

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -63,24 +63,6 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
6363
private var myInstance: TypeComparer = this
6464
def currentInstance: TypeComparer = myInstance
6565

66-
private var myNecessaryConstraintsOnly = false
67-
/** When collecting the constraints needed for a particular subtyping
68-
* judgment to be true, we sometimes need to approximate the constraint
69-
* set (see `TypeComparer#either` for example).
70-
*
71-
* Normally, this means adding extra constraints which may not be necessary
72-
* for the subtyping judgment to be true, but if this variable is set to true
73-
* we will instead under-approximate and keep only the constraints that must
74-
* always be present for the subtyping judgment to hold.
75-
*
76-
* This is needed for GADT bounds inference to be sound, but it is also used
77-
* when constraining a method call based on its expected type to avoid adding
78-
* constraints that would later prevent us from typechecking method
79-
* arguments, see or-inf.scala and and-inf.scala for examples.
80-
*/
81-
protected def necessaryConstraintsOnly(using Context) =
82-
ctx.mode.is(Mode.GadtConstraintInference) || myNecessaryConstraintsOnly
83-
8466
/** Is a subtype check in progress? In that case we may not
8567
* permanently instantiate type variables, because the corresponding
8668
* constraint might still be retracted and the instantiation should

0 commit comments

Comments
 (0)