Skip to content

Fix OrderingConstraint#order forgetting constraints; fix avoidLambdaParams #15077

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
Original file line number Diff line number Diff line change
Expand Up @@ -627,11 +627,7 @@ trait ConstraintHandling {
case x =>
// Happens if param was already solved while processing earlier params of the same TypeLambda.
// See #4720.

// Should propagate bounds even when param has been solved.
// See #11682.
lower.forall(addOneBound(_, x, isUpper = true)) &&
upper.forall(addOneBound(_, x, isUpper = false))
true
}
}
}
Expand Down Expand Up @@ -677,6 +673,11 @@ trait ConstraintHandling {
case t @ TypeParamRef(tl: TypeLambda, n) if comparedTypeLambdas contains tl =>
val bounds = tl.paramInfos(n)
range(bounds.lo, bounds.hi)
case tl: TypeLambda =>
val saved = comparedTypeLambdas
comparedTypeLambdas -= tl
try mapOver(tl)
finally comparedTypeLambdas = saved
case _ =>
mapOver(t)
}
Expand Down
20 changes: 13 additions & 7 deletions compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -354,13 +354,18 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
* `<:<` relationships between parameters ("edges") but not bounds.
*/
def order(current: This, param1: TypeParamRef, param2: TypeParamRef, direction: UnificationDirection = NoUnification)(using Context): This =
if (param1 == param2 || current.isLess(param1, param2)) this
else {
assert(contains(param1), i"$param1")
assert(contains(param2), i"$param2")
// /!\ Careful here: we're adding constraints on `current`, not `this`, so
// think twice when using an instance method! We only need to pass `this` as
// the `prev` argument in methods on `ConstraintLens`.
// TODO: Refactor this code to take `prev` as a parameter and add
// constraints on `this` instead?
if param1 == param2 || current.isLess(param1, param2) then current
else
assert(current.contains(param1), i"$param1")
assert(current.contains(param2), i"$param2")
val unifying = direction != NoUnification
val newUpper = {
val up = exclusiveUpper(param2, param1)
val up = current.exclusiveUpper(param2, param1)
if unifying then
// Since param2 <:< param1 already holds now, filter out param1 to avoid adding
// duplicated orderings.
Expand All @@ -374,7 +379,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
param2 :: up
}
val newLower = {
val lower = exclusiveLower(param1, param2)
val lower = current.exclusiveLower(param1, param2)
if unifying then
// Similarly, filter out param2 from lowerly-ordered parameters
// to avoid duplicated orderings.
Expand All @@ -390,7 +395,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
val current1 = newLower.foldLeft(current)(upperLens.map(this, _, _, newUpper ::: _))
val current2 = newUpper.foldLeft(current1)(lowerLens.map(this, _, _, newLower ::: _))
current2
}
end if
end order

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