From a8641c5cbe6ad22707ea52c639ee894cfa09db57 Mon Sep 17 00:00:00 2001
From: Guillaume Martres <smarter@ubuntu.com>
Date: Thu, 28 Apr 2022 01:36:11 +0200
Subject: [PATCH] Fix OrderingConstraint#order forgetting constraints; fix
 avoidLambdaParams

`order` takes `current` as input and returns a constraint set that subsumes both
`current` and `param1 <: param2`, but it's an instance method because it relies
on `this` to determine if `current` is used linearly such that we can reuse its
backing arrays instead of copying them. However, the implementation of `order`
mistakenly returned `this` and called methods on `this` instead of `current`.
This lead to issues like #11682 but that was compensated by logic inserted
in ConstraintHandling#addToConstraint which we can now remove.

Fixing this also required fixing an unrelated issue in avoidLambdaParams to
prevent a regression in tests/pos/i9676.scala: we shouldn't avoid a lambda param
under its own binder even if it is in `comparedTypeLambdas`, the sequence of
operations where this happens is:

[A] =>> List[A] <:< [A] =>> G[A]
  // comparedTypeLambdas ++= ([A] =>> List[A], [A] =>> G[A])
  List[A] <:< G[A]
    [A] =>> List[A] <:< G
      // previously, avoidLambdaParams([A] =>> List[A]) = [A] =>> List[Any],
      // now it leaves the type lambda alone.

We end up checking `[A] =>> List[A] <:< G` instead of just `List <:< G` because
of `ensureLambdaSub` in `compareAppliedTypeParamRef`. I'm not sure if this is
actually needed, but I decided to not disturb that code too much for now.
---
 .../tools/dotc/core/ConstraintHandling.scala  | 11 +++++-----
 .../tools/dotc/core/OrderingConstraint.scala  | 20 ++++++++++++-------
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
index 46c325209dee..ff3f5c4269dd 100644
--- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
+++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
@@ -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
         }
       }
     }
@@ -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)
           }
diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
index 6974f7f1b836..675816ad200f 100644
--- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
+++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
@@ -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.
@@ -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.
@@ -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:
    *