From 14a6cac2bfffc9a13969673ba4f63a51b79147bd Mon Sep 17 00:00:00 2001
From: Eugene Flesselle <eugene@flesselle.net>
Date: Wed, 15 May 2024 17:14:05 +0200
Subject: [PATCH] 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