From 49940ee21a3dbf45f15e049bcdb530c72825f37e Mon Sep 17 00:00:00 2001 From: Eugene Flesselle Date: Wed, 17 Apr 2024 02:50:52 +0200 Subject: [PATCH 1/4] Retry `constraint.replace` after `constraint.updateEntry` Checking `isSub` on the resulting bounds can have introduced new constraints, which might allow us to replace the type parameter entirely. --- .../tools/dotc/core/ConstraintHandling.scala | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala index 06711ec97abf..d29e141184c9 100644 --- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala +++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala @@ -302,14 +302,18 @@ trait ConstraintHandling { false else val bound = legalBound(param, rawBound, isUpper) + lazy val recBound = bound.existsPart(_ eq param, StopAt.Static) + + // If the narrowed bounds are equal and not recursive, + // we can remove `param` from the constraint. + def tryReplace(lo: Type, hi: Type): Boolean = + val equalBounds = (if isUpper then lo else hi) eq bound + val canReplace = equalBounds && !recBound + if canReplace then constraint = constraint.replace(param, bound) + canReplace + 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 + tryReplace(lo, hi) || locally: // Narrow one of the bounds of type parameter `param` // If `isUpper` is true, ensure that `param <: `bound`, otherwise ensure // that `param >: bound`. @@ -328,7 +332,9 @@ trait ConstraintHandling { || { constraint = c1 val TypeBounds(lo, hi) = constraint.entry(param): @unchecked - isSub(lo, hi) + val isSat = isSub(lo, hi) + if isSat then tryReplace(lo, hi) // isSub may have introduced new constraints + isSat } end addOneBound From 97ed37e7bc51b9c8f0dcd01a41b794f226445aef Mon Sep 17 00:00:00 2001 From: Eugene Flesselle Date: Wed, 15 May 2024 15:45:14 +0200 Subject: [PATCH 2/4] Fix: don't use oldBounds to re-tryReplace --- .../src/dotty/tools/dotc/core/ConstraintHandling.scala | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala index d29e141184c9..9c7b00bb21f5 100644 --- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala +++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala @@ -306,14 +306,15 @@ trait ConstraintHandling { // If the narrowed bounds are equal and not recursive, // we can remove `param` from the constraint. - def tryReplace(lo: Type, hi: Type): Boolean = + def tryReplace: Boolean = + val TypeBounds(lo, hi) = constraint.nonParamBounds(param) val equalBounds = (if isUpper then lo else hi) eq bound val canReplace = equalBounds && !recBound if canReplace then constraint = constraint.replace(param, bound) canReplace - val oldBounds @ TypeBounds(lo, hi) = constraint.nonParamBounds(param) - tryReplace(lo, hi) || locally: + tryReplace || locally: + val oldBounds @ TypeBounds(lo, hi) = constraint.nonParamBounds(param) // Narrow one of the bounds of type parameter `param` // If `isUpper` is true, ensure that `param <: `bound`, otherwise ensure // that `param >: bound`. @@ -333,7 +334,7 @@ trait ConstraintHandling { constraint = c1 val TypeBounds(lo, hi) = constraint.entry(param): @unchecked val isSat = isSub(lo, hi) - if isSat then tryReplace(lo, hi) // isSub may have introduced new constraints + if isSat then tryReplace // isSub may have introduced new constraints isSat } end addOneBound From 14a6cac2bfffc9a13969673ba4f63a51b79147bd Mon Sep 17 00:00:00 2001 From: Eugene Flesselle Date: Wed, 15 May 2024 17:14:05 +0200 Subject: [PATCH 3/4] Do tryReplace using narrowedBounds instead of legalBound 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. The second tryReplace after updateEntry and isSub does not address this specific issue but #19955. --- .../tools/dotc/core/ConstraintHandling.scala | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala index 9c7b00bb21f5..0546d0d4a5e0 100644 --- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala +++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala @@ -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 @@ -301,32 +301,33 @@ trait ConstraintHandling { // so we shouldn't allow them as constraints either. false else - val bound = legalBound(param, rawBound, isUpper) - lazy val recBound = bound.existsPart(_ eq param, StopAt.Static) + + 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: Boolean = - val TypeBounds(lo, hi) = constraint.nonParamBounds(param) - val equalBounds = (if isUpper then lo else hi) eq bound - val canReplace = equalBounds && !recBound - if canReplace then constraint = constraint.replace(param, bound) + 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 || locally: - val oldBounds @ TypeBounds(lo, hi) = constraint.nonParamBounds(param) + tryReplace(narrowedBounds) || locally: // 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 //println(i"narrow bounds for $param from $oldBounds to $narrowedBounds") val c1 = constraint.updateEntry(param, narrowedBounds) (c1 eq constraint) @@ -334,7 +335,9 @@ trait ConstraintHandling { constraint = c1 val TypeBounds(lo, hi) = constraint.entry(param): @unchecked val isSat = isSub(lo, hi) - if isSat then tryReplace // isSub may have introduced new constraints + if isSat then + // isSub may have narrowed the bounds further + tryReplace(constraint.nonParamBounds(param)) isSat } end addOneBound From b1d7735285981c13f0dd1e0bcd3fee62785e1520 Mon Sep 17 00:00:00 2001 From: Eugene Flesselle Date: Thu, 16 May 2024 22:10:59 +0200 Subject: [PATCH 4/4] Fix doc position --- compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala index 0546d0d4a5e0..e63911a6a883 100644 --- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala +++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala @@ -302,6 +302,9 @@ trait ConstraintHandling { false 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: TypeBounds = val bound = legalBound(param, rawBound, isUpper) val oldBounds @ TypeBounds(lo, hi) = constraint.nonParamBounds(param) @@ -325,9 +328,6 @@ trait ConstraintHandling { canReplace tryReplace(narrowedBounds) || locally: - // Narrow one of the bounds of type parameter `param` - // If `isUpper` is true, ensure that `param <: `bound`, otherwise ensure - // that `param >: bound`. //println(i"narrow bounds for $param from $oldBounds to $narrowedBounds") val c1 = constraint.updateEntry(param, narrowedBounds) (c1 eq constraint)