From c60817704ca44b777722fbfdb35b016a1b58d97f Mon Sep 17 00:00:00 2001
From: Eugene Flesselle <eugene@flesselle.net>
Date: Thu, 16 May 2024 23:19:00 +0200
Subject: [PATCH] Do `constraint.replace` when `addOneBound` produces equal
 bounds

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
---
 .../tools/dotc/core/ConstraintHandling.scala  | 56 +++++++++++--------
 1 file changed, 33 insertions(+), 23 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
index 06711ec97abf..e63911a6a883 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,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