Skip to content

Commit a8641c5

Browse files
committed
Fix OrderingConstraint#order forgetting constraints; fix avoidLambdaParams
`order` takes `current` as input and returns a constraint set that subsumes both `current` and `param1 <: param2`, but it's an instance method because it relies on `this` to determine if `current` is used linearly such that we can reuse its backing arrays instead of copying them. However, the implementation of `order` mistakenly returned `this` and called methods on `this` instead of `current`. This lead to issues like #11682 but that was compensated by logic inserted in ConstraintHandling#addToConstraint which we can now remove. Fixing this also required fixing an unrelated issue in avoidLambdaParams to prevent a regression in tests/pos/i9676.scala: we shouldn't avoid a lambda param under its own binder even if it is in `comparedTypeLambdas`, the sequence of operations where this happens is: [A] =>> List[A] <:< [A] =>> G[A] // comparedTypeLambdas ++= ([A] =>> List[A], [A] =>> G[A]) List[A] <:< G[A] [A] =>> List[A] <:< G // previously, avoidLambdaParams([A] =>> List[A]) = [A] =>> List[Any], // now it leaves the type lambda alone. We end up checking `[A] =>> List[A] <:< G` instead of just `List <:< G` because of `ensureLambdaSub` in `compareAppliedTypeParamRef`. I'm not sure if this is actually needed, but I decided to not disturb that code too much for now.
1 parent f8e5203 commit a8641c5

File tree

2 files changed

+19
-12
lines changed

2 files changed

+19
-12
lines changed

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -627,11 +627,7 @@ trait ConstraintHandling {
627627
case x =>
628628
// Happens if param was already solved while processing earlier params of the same TypeLambda.
629629
// See #4720.
630-
631-
// Should propagate bounds even when param has been solved.
632-
// See #11682.
633-
lower.forall(addOneBound(_, x, isUpper = true)) &&
634-
upper.forall(addOneBound(_, x, isUpper = false))
630+
true
635631
}
636632
}
637633
}
@@ -677,6 +673,11 @@ trait ConstraintHandling {
677673
case t @ TypeParamRef(tl: TypeLambda, n) if comparedTypeLambdas contains tl =>
678674
val bounds = tl.paramInfos(n)
679675
range(bounds.lo, bounds.hi)
676+
case tl: TypeLambda =>
677+
val saved = comparedTypeLambdas
678+
comparedTypeLambdas -= tl
679+
try mapOver(tl)
680+
finally comparedTypeLambdas = saved
680681
case _ =>
681682
mapOver(t)
682683
}

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -354,13 +354,18 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
354354
* `<:<` relationships between parameters ("edges") but not bounds.
355355
*/
356356
def order(current: This, param1: TypeParamRef, param2: TypeParamRef, direction: UnificationDirection = NoUnification)(using Context): This =
357-
if (param1 == param2 || current.isLess(param1, param2)) this
358-
else {
359-
assert(contains(param1), i"$param1")
360-
assert(contains(param2), i"$param2")
357+
// /!\ Careful here: we're adding constraints on `current`, not `this`, so
358+
// think twice when using an instance method! We only need to pass `this` as
359+
// the `prev` argument in methods on `ConstraintLens`.
360+
// TODO: Refactor this code to take `prev` as a parameter and add
361+
// constraints on `this` instead?
362+
if param1 == param2 || current.isLess(param1, param2) then current
363+
else
364+
assert(current.contains(param1), i"$param1")
365+
assert(current.contains(param2), i"$param2")
361366
val unifying = direction != NoUnification
362367
val newUpper = {
363-
val up = exclusiveUpper(param2, param1)
368+
val up = current.exclusiveUpper(param2, param1)
364369
if unifying then
365370
// Since param2 <:< param1 already holds now, filter out param1 to avoid adding
366371
// duplicated orderings.
@@ -374,7 +379,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
374379
param2 :: up
375380
}
376381
val newLower = {
377-
val lower = exclusiveLower(param1, param2)
382+
val lower = current.exclusiveLower(param1, param2)
378383
if unifying then
379384
// Similarly, filter out param2 from lowerly-ordered parameters
380385
// to avoid duplicated orderings.
@@ -390,7 +395,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
390395
val current1 = newLower.foldLeft(current)(upperLens.map(this, _, _, newUpper ::: _))
391396
val current2 = newUpper.foldLeft(current1)(lowerLens.map(this, _, _, newLower ::: _))
392397
current2
393-
}
398+
end if
399+
end order
394400

395401
/** The list of parameters P such that, for a fresh type parameter Q:
396402
*

0 commit comments

Comments
 (0)