Skip to content

Commit

Permalink
Do constraint.replace when addOneBound produces equal bounds
Browse files Browse the repository at this point in the history
as an optimization

In #20120, we reach constraints with equal bounds that are intersection types,
they are formed from multiple successive calls to `addOneBound`.
We miss the `replace` optimization in this case because
the bounds only become equal progressively, and
we are only checking for equality with the constraint being added.

Additionally, we recheck for equal bounds after `constraint.updateEntry`
as checking `isSub` can have narrowed the bounds further.
#19955 is an example where this second optimization applies.

Fix #20120
Close #20208 the original implementation
  • Loading branch information
EugeneFlesselle committed May 16, 2024
1 parent 73bb2aa commit c608177
Showing 1 changed file with 33 additions and 23 deletions.
56 changes: 33 additions & 23 deletions compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ trait ConstraintHandling {
*/
private var myTrustBounds = true

inline def withUntrustedBounds(op: => Type): Type =
transparent inline def withUntrustedBounds(op: => Type): Type =
val saved = myTrustBounds
myTrustBounds = false
try op finally myTrustBounds = saved
Expand Down Expand Up @@ -301,34 +301,44 @@ trait ConstraintHandling {
// so we shouldn't allow them as constraints either.
false
else
val bound = legalBound(param, rawBound, isUpper)
val oldBounds @ TypeBounds(lo, hi) = constraint.nonParamBounds(param)
val equalBounds = (if isUpper then lo else hi) eq bound
if equalBounds && !bound.existsPart(_ eq param, StopAt.Static) then
// The narrowed bounds are equal and not recursive,
// so we can remove `param` from the constraint.
constraint = constraint.replace(param, bound)
true
else
// Narrow one of the bounds of type parameter `param`
// If `isUpper` is true, ensure that `param <: `bound`, otherwise ensure
// that `param >: bound`.
val narrowedBounds =
val saved = homogenizeArgs
homogenizeArgs = Config.alignArgsInAnd
try
withUntrustedBounds(
if isUpper then oldBounds.derivedTypeBounds(lo, hi & bound)
else oldBounds.derivedTypeBounds(lo | bound, hi))
finally
homogenizeArgs = saved

// Narrow one of the bounds of type parameter `param`
// If `isUpper` is true, ensure that `param <: `bound`,
// otherwise ensure that `param >: bound`.
val narrowedBounds: TypeBounds =
val bound = legalBound(param, rawBound, isUpper)
val oldBounds @ TypeBounds(lo, hi) = constraint.nonParamBounds(param)

val saved = homogenizeArgs
homogenizeArgs = Config.alignArgsInAnd
try
withUntrustedBounds(
if isUpper then oldBounds.derivedTypeBounds(lo, hi & bound)
else oldBounds.derivedTypeBounds(lo | bound, hi))
finally
homogenizeArgs = saved
end narrowedBounds

// If the narrowed bounds are equal and not recursive,
// we can remove `param` from the constraint.
def tryReplace(newBounds: TypeBounds): Boolean =
val TypeBounds(lo, hi) = newBounds
val canReplace = (lo eq hi) && !newBounds.existsPart(_ eq param, StopAt.Static)
if canReplace then constraint = constraint.replace(param, lo)
canReplace

tryReplace(narrowedBounds) || locally:
//println(i"narrow bounds for $param from $oldBounds to $narrowedBounds")
val c1 = constraint.updateEntry(param, narrowedBounds)
(c1 eq constraint)
|| {
constraint = c1
val TypeBounds(lo, hi) = constraint.entry(param): @unchecked
isSub(lo, hi)
val isSat = isSub(lo, hi)
if isSat then
// isSub may have narrowed the bounds further
tryReplace(constraint.nonParamBounds(param))
isSat
}
end addOneBound

Expand Down

0 comments on commit c608177

Please sign in to comment.