From c657700b05c0e94107d7e468a0382233d6622c0e Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 29 Apr 2020 10:49:12 +0200
Subject: [PATCH 01/22] Don't lose info in bounds when pruning

---
 .../tools/dotc/core/ConstraintHandling.scala  | 35 ++++++++++---------
 tests/{neg => pos}/i6565.scala                |  4 +--
 2 files changed, 20 insertions(+), 19 deletions(-)
 rename tests/{neg => pos}/i6565.scala (84%)

diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
index 0f61fd2e25fe..fdfbc5887c4e 100644
--- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
+++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
@@ -495,30 +495,31 @@ trait ConstraintHandling[AbstractContext] {
        *
        *  @return The pruned type if all `addLess` calls succeed, `NoType` otherwise.
        */
-      def prune(bound: Type): Type = bound match {
-        case bound: AndType =>
-          val p1 = prune(bound.tp1)
-          val p2 = prune(bound.tp2)
-          if (p1.exists && p2.exists) bound.derivedAndType(p1, p2)
+      def prune(bnd: Type): Type = bnd match {
+        case bnd: AndType if !fromBelow =>
+          val p1 = prune(bnd.tp1)
+          val p2 = prune(bnd.tp2)
+          if (p1.exists && p2.exists) bnd.derivedAndType(p1, p2)
           else NoType
-        case bound: OrType =>
-          val p1 = prune(bound.tp1)
-          val p2 = prune(bound.tp2)
-          if (p1.exists && p2.exists) bound.derivedOrType(p1, p2)
+        case bnd: OrType if fromBelow =>
+          val p1 = prune(bnd.tp1)
+          val p2 = prune(bnd.tp2)
+          if (p1.exists && p2.exists) bnd.derivedOrType(p1, p2)
           else NoType
-        case bound: TypeVar if constraint contains bound.origin =>
-          prune(bound.underlying)
-        case bound: TypeParamRef =>
-          constraint.entry(bound) match {
-            case NoType => pruneLambdaParams(bound)
+        case bnd: TypeVar if constraint contains bnd.origin =>
+          prune(bnd.underlying)
+        case bnd: TypeParamRef if bnd ne param =>
+          constraint.entry(bnd) match {
+            case NoType => pruneLambdaParams(bnd)
             case _: TypeBounds =>
-              if (!addParamBound(bound)) NoType
+              assertFail(i"pruning $param $bound $fromBelow $bnd")
+              if (!addParamBound(bnd)) NoType
               else if (fromBelow) defn.NothingType
               else defn.AnyType
             case inst =>
               prune(inst)
           }
-        case bound: ExprType =>
+        case bnd: ExprType =>
           // ExprTypes are not value types, so type parameters should not
           // be instantiated to ExprTypes. A scenario where such an attempted
           // instantiation can happen is if we unify (=> T) => () with A => ()
@@ -530,7 +531,7 @@ trait ConstraintHandling[AbstractContext] {
           // the resulting types down) and is largely unknown terrain.
           NoType
         case _ =>
-          pruneLambdaParams(bound)
+          pruneLambdaParams(bnd)
       }
 
       def kindCompatible(tp1: Type, tp2: Type): Boolean =
diff --git a/tests/neg/i6565.scala b/tests/pos/i6565.scala
similarity index 84%
rename from tests/neg/i6565.scala
rename to tests/pos/i6565.scala
index d5fab12842d3..5e199001ff4e 100644
--- a/tests/neg/i6565.scala
+++ b/tests/pos/i6565.scala
@@ -12,6 +12,6 @@ lazy val ok: Lifted[String] = { // ok despite map returning a union
   point("a").map(_ => if true then "foo" else error) // ok
 }
 
-lazy val bad: Lifted[String] = { // found Lifted[Object]
-  point("a").flatMap(_ => point("b").map(_ => if true then "foo" else error)) // error
+lazy val nowAlsoOK: Lifted[String] = {
+  point("a").flatMap(_ => point("b").map(_ => if true then "foo" else error))
 }

From aca3133634a30e7e168469306d8a4ae2b2e6b14f Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 29 Apr 2020 11:13:21 +0200
Subject: [PATCH 02/22] Simplify prune

---
 .../tools/dotc/core/ConstraintHandling.scala  | 82 ++++++-------------
 1 file changed, 26 insertions(+), 56 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
index fdfbc5887c4e..f0ff31005cca 100644
--- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
+++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
@@ -460,66 +460,36 @@ trait ConstraintHandling[AbstractContext] {
             if (fromBelow) isSubType(bound, tp) else isSubType(tp, bound)
         }
 
-      /** Drop all constrained parameters that occur at the toplevel in `bound` and
-       *  handle them by `addLess` calls.
-       *  The preconditions make sure that such parameters occur only
-       *  in one of two ways:
+      /** Normalize the bound `bnd` in the following ways:
        *
-       *  1.
+       *   1. Any toplevel occurrences of the compared parameter `param` are
+       *      replaced by `Nothing` if bound is from below or `Any` otherwise.
+       *   2. Toplevel occurrences of TypeVars over TypeRefs in the current
+       *      constraint are dereferenced.
+       *   3. Toplevel occurrences of TypeRefs that are instantiated in the current
+       *      constraint are also referenced.
+       *   4. Toplevel occurrences of ExprTypes lead to a `NoType` return, which
+       *      causes the addConstraint operation to fail.
        *
-       *    P <: Ts1 | ... | Tsm   (m > 0)
-       *    Tsi = T1 & ... Tn      (n >= 0)
-       *    Some of the Ti are constrained parameters
-       *
-       *  2.
-       *
-       *    Ts1 & ... & Tsm <: P   (m > 0)
-       *    Tsi = T1 | ... | Tn    (n >= 0)
-       *    Some of the Ti are constrained parameters
-       *
-       *  In each case we cannot leave the parameter in place,
-       *  because that would risk making a parameter later a subtype or supertype
-       *  of a bound where the parameter occurs again at toplevel, which leads to cycles
-       *  in the subtyping test. So we intentionally narrow the constraint by
-       *  recording an isLess relationship instead (even though this is not implied
-       *  by the bound).
-       *
-       *  Normally, narrowing a constraint is better than widening it, because
-       *  narrowing leads to incompleteness (which we face anyway, see for
-       *  instance `TypeComparer#either`) but widening leads to unsoundness,
-       *  but note the special handling in `ConstrainResult` mode below.
-       *
-       *  A test case that demonstrates the problem is i864.scala.
-       *  Turn Config.checkConstraintsSeparated on to get an accurate diagnostic
-       *  of the cycle when it is created.
-       *
-       *  @return The pruned type if all `addLess` calls succeed, `NoType` otherwise.
+       *   An occurrence is toplevel if it is the bound itself,
+       *   or the bound is a union or intersection, and the ocurrence is
+       *   toplevel in one of the operands of the `&` or `|`.
        */
-      def prune(bnd: Type): Type = bnd match {
-        case bnd: AndType if !fromBelow =>
+      def prune(bnd: Type): Type = bnd match
+        case bnd: AndOrType =>
           val p1 = prune(bnd.tp1)
           val p2 = prune(bnd.tp2)
-          if (p1.exists && p2.exists) bnd.derivedAndType(p1, p2)
+          if (p1.exists && p2.exists) bnd.derivedAndOrType(p1, p2)
           else NoType
-        case bnd: OrType if fromBelow =>
-          val p1 = prune(bnd.tp1)
-          val p2 = prune(bnd.tp2)
-          if (p1.exists && p2.exists) bnd.derivedOrType(p1, p2)
-          else NoType
-        case bnd: TypeVar if constraint contains bnd.origin =>
+        case bnd: TypeVar if constraint contains bnd.origin => // (2)
           prune(bnd.underlying)
-        case bnd: TypeParamRef if bnd ne param =>
-          constraint.entry(bnd) match {
-            case NoType => pruneLambdaParams(bnd)
-            case _: TypeBounds =>
-              assertFail(i"pruning $param $bound $fromBelow $bnd")
-              if (!addParamBound(bnd)) NoType
-              else if (fromBelow) defn.NothingType
-              else defn.AnyType
-            case inst =>
-              prune(inst)
-          }
-        case bnd: ExprType =>
+        case bnd: TypeParamRef =>
+          if bnd eq param then // (1)
+            if fromBelow then defn.NothingType else defn.AnyType
+          else constraint.entry(bnd) match
+            case _: TypeBounds => bnd
+            case inst => prune(inst) // (3)
+        case bnd: ExprType => // (4)
           // ExprTypes are not value types, so type parameters should not
           // be instantiated to ExprTypes. A scenario where such an attempted
           // instantiation can happen is if we unify (=> T) => () with A => ()
@@ -531,8 +501,7 @@ trait ConstraintHandling[AbstractContext] {
           // the resulting types down) and is largely unknown terrain.
           NoType
         case _ =>
-          pruneLambdaParams(bnd)
-      }
+          bnd
 
       def kindCompatible(tp1: Type, tp2: Type): Boolean =
         val tparams1 = tp1.typeParams
@@ -547,7 +516,8 @@ trait ConstraintHandling[AbstractContext] {
           addParamBound(bound)
         case _ =>
           val savedConstraint = constraint
-          val pbound = prune(bound)
+          val pbound = prune(pruneLambdaParams(bound))
+          assert(constraint eq savedConstraint)
           val constraintsNarrowed = constraint ne savedConstraint
 
           val res =

From 94f3f0cd17a05287e418b9e7c299b04fbc92c018 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 29 Apr 2020 11:27:11 +0200
Subject: [PATCH 03/22] Simplify some more

---
 .../tools/dotc/core/ConstraintHandling.scala  | 25 ++++---------------
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
index f0ff31005cca..74778d9221f0 100644
--- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
+++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
@@ -422,7 +422,6 @@ trait ConstraintHandling[AbstractContext] {
     def description = i"constr $param ${if (fromBelow) ">:" else "<:"} $bound:\n$constraint"
     //checkPropagated(s"adding $description")(true) // DEBUG in case following fails
     checkPropagated(s"added $description") {
-      addConstraintInvocations += 1
 
       /** When comparing lambdas we might get constraints such as
        *  `A <: X0` or `A = List[X0]` where `A` is a constrained parameter
@@ -511,29 +510,15 @@ trait ConstraintHandling[AbstractContext] {
         || tp1.hasAnyKind
         || tp2.hasAnyKind
 
-      try bound match {
+      addConstraintInvocations += 1
+      try bound match
         case bound: TypeParamRef if constraint contains bound =>
           addParamBound(bound)
         case _ =>
-          val savedConstraint = constraint
           val pbound = prune(pruneLambdaParams(bound))
-          assert(constraint eq savedConstraint)
-          val constraintsNarrowed = constraint ne savedConstraint
-
-          val res =
-            pbound.exists
-            && kindCompatible(param, pbound)
-            && (if fromBelow then addLowerBound(param, pbound) else addUpperBound(param, pbound))
-          // If we're in `ConstrainResult` mode, we don't want to commit to a
-          // set of constraints that would later prevent us from typechecking
-          // arguments, so if `pruneParams` had to narrow the constraints, we
-          // simply do not record any new constraint.
-          // Unlike in `TypeComparer#either`, the same reasoning does not apply
-          // to GADT mode because this code is never run on GADT constraints.
-          if ctx.mode.is(Mode.ConstrainResult) && constraintsNarrowed then
-            constraint = savedConstraint
-          res
-      }
+          pbound.exists
+          && kindCompatible(param, pbound)
+          && (if fromBelow then addLowerBound(param, pbound) else addUpperBound(param, pbound))
       finally addConstraintInvocations -= 1
     }
   }

From 247cc861c4092bc805a30b51a19ea2e368883329 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 29 Apr 2020 11:33:04 +0200
Subject: [PATCH 04/22] Rearrange parts of ConstraintHandling for legibility

---
 .../tools/dotc/core/ConstraintHandling.scala  | 175 +++++++++---------
 1 file changed, 88 insertions(+), 87 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
index 74778d9221f0..b104a68a7d83 100644
--- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
+++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
@@ -418,98 +418,99 @@ trait ConstraintHandling[AbstractContext] {
    *  not be AndTypes and lower bounds may not be OrTypes. This is assured by the
    *  way isSubType is organized.
    */
-  protected def addConstraint(param: TypeParamRef, bound: Type, fromBelow: Boolean)(implicit actx: AbstractContext): Boolean = {
-    def description = i"constr $param ${if (fromBelow) ">:" else "<:"} $bound:\n$constraint"
-    //checkPropagated(s"adding $description")(true) // DEBUG in case following fails
-    checkPropagated(s"added $description") {
-
-      /** When comparing lambdas we might get constraints such as
-       *  `A <: X0` or `A = List[X0]` where `A` is a constrained parameter
-       *  and `X0` is a lambda parameter. The constraint for `A` is not allowed
-       *  to refer to such a lambda parameter because the lambda parameter is
-       *  not visible where `A` is defined. Consequently, we need to
-       *  approximate the bound so that the lambda parameter does not appear in it.
-       *  If `tp` is an upper bound, we need to approximate with something smaller,
-       *  otherwise something larger.
-       *  Test case in pos/i94-nada.scala. This test crashes with an illegal instance
-       *  error in Test2 when the rest of the SI-2712 fix is applied but `pruneLambdaParams` is
-       *  missing.
-       */
-      def pruneLambdaParams(tp: Type) =
-        if (comparedTypeLambdas.nonEmpty) {
-          val approx = new ApproximatingTypeMap {
-            if (!fromBelow) variance = -1
-            def apply(t: Type): Type = t match {
-              case t @ TypeParamRef(tl: TypeLambda, n) if comparedTypeLambdas contains tl =>
-                val bounds = tl.paramInfos(n)
-                range(bounds.lo, bounds.hi)
-              case _ =>
-                mapOver(t)
-            }
+  protected def addConstraint(param: TypeParamRef, bound: Type, fromBelow: Boolean)(implicit actx: AbstractContext): Boolean =
+
+    /** When comparing lambdas we might get constraints such as
+     *  `A <: X0` or `A = List[X0]` where `A` is a constrained parameter
+     *  and `X0` is a lambda parameter. The constraint for `A` is not allowed
+     *  to refer to such a lambda parameter because the lambda parameter is
+     *  not visible where `A` is defined. Consequently, we need to
+     *  approximate the bound so that the lambda parameter does not appear in it.
+     *  If `tp` is an upper bound, we need to approximate with something smaller,
+     *  otherwise something larger.
+     *  Test case in pos/i94-nada.scala. This test crashes with an illegal instance
+     *  error in Test2 when the rest of the SI-2712 fix is applied but `pruneLambdaParams` is
+     *  missing.
+     */
+    def pruneLambdaParams(tp: Type) =
+      if (comparedTypeLambdas.nonEmpty) {
+        val approx = new ApproximatingTypeMap {
+          if (!fromBelow) variance = -1
+          def apply(t: Type): Type = t match {
+            case t @ TypeParamRef(tl: TypeLambda, n) if comparedTypeLambdas contains tl =>
+              val bounds = tl.paramInfos(n)
+              range(bounds.lo, bounds.hi)
+            case _ =>
+              mapOver(t)
           }
-          approx(tp)
         }
-        else tp
+        approx(tp)
+      }
+      else tp
+
+    def addParamBound(bound: TypeParamRef) =
+      constraint.entry(param) match {
+        case _: TypeBounds =>
+          if (fromBelow) addLess(bound, param) else addLess(param, bound)
+        case tp =>
+          if (fromBelow) isSubType(bound, tp) else isSubType(tp, bound)
+      }
 
-      def addParamBound(bound: TypeParamRef) =
-        constraint.entry(param) match {
-          case _: TypeBounds =>
-            if (fromBelow) addLess(bound, param) else addLess(param, bound)
-          case tp =>
-            if (fromBelow) isSubType(bound, tp) else isSubType(tp, bound)
-        }
+    /** Normalize the bound `bnd` in the following ways:
+     *
+     *   1. Any toplevel occurrences of the compared parameter `param` are
+     *      replaced by `Nothing` if bound is from below or `Any` otherwise.
+     *   2. Toplevel occurrences of TypeVars over TypeRefs in the current
+     *      constraint are dereferenced.
+     *   3. Toplevel occurrences of TypeRefs that are instantiated in the current
+     *      constraint are also referenced.
+     *   4. Toplevel occurrences of ExprTypes lead to a `NoType` return, which
+     *      causes the addConstraint operation to fail.
+     *
+     *   An occurrence is toplevel if it is the bound itself,
+     *   or the bound is a union or intersection, and the ocurrence is
+     *   toplevel in one of the operands of the `&` or `|`.
+     */
+    def prune(bnd: Type): Type = bnd match
+      case bnd: AndOrType =>
+        val p1 = prune(bnd.tp1)
+        val p2 = prune(bnd.tp2)
+        if (p1.exists && p2.exists) bnd.derivedAndOrType(p1, p2)
+        else NoType
+      case bnd: TypeVar if constraint contains bnd.origin => // (2)
+        prune(bnd.underlying)
+      case bnd: TypeParamRef =>
+        if bnd eq param then // (1)
+          if fromBelow then defn.NothingType else defn.AnyType
+        else constraint.entry(bnd) match
+          case _: TypeBounds => bnd
+          case inst => prune(inst) // (3)
+      case bnd: ExprType => // (4)
+        // ExprTypes are not value types, so type parameters should not
+        // be instantiated to ExprTypes. A scenario where such an attempted
+        // instantiation can happen is if we unify (=> T) => () with A => ()
+        // where A is a TypeParamRef. See the comment on EtaExpansion.etaExpand
+        // why types such as (=> T) => () can be constructed and i7969.scala
+        // as a test where this happens.
+        // Note that scalac by contrast allows such instantiations. But letting
+        // type variables be ExprTypes has its own problems (e.g. you can't write
+        // the resulting types down) and is largely unknown terrain.
+        NoType
+      case _ =>
+        bnd
 
-      /** Normalize the bound `bnd` in the following ways:
-       *
-       *   1. Any toplevel occurrences of the compared parameter `param` are
-       *      replaced by `Nothing` if bound is from below or `Any` otherwise.
-       *   2. Toplevel occurrences of TypeVars over TypeRefs in the current
-       *      constraint are dereferenced.
-       *   3. Toplevel occurrences of TypeRefs that are instantiated in the current
-       *      constraint are also referenced.
-       *   4. Toplevel occurrences of ExprTypes lead to a `NoType` return, which
-       *      causes the addConstraint operation to fail.
-       *
-       *   An occurrence is toplevel if it is the bound itself,
-       *   or the bound is a union or intersection, and the ocurrence is
-       *   toplevel in one of the operands of the `&` or `|`.
-       */
-      def prune(bnd: Type): Type = bnd match
-        case bnd: AndOrType =>
-          val p1 = prune(bnd.tp1)
-          val p2 = prune(bnd.tp2)
-          if (p1.exists && p2.exists) bnd.derivedAndOrType(p1, p2)
-          else NoType
-        case bnd: TypeVar if constraint contains bnd.origin => // (2)
-          prune(bnd.underlying)
-        case bnd: TypeParamRef =>
-          if bnd eq param then // (1)
-            if fromBelow then defn.NothingType else defn.AnyType
-          else constraint.entry(bnd) match
-            case _: TypeBounds => bnd
-            case inst => prune(inst) // (3)
-        case bnd: ExprType => // (4)
-          // ExprTypes are not value types, so type parameters should not
-          // be instantiated to ExprTypes. A scenario where such an attempted
-          // instantiation can happen is if we unify (=> T) => () with A => ()
-          // where A is a TypeParamRef. See the comment on EtaExpansion.etaExpand
-          // why types such as (=> T) => () can be constructed and i7969.scala
-          // as a test where this happens.
-          // Note that scalac by contrast allows such instantiations. But letting
-          // type variables be ExprTypes has its own problems (e.g. you can't write
-          // the resulting types down) and is largely unknown terrain.
-          NoType
-        case _ =>
-          bnd
+    def kindCompatible(tp1: Type, tp2: Type): Boolean =
+      val tparams1 = tp1.typeParams
+      val tparams2 = tp2.typeParams
+      tparams1.corresponds(tparams2)((p1, p2) => kindCompatible(p1.paramInfo, p2.paramInfo))
+      && (tparams1.isEmpty || kindCompatible(tp1.hkResult, tp2.hkResult))
+      || tp1.hasAnyKind
+      || tp2.hasAnyKind
 
-      def kindCompatible(tp1: Type, tp2: Type): Boolean =
-        val tparams1 = tp1.typeParams
-        val tparams2 = tp2.typeParams
-        tparams1.corresponds(tparams2)((p1, p2) => kindCompatible(p1.paramInfo, p2.paramInfo))
-        && (tparams1.isEmpty || kindCompatible(tp1.hkResult, tp2.hkResult))
-        || tp1.hasAnyKind
-        || tp2.hasAnyKind
+    def description = i"constr $param ${if (fromBelow) ">:" else "<:"} $bound:\n$constraint"
 
+    //checkPropagated(s"adding $description")(true) // DEBUG in case following fails
+    checkPropagated(s"added $description") {
       addConstraintInvocations += 1
       try bound match
         case bound: TypeParamRef if constraint contains bound =>
@@ -521,7 +522,7 @@ trait ConstraintHandling[AbstractContext] {
           && (if fromBelow then addLowerBound(param, pbound) else addUpperBound(param, pbound))
       finally addConstraintInvocations -= 1
     }
-  }
+  end addConstraint
 
   /** Check that constraint is fully propagated. See comment in Config.checkConstraintsPropagated */
   def checkPropagated(msg: => String)(result: Boolean)(implicit actx: AbstractContext): Boolean = {

From 14689bfa106c297623587acd9cabfe9b7e9b832a Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 29 Apr 2020 16:17:19 +0200
Subject: [PATCH 05/22] Avoid cycles when adding bounds to constraints

---
 .../tools/dotc/core/ConstraintHandling.scala  | 37 +++++++++++--------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
index b104a68a7d83..2bb16c1ce529 100644
--- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
+++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
@@ -86,22 +86,24 @@ trait ConstraintHandling[AbstractContext] {
   def fullBounds(param: TypeParamRef)(implicit actx: AbstractContext): TypeBounds =
     nonParamBounds(param).derivedTypeBounds(fullLowerBound(param), fullUpperBound(param))
 
-  protected def addOneBound(param: TypeParamRef, bound: Type, isUpper: Boolean)(implicit actx: AbstractContext): Boolean =
+  protected def addOneBound(param: TypeParamRef, rawBound: Type, isUpper: Boolean)(implicit actx: AbstractContext): Boolean =
     !constraint.contains(param) || {
-      def occursIn(bound: Type): Boolean = {
-        val b = bound.dealias
-        (b eq param) || {
-          b match {
-            case b: AndType => occursIn(b.tp1) || occursIn(b.tp2)
-            case b: OrType  => occursIn(b.tp1) || occursIn(b.tp2)
-            case b: TypeVar => occursIn(b.origin)
-            case b: TermRef => occursIn(b.underlying)
-            case _ => false
-          }
-        }
-      }
-      if (Config.checkConstraintsSeparated)
-        assert(!occursIn(bound), s"$param occurs in $bound")
+      def avoidCycle(tp: Type): Type = tp match
+        case tp: AndOrType =>
+          tp.derivedAndOrType(avoidCycle(tp.tp1), avoidCycle(tp.tp2))
+        case tp: TypeParamRef =>
+          if tp eq param then
+            //println(i"stripping $tp from $rawBound, upper = $isUpper in $constraint")
+            if isUpper then defn.AnyType else defn.NothingType
+          else constraint.entry(tp) match
+            case TypeBounds(lo, hi) => if lo eq hi then avoidCycle(lo) else tp
+            case inst => avoidCycle(inst)
+        case tp: TypeVar =>
+          val underlying1 = avoidCycle(tp.underlying)
+          if underlying1 eq tp.underlying then tp else underlying1
+        case _ =>
+          tp
+      val bound = avoidCycle(rawBound)
 
       val oldBounds @ TypeBounds(lo, hi) = constraint.nonParamBounds(param)
       val equalBounds = isUpper && (lo eq bound) || !isUpper && (bound eq hi)
@@ -275,6 +277,9 @@ trait ConstraintHandling[AbstractContext] {
             //  from above  | hi  lo  lo
             //
             if (variance == 0 || fromBelow == (variance < 0)) bounds.lo else bounds.hi
+          case `param` =>
+            if variance == 0 || fromBelow == (variance < 0) then defn.AnyType
+            else defn.NothingType
           case _ => tp
         }
       }
@@ -483,7 +488,7 @@ trait ConstraintHandling[AbstractContext] {
         if bnd eq param then // (1)
           if fromBelow then defn.NothingType else defn.AnyType
         else constraint.entry(bnd) match
-          case _: TypeBounds => bnd
+          case TypeBounds(lo, hi) => if lo eq hi then prune(lo) else bnd
           case inst => prune(inst) // (3)
       case bnd: ExprType => // (4)
         // ExprTypes are not value types, so type parameters should not

From f8765379300ecf5a2626fbdb0445bbac584da69a Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 29 Apr 2020 16:44:57 +0200
Subject: [PATCH 06/22] Disable three scalatest tests

---
 community-build/community-projects/scalatest | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/community-build/community-projects/scalatest b/community-build/community-projects/scalatest
index 11be13672aa8..4a2fe34ebba0 160000
--- a/community-build/community-projects/scalatest
+++ b/community-build/community-projects/scalatest
@@ -1 +1 @@
-Subproject commit 11be13672aa8ac4e634cd396524275f3634e544f
+Subproject commit 4a2fe34ebba038b7145248632f8523ce53d6c76e

From bf55005c3c32f91b7f2c0fa0fead30e550bdcda3 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 29 Apr 2020 17:52:59 +0200
Subject: [PATCH 07/22] Integrate prune logic in addOneBound

New cyclic bounds can also appear as a consequence of unification.
The latter goes through addOneBound, but not through addConstraint.
So the prune logic that avoids cycles should go to addOneBound.

ExprType checking could logically stay with addConstraint but it
was simpler to move it as well.
---
 .../tools/dotc/core/ConstraintHandling.scala  | 133 ++++++++----------
 1 file changed, 60 insertions(+), 73 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
index 2bb16c1ce529..a007790306ab 100644
--- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
+++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
@@ -87,55 +87,87 @@ trait ConstraintHandling[AbstractContext] {
     nonParamBounds(param).derivedTypeBounds(fullLowerBound(param), fullUpperBound(param))
 
   protected def addOneBound(param: TypeParamRef, rawBound: Type, isUpper: Boolean)(implicit actx: AbstractContext): Boolean =
-    !constraint.contains(param) || {
-      def avoidCycle(tp: Type): Type = tp match
-        case tp: AndOrType =>
-          tp.derivedAndOrType(avoidCycle(tp.tp1), avoidCycle(tp.tp2))
-        case tp: TypeParamRef =>
-          if tp eq param then
-            //println(i"stripping $tp from $rawBound, upper = $isUpper in $constraint")
-            if isUpper then defn.AnyType else defn.NothingType
-          else constraint.entry(tp) match
-            case TypeBounds(lo, hi) => if lo eq hi then avoidCycle(lo) else tp
-            case inst => avoidCycle(inst)
-        case tp: TypeVar =>
-          val underlying1 = avoidCycle(tp.underlying)
-          if underlying1 eq tp.underlying then tp else underlying1
-        case _ =>
-          tp
-      val bound = avoidCycle(rawBound)
+
+    /** Adjust the bound `tp` in the following ways:
+     *
+     *   1. Any toplevel occurrences of the compared parameter `param` are
+     *      replaced by `Nothing` if bound is from below or `Any` otherwise.
+     *   2. Toplevel occurrences of TypeVars over TypeRefs in the current
+     *      constraint are dereferenced.
+     *   3. Toplevel occurrences of TypeRefs that are instantiated in the current
+     *      constraint are also dereferenced.
+     *   4. Toplevel occurrences of ExprTypes lead to a `NoType` return, which
+     *      causes the addOneBound operation to fail.
+     *
+     *   An occurrence is toplevel if it is the bound itself,
+     *   or the bound is a union or intersection, and the ocurrence is
+     *   toplevel in one of the operands of the `&` or `|`.
+     */
+    def adjust(tp: Type): Type = tp match
+      case tp: AndOrType =>
+        val p1 = adjust(tp.tp1)
+        val p2 = adjust(tp.tp2)
+        if p1.exists && p2.exists then tp.derivedAndOrType(p1, p2) else NoType
+      case tp: TypeParamRef =>
+        if tp eq param then // (1)
+          //println(i"stripping $tp from $rawBound, upper = $isUpper in $constraint")
+          if isUpper then defn.AnyType else defn.NothingType
+        else constraint.entry(tp) match  // (3)
+          case TypeBounds(lo, hi) => if lo eq hi then adjust(lo) else tp
+          case inst => adjust(inst)
+      case tp: TypeVar => // (2)
+        val underlying1 = adjust(tp.underlying)
+        if (underlying1 ne tp.underlying) || constraint.contains(tp.origin)
+        then underlying1
+        else tp
+      case tp: ExprType => // (4)
+        // ExprTypes are not value types, so type parameters should not
+        // be instantiated to ExprTypes. A scenario where such an attempted
+        // instantiation can happen is if we unify (=> T) => () with A => ()
+        // where A is a TypeParamRef. See the comment on EtaExpansion.etaExpand
+        // why types such as (=> T) => () can be constructed and i7969.scala
+        // as a test where this happens.
+        // Note that scalac by contrast allows such instantiations. But letting
+        // type variables be ExprTypes has its own problems (e.g. you can't write
+        // the resulting types down) and is largely unknown terrain.
+        NoType
+      case _ =>
+        tp
+
+    if !constraint.contains(param) then true
+    else
+      val bound = adjust(rawBound)
 
       val oldBounds @ TypeBounds(lo, hi) = constraint.nonParamBounds(param)
       val equalBounds = isUpper && (lo eq bound) || !isUpper && (bound eq hi)
-      if (equalBounds &&
-         !bound.existsPart(bp => bp.isInstanceOf[WildcardType] || (bp eq param))) {
+      if !bound.exists then false
+      else if equalBounds
+        && !bound.existsPart(bp => bp.isInstanceOf[WildcardType] || (bp eq param))
+      then
         // The narrowed bounds are equal and do not contain wildcards,
         // so we can remove `param` from the constraint.
         // (Handling wildcards requires choosing a bound, but we don't know which
         // bound to choose here, this is handled in `ConstraintHandling#approximation`)
         constraint = constraint.replace(param, bound)
         true
-      }
-      else {
+      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 narrowedBounds =
           val saved = homogenizeArgs
           homogenizeArgs = Config.alignArgsInAnd
           try
             if (isUpper) oldBounds.derivedTypeBounds(lo, hi & bound)
             else oldBounds.derivedTypeBounds(lo | bound, hi)
           finally homogenizeArgs = saved
-        }
         val c1 = constraint.updateEntry(param, narrowedBounds)
         (c1 eq constraint) || {
           constraint = c1
           val TypeBounds(lo, hi) = constraint.entry(param)
           isSubType(lo, hi)
         }
-      }
-    }
+  end addOneBound
 
   private def location(implicit ctx: Context) = "" // i"in ${ctx.typerState.stateChainStr}" // use for debugging
 
@@ -201,7 +233,6 @@ trait ConstraintHandling[AbstractContext] {
     up.forall(addOneBound(_, lo, isUpper = false))
   }
 
-
   protected def isSubType(tp1: Type, tp2: Type, whenFrozen: Boolean)(implicit actx: AbstractContext): Boolean =
     if (whenFrozen)
       isSubTypeWhenFrozen(tp1, tp2)
@@ -437,7 +468,7 @@ trait ConstraintHandling[AbstractContext] {
      *  error in Test2 when the rest of the SI-2712 fix is applied but `pruneLambdaParams` is
      *  missing.
      */
-    def pruneLambdaParams(tp: Type) =
+    def avoidLambdaParams(tp: Type) =
       if (comparedTypeLambdas.nonEmpty) {
         val approx = new ApproximatingTypeMap {
           if (!fromBelow) variance = -1
@@ -461,49 +492,6 @@ trait ConstraintHandling[AbstractContext] {
           if (fromBelow) isSubType(bound, tp) else isSubType(tp, bound)
       }
 
-    /** Normalize the bound `bnd` in the following ways:
-     *
-     *   1. Any toplevel occurrences of the compared parameter `param` are
-     *      replaced by `Nothing` if bound is from below or `Any` otherwise.
-     *   2. Toplevel occurrences of TypeVars over TypeRefs in the current
-     *      constraint are dereferenced.
-     *   3. Toplevel occurrences of TypeRefs that are instantiated in the current
-     *      constraint are also referenced.
-     *   4. Toplevel occurrences of ExprTypes lead to a `NoType` return, which
-     *      causes the addConstraint operation to fail.
-     *
-     *   An occurrence is toplevel if it is the bound itself,
-     *   or the bound is a union or intersection, and the ocurrence is
-     *   toplevel in one of the operands of the `&` or `|`.
-     */
-    def prune(bnd: Type): Type = bnd match
-      case bnd: AndOrType =>
-        val p1 = prune(bnd.tp1)
-        val p2 = prune(bnd.tp2)
-        if (p1.exists && p2.exists) bnd.derivedAndOrType(p1, p2)
-        else NoType
-      case bnd: TypeVar if constraint contains bnd.origin => // (2)
-        prune(bnd.underlying)
-      case bnd: TypeParamRef =>
-        if bnd eq param then // (1)
-          if fromBelow then defn.NothingType else defn.AnyType
-        else constraint.entry(bnd) match
-          case TypeBounds(lo, hi) => if lo eq hi then prune(lo) else bnd
-          case inst => prune(inst) // (3)
-      case bnd: ExprType => // (4)
-        // ExprTypes are not value types, so type parameters should not
-        // be instantiated to ExprTypes. A scenario where such an attempted
-        // instantiation can happen is if we unify (=> T) => () with A => ()
-        // where A is a TypeParamRef. See the comment on EtaExpansion.etaExpand
-        // why types such as (=> T) => () can be constructed and i7969.scala
-        // as a test where this happens.
-        // Note that scalac by contrast allows such instantiations. But letting
-        // type variables be ExprTypes has its own problems (e.g. you can't write
-        // the resulting types down) and is largely unknown terrain.
-        NoType
-      case _ =>
-        bnd
-
     def kindCompatible(tp1: Type, tp2: Type): Boolean =
       val tparams1 = tp1.typeParams
       val tparams2 = tp2.typeParams
@@ -521,9 +509,8 @@ trait ConstraintHandling[AbstractContext] {
         case bound: TypeParamRef if constraint contains bound =>
           addParamBound(bound)
         case _ =>
-          val pbound = prune(pruneLambdaParams(bound))
-          pbound.exists
-          && kindCompatible(param, pbound)
+          val pbound = avoidLambdaParams(bound)
+          kindCompatible(param, pbound)
           && (if fromBelow then addLowerBound(param, pbound) else addUpperBound(param, pbound))
       finally addConstraintInvocations -= 1
     }

From 788be025da3f289c4a8e1bbb71ea151ad14d27bc Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 29 Apr 2020 18:08:40 +0200
Subject: [PATCH 08/22] Drop redundant Config option

Instead of checking that constraints are "separated"
in addOneBound, we now ensure that they are.
---
 compiler/src/dotty/tools/dotc/config/Config.scala | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala
index b97529296c19..6bad889890d4 100644
--- a/compiler/src/dotty/tools/dotc/config/Config.scala
+++ b/compiler/src/dotty/tools/dotc/config/Config.scala
@@ -15,13 +15,6 @@ object Config {
    */
   final val checkConstraintsNonCyclic = false
 
-  /** Make sure none of the bounds of a parameter in an OrderingConstraint
-   *  contains this parameter at its toplevel (i.e. as an operand of a
-   *  combination of &'s and |'s.). The check is performed each time a new bound
-   *  is added to the constraint.
-   */
-  final val checkConstraintsSeparated = false
-
   /** Check that each constraint resulting from a subtype test
    *  is satisfiable.
    */

From 74d81ab3a5793e53ede555023e8b31bc7b83aeb7 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 29 Apr 2020 18:24:47 +0200
Subject: [PATCH 09/22] Drop redundant code

The dropped lines were added as an intermediate stage before we
settled on doing this in addOneBound.
---
 compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
index a007790306ab..4ec8a1af8b66 100644
--- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
+++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
@@ -308,9 +308,6 @@ trait ConstraintHandling[AbstractContext] {
             //  from above  | hi  lo  lo
             //
             if (variance == 0 || fromBelow == (variance < 0)) bounds.lo else bounds.hi
-          case `param` =>
-            if variance == 0 || fromBelow == (variance < 0) then defn.AnyType
-            else defn.NothingType
           case _ => tp
         }
       }

From c58b23f166c853f0feda0a8ba9668a8cd6b6873c Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 29 Apr 2020 21:39:48 +0200
Subject: [PATCH 10/22] Fix adjust

Need to take into account type parameters that are not bound
by constraint
---
 compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
index 4ec8a1af8b66..94026888e374 100644
--- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
+++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
@@ -109,10 +109,12 @@ trait ConstraintHandling[AbstractContext] {
         val p2 = adjust(tp.tp2)
         if p1.exists && p2.exists then tp.derivedAndOrType(p1, p2) else NoType
       case tp: TypeParamRef =>
+        if constraint.contains(tp) then
+          constr.println(i"${if tp eq param then "stripping" else "keeping"} $tp from $rawBound, upper = $isUpper in $constraint")
         if tp eq param then // (1)
-          //println(i"stripping $tp from $rawBound, upper = $isUpper in $constraint")
           if isUpper then defn.AnyType else defn.NothingType
         else constraint.entry(tp) match  // (3)
+          case NoType => tp
           case TypeBounds(lo, hi) => if lo eq hi then adjust(lo) else tp
           case inst => adjust(inst)
       case tp: TypeVar => // (2)

From 9ebdac43334a58af3912beed617eda5e8b048087 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Thu, 30 Apr 2020 15:27:24 +0200
Subject: [PATCH 11/22] Improve handling of printing unique ids on debug

---
 compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala   | 6 ++++--
 compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
index a565e593923d..100891a50df0 100644
--- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
+++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
@@ -26,6 +26,8 @@ class PlainPrinter(_ctx: Context) extends Printer {
 
   protected def maxToTextRecursions: Int = 100
 
+  protected def showUniqueIds = ctx.settings.uniqid.value || Printer.debugPrintUnique
+
   protected final def limiter: MessageLimiter = ctx.property(MessageLimiter).get
 
   protected def controlled(op: => Text): Text = limiter.controlled(op)
@@ -248,14 +250,14 @@ class PlainPrinter(_ctx: Context) extends Printer {
 
   /** If -uniqid is set, the hashcode of the lambda type, after a # */
   protected def lambdaHash(pt: LambdaType): Text =
-    if (ctx.settings.uniqid.value)
+    if (showUniqueIds)
       try "#" + pt.hashCode
       catch { case ex: NullPointerException => "" }
     else ""
 
   /** If -uniqid is set, the unique id of symbol, after a # */
   protected def idString(sym: Symbol): String =
-    if (ctx.settings.uniqid.value || Printer.debugPrintUnique) "#" + sym.id else ""
+    if (showUniqueIds || Printer.debugPrintUnique) "#" + sym.id else ""
 
   def nameString(sym: Symbol): String =
     simpleNameString(sym) + idString(sym) // + "<" + (if (sym.exists) sym.owner else "") + ">"
diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
index 98d231de643c..50be7e0e4558 100644
--- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
+++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
@@ -213,7 +213,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
       case tp: RefinedType if defn.isFunctionType(tp) && !printDebug =>
         toTextDependentFunction(tp.refinedInfo.asInstanceOf[MethodType])
       case tp: TypeRef =>
-        if (tp.symbol.isAnonymousClass && !ctx.settings.uniqid.value)
+        if (tp.symbol.isAnonymousClass && !showUniqueIds)
           toText(tp.info)
         else if (tp.symbol.is(Param))
           tp.prefix match {
@@ -729,7 +729,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
   protected def optAscription[T >: Untyped](tpt: Tree[T]): Text = optText(tpt)(": " ~ _)
 
   private def idText(tree: untpd.Tree): Text =
-    if ((ctx.settings.uniqid.value || Printer.debugPrintUnique) && tree.hasType && tree.symbol.exists) s"#${tree.symbol.id}" else ""
+    if showUniqueIds && tree.hasType && tree.symbol.exists then s"#${tree.symbol.id}" else ""
 
   private def useSymbol(tree: untpd.Tree) =
     tree.hasType && tree.symbol.exists && ctx.settings.YprintSyms.value

From 462279c3df915b8cf21b0895faad9abe24c3e931 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Thu, 30 Apr 2020 15:53:38 +0200
Subject: [PATCH 12/22] Refactor OrderingConstraint

Make sure cycles are detected and avoided in OrderingConstraint itself
---
 .../dotty/tools/dotc/core/Constraint.scala    |   2 +-
 .../tools/dotc/core/OrderingConstraint.scala  | 293 +++++++++---------
 2 files changed, 151 insertions(+), 144 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/Constraint.scala b/compiler/src/dotty/tools/dotc/core/Constraint.scala
index b40b806c85bb..d10841530e9c 100644
--- a/compiler/src/dotty/tools/dotc/core/Constraint.scala
+++ b/compiler/src/dotty/tools/dotc/core/Constraint.scala
@@ -151,7 +151,7 @@ abstract class Constraint extends Showable {
   def & (other: Constraint, otherHasErrors: Boolean)(implicit ctx: Context): Constraint
 
   /** Check that no constrained parameter contains itself as a bound */
-  def checkNonCyclic()(implicit ctx: Context): Unit
+  def checkNonCyclic()(implicit ctx: Context): this.type
 
   /** Check that constraint only refers to TypeParamRefs bound by itself */
   def checkClosed()(implicit ctx: Context): Unit
diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
index 54527637231a..13d476564b09 100644
--- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
+++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
@@ -26,7 +26,6 @@ object OrderingConstraint {
   /** A new constraint with given maps */
   private def newConstraint(boundsMap: ParamBounds, lowerMap: ParamOrdering, upperMap: ParamOrdering)(implicit ctx: Context) : OrderingConstraint = {
     val result = new OrderingConstraint(boundsMap, lowerMap, upperMap)
-    if (Config.checkConstraintsNonCyclic) result.checkNonCyclic()
     ctx.run.recordConstraintSize(result, result.boundsMap.size)
     result
   }
@@ -207,20 +206,6 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
 
 // ---------- Adding TypeLambdas --------------------------------------------------
 
-  /** The list of parameters P such that, for a fresh type parameter Q:
-   *
-   *    Q <: tp  implies  Q <: P      and isUpper = true, or
-   *    tp <: Q  implies  P <: Q      and isUpper = false
-   */
-  def dependentParams(tp: Type, isUpper: Boolean): List[TypeParamRef] = tp match {
-    case param: TypeParamRef if contains(param) =>
-      param :: (if (isUpper) upper(param) else lower(param))
-    case tp: AndType => dependentParams(tp.tp1, isUpper) | (dependentParams(tp.tp2, isUpper))
-    case tp: OrType  => dependentParams(tp.tp1, isUpper).intersect(dependentParams(tp.tp2, isUpper))
-    case _ =>
-      Nil
-  }
-
   /** The bound type `tp` without constrained parameters which are clearly
    *  dependent. A parameter in an upper bound is clearly dependent if it appears
    *  in a hole of a context H given by:
@@ -271,6 +256,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
 
   /** The bound type `tp` without clearly dependent parameters.
    *  A top or bottom type if type consists only of dependent parameters.
+   *  TODO: try to do without normalization? It would mean it is more efficient
+   *  to pull out full bounds from a constraint.
    *  @param isUpper   If true, `bound` is an upper bound, else a lower bound.
    */
   private def normalizedType(tp: Type, paramBuf: mutable.ListBuffer[TypeParamRef],
@@ -307,12 +294,49 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
       hiBuf.clear()
       i += 1
     }
-    if (Config.checkConstraintsNonCyclic) checkNonCyclic()
-    current
+    current.checkNonCyclic()
   }
 
 // ---------- Updates ------------------------------------------------------------
 
+  /** If `inst` is a TypeBounds, make sure it does not contain toplevel
+   *  references to `param`. Toplevel means: the term itself or a factor in some
+   *  combination of `&` or `|` types.
+   *  Any such references are replace by `Nothing` in the lower bound and `Any`
+   *  in the upper bound.
+   *  References can be direct or indirect through instantiations of other
+   *  parameters in the constraint.
+   */
+  private def ensureNonCyclic(param: TypeParamRef, inst: Type)(using Context): Type =
+
+    def recur(tp: Type, fromBelow: Boolean): Type = tp match
+      case tp: AndOrType =>
+        val r1 = recur(tp.tp1, fromBelow)
+        val r2 = recur(tp.tp2, fromBelow)
+        if (r1 eq tp.tp1) && (r2 eq tp.tp2) then tp
+        else if tp.isAnd then r1 & r2
+        else r1 | r2
+      case tp: TypeParamRef =>
+        if tp eq param then
+          if fromBelow then defn.NothingType else defn.AnyType
+        else entry(tp) match
+          case NoType => tp
+          case TypeBounds(lo, hi) => if lo eq hi then recur(lo, fromBelow) else tp
+          case inst => recur(inst, fromBelow)
+      case tp: TypeVar if false => // TODO: needed?
+        val underlying1 = recur(tp.underlying, fromBelow)
+        if underlying1 ne tp.underlying then underlying1 else tp
+      case _ => tp
+
+    inst match
+      case bounds: TypeBounds =>
+        bounds.derivedTypeBounds(
+          recur(bounds.lo, fromBelow = true),
+          recur(bounds.hi, fromBelow = false))
+      case _ =>
+        inst
+  end ensureNonCyclic
+
   /** Add the fact `param1 <: param2` to the constraint `current` and propagate
    *  `<:<` relationships between parameters ("edges") but not bounds.
    */
@@ -328,115 +352,72 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
       current2
     }
 
-  def addLess(param1: TypeParamRef, param2: TypeParamRef)(implicit ctx: Context): This =
-    order(this, param1, param2)
+  /** The list of parameters P such that, for a fresh type parameter Q:
+   *
+   *    Q <: tp  implies  Q <: P      and isUpper = true, or
+   *    tp <: Q  implies  P <: Q      and isUpper = false
+   */
+  private def dependentParams(tp: Type, isUpper: Boolean): List[TypeParamRef] = tp match
+    case param: TypeParamRef if contains(param) =>
+      param :: (if (isUpper) upper(param) else lower(param))
+    case tp: AndType if isUpper  =>
+      dependentParams(tp.tp1, isUpper) | (dependentParams(tp.tp2, isUpper))
+    case tp: OrType if !isUpper =>
+      dependentParams(tp.tp1, isUpper).intersect(dependentParams(tp.tp2, isUpper))
+    case _ =>
+      Nil
 
-  def updateEntry(current: This, param: TypeParamRef, tp: Type)(implicit ctx: Context): This = {
+  private def updateEntry(current: This, param: TypeParamRef, tp: Type)(implicit ctx: Context): This = {
     var current1 = boundsLens.update(this, current, param, tp)
     tp match {
       case TypeBounds(lo, hi) =>
-        for (p <- dependentParams(lo, isUpper = false))
+        for p <- dependentParams(lo, isUpper = false) do
           current1 = order(current1, p, param)
-        for (p <- dependentParams(hi, isUpper = true))
+        for p <- dependentParams(hi, isUpper = true) do
           current1 = order(current1, param, p)
       case _ =>
     }
     current1
   }
 
+  /** The public version of `updateEntry`. Guarantees that there are no cycles */
   def updateEntry(param: TypeParamRef, tp: Type)(implicit ctx: Context): This =
-    updateEntry(this, param, tp)
+    updateEntry(this, param, ensureNonCyclic(param, tp)).checkNonCyclic()
 
-  def unify(p1: TypeParamRef, p2: TypeParamRef)(implicit ctx: Context): This = {
+  def addLess(param1: TypeParamRef, param2: TypeParamRef)(implicit ctx: Context): This =
+    order(this, param1, param2).checkNonCyclic()
+
+  def unify(p1: TypeParamRef, p2: TypeParamRef)(implicit ctx: Context): This =
     val p1Bounds = (nonParamBounds(p1) & nonParamBounds(p2)).substParam(p2, p1)
     updateEntry(p1, p1Bounds).replace(p2, p1)
-  }
 
-// ---------- Removals ------------------------------------------------------------
+// ---------- Replacements and Removals -------------------------------------
 
   /** A new constraint which is derived from this constraint by removing
    *  the type parameter `param` from the domain and replacing all top-level occurrences
-   *  of the parameter elsewhere in the constraint by type `tp`, or a conservative
-   *  approximation of it if that is needed to avoid cycles.
-   *  Occurrences nested inside a refinement or prefix are not affected.
-   *
-   *  The reason we need to substitute top-level occurrences of the parameter
-   *  is to deal with situations like the following. Say we have in the constraint
-   *
-   *      P <: Q & String
-   *      Q
-   *
-   *  and we replace Q with P. Then substitution gives
-   *
-   *      P <: P & String
-   *
-   *  this would be a cyclic constraint is therefore changed by `normalize` and
-   *  `recombine` below to
-   *
-   *      P <: String
-   *
-   *  approximating the RHS occurrence of P with Any. Without the substitution we
-   *  would not find out where we need to approximate. Occurrences of parameters
-   *  that are not top-level are not affected.
+   *  of the parameter elsewhere in the constraint by type `tp`.
    */
-  def replace(param: TypeParamRef, tp: Type)(implicit ctx: Context): OrderingConstraint = {
+  def replace(param: TypeParamRef, tp: Type)(implicit ctx: Context): OrderingConstraint =
     val replacement = tp.dealiasKeepAnnots.stripTypeVar
-    if (param == replacement) this
-    else {
+    if param == replacement then this
+    else
       assert(replacement.isValueTypeOrLambda)
-      val poly = param.binder
-      val idx = param.paramNum
-
-      def removeParam(ps: List[TypeParamRef]) =
-        ps.filterNot(p => p.binder.eq(poly) && p.paramNum == idx)
-
-      def replaceParam(tp: Type, atPoly: TypeLambda, atIdx: Int): Type = tp match {
-        case bounds @ TypeBounds(lo, hi) =>
-
-          def recombineAnd(and: AndType, op: (Type, Boolean) => Type, isUpper: Boolean): Type = {
-            val tp1 = op(and.tp1, isUpper)
-            val tp2 = op(and.tp2, isUpper)
-            if (tp1.eq(and.tp1) && tp2.eq(and.tp2)) and
-            else tp1 & tp2
-          }
-
-          def recombineOr(or: OrType, op: (Type, Boolean) => Type, isUpper: Boolean): Type = {
-            val tp1 = op(or.tp1, isUpper)
-            val tp2 = op(or.tp2, isUpper)
-            if (tp1.eq(or.tp1) && tp2.eq(or.tp2)) or
-            else tp1 | tp2
-          }
-
-          def normalize(tp: Type, isUpper: Boolean): Type = tp match {
-            case p: TypeParamRef if p.binder == atPoly && p.paramNum == atIdx =>
-              if (isUpper) defn.AnyType else defn.NothingType
-            case tp: AndType if isUpper  => recombineAnd(tp, normalize, isUpper)
-            case tp: OrType  if !isUpper => recombineOr (tp, normalize, isUpper)
-            case _ => tp
-          }
+      var current =
+        if isRemovable(param.binder) then remove(param.binder)
+        else updateEntry(this, param, replacement)
 
-          def replaceIn(tp: Type, isUpper: Boolean): Type = tp match {
-            case `param` => normalize(replacement, isUpper)
-            case tp: AndType if isUpper  => recombineAnd(tp, replaceIn, isUpper)
-            case tp: OrType  if !isUpper => recombineOr (tp, replaceIn, isUpper)
-            case _ => tp.substParam(param, replacement)
-          }
+      def removeParam(ps: List[TypeParamRef]) = ps.filter(param ne _)
 
-          bounds.derivedTypeBounds(replaceIn(lo, isUpper = false), replaceIn(hi, isUpper = true))
-        case _ =>
-          tp.substParam(param, replacement)
-      }
+      def replaceParam(tp: Type, atPoly: TypeLambda, atIdx: Int): Type =
+        current.ensureNonCyclic(atPoly.paramRefs(atIdx), tp.substParam(param, replacement))
 
-      var current =
-        if (isRemovable(poly)) remove(poly) else updateEntry(param, replacement)
-      current.foreachParam {(p, i) =>
+      current.foreachParam { (p, i) =>
         current = boundsLens.map(this, current, p, i, replaceParam(_, p, i))
         current = lowerLens.map(this, current, p, i, removeParam)
         current = upperLens.map(this, current, p, i, removeParam)
       }
-      current
-    }
-  }
+      current.checkNonCyclic()
+  end replace
 
   def remove(pt: TypeLambda)(implicit ctx: Context): This = {
     def removeFromOrdering(po: ParamOrdering) = {
@@ -447,6 +428,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
       po.remove(pt).mapValuesNow(removeFromBoundss)
     }
     newConstraint(boundsMap.remove(pt), removeFromOrdering(lowerMap), removeFromOrdering(upperMap))
+      .checkNonCyclic()
   }
 
   def isRemovable(pt: TypeLambda): Boolean = {
@@ -460,36 +442,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
     allRemovable(paramCount(entries) - 1)
   }
 
-// ---------- Exploration --------------------------------------------------------
-
-  def domainLambdas: List[TypeLambda] = boundsMap.keys
-
-  def domainParams: List[TypeParamRef] =
-    for {
-      (poly, entries) <- boundsMap.toList
-      n <- 0 until paramCount(entries)
-      if entries(n).exists
-    }
-    yield poly.paramRefs(n)
-
-  def forallParams(p: TypeParamRef => Boolean): Boolean =
-    boundsMap.forallBinding { (poly, entries) =>
-      !0.until(paramCount(entries)).exists(i => isBounds(entries(i)) && !p(poly.paramRefs(i)))
-    }
-
-  def foreachParam(p: (TypeLambda, Int) => Unit): Unit =
-    boundsMap.foreachBinding { (poly, entries) =>
-      0.until(poly.paramNames.length).foreach(p(poly, _))
-    }
-
-  def foreachTypeVar(op: TypeVar => Unit): Unit =
-    boundsMap.foreachBinding { (poly, entries) =>
-      for (i <- 0 until paramCount(entries))
-        typeVar(entries, i) match {
-          case tv: TypeVar if !tv.inst.exists => op(tv)
-          case _ =>
-        }
-    }
+// ----------- Joins -----------------------------------------------------
 
   def & (other: Constraint, otherHasErrors: Boolean)(implicit ctx: Context): OrderingConstraint = {
 
@@ -566,7 +519,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
       if (binder eq tl) tvar.setOrigin(tl1.paramRefs(n))
     }
     constr.println(i"renamd $this to $current")
-    current
+    current.checkNonCyclic()
   }
 
   def ensureFresh(tl: TypeLambda)(implicit ctx: Context): TypeLambda =
@@ -582,18 +535,36 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
     }
     else tl
 
-  override def checkClosed()(implicit ctx: Context): Unit = {
-    def isFreeTypeParamRef(tp: Type) = tp match {
-      case TypeParamRef(binder: TypeLambda, _) => !contains(binder)
-      case _ => false
+// ---------- Exploration --------------------------------------------------------
+
+  def domainLambdas: List[TypeLambda] = boundsMap.keys
+
+  def domainParams: List[TypeParamRef] =
+    for {
+      (poly, entries) <- boundsMap.toList
+      n <- 0 until paramCount(entries)
+      if entries(n).exists
+    }
+    yield poly.paramRefs(n)
+
+  def forallParams(p: TypeParamRef => Boolean): Boolean =
+    boundsMap.forallBinding { (poly, entries) =>
+      !0.until(paramCount(entries)).exists(i => isBounds(entries(i)) && !p(poly.paramRefs(i)))
+    }
+
+  def foreachParam(p: (TypeLambda, Int) => Unit): Unit =
+    boundsMap.foreachBinding { (poly, entries) =>
+      0.until(poly.paramNames.length).foreach(p(poly, _))
+    }
+
+  def foreachTypeVar(op: TypeVar => Unit): Unit =
+    boundsMap.foreachBinding { (poly, entries) =>
+      for (i <- 0 until paramCount(entries))
+        typeVar(entries, i) match {
+          case tv: TypeVar if !tv.inst.exists => op(tv)
+          case _ =>
+        }
     }
-    def checkClosedType(tp: Type, where: String) =
-      if (tp != null)
-        assert(!tp.existsPart(isFreeTypeParamRef), i"unclosed constraint: $this refers to $tp in $where")
-    boundsMap.foreachBinding((_, tps) => tps.foreach(checkClosedType(_, "bounds")))
-    lowerMap.foreachBinding((_, paramss) => paramss.foreach(_.foreach(checkClosedType(_, "lower"))))
-    upperMap.foreachBinding((_, paramss) => paramss.foreach(_.foreach(checkClosedType(_, "upper"))))
-  }
 
   private var myUninstVars: mutable.ArrayBuffer[TypeVar] = _
 
@@ -612,13 +583,47 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
     myUninstVars
   }
 
-// ---------- Cyclic checking -------------------------------------------
+// ---------- Checking -----------------------------------------------
 
-  def checkNonCyclic()(implicit ctx: Context): Unit =
-    domainParams.foreach(checkNonCyclic)
+  def checkNonCyclic()(implicit ctx: Context): this.type =
+    if Config.checkConstraintsNonCyclic then domainParams.foreach(checkNonCyclic)
+    this
 
   private def checkNonCyclic(param: TypeParamRef)(implicit ctx: Context): Unit =
-    assert(!isLess(param, param), i"cyclic constraint involving $param in $this")
+    assert(!isLess(param, param), i"cyclic ordering involving $param in $this, upper = ${upper(param)}")
+
+    def recur(tp: Type)(using Context): Unit = tp match
+      case tp: AndOrType =>
+        recur(tp.tp1)
+        recur(tp.tp2)
+      case tp: TypeParamRef =>
+        assert(tp ne param, i"cyclic bound for $param: ${entry(param)} in $this")
+        entry(tp) match
+          case NoType =>
+          case TypeBounds(lo, hi) => if lo eq hi then recur(lo)
+          case inst => recur(inst)
+      case TypeBounds(lo, hi) =>
+        recur(lo)
+        recur(hi)
+      case _ =>
+
+    recur(entry(param))
+  end checkNonCyclic
+
+  override def checkClosed()(using Context): Unit =
+
+    def isFreeTypeParamRef(tp: Type) = tp match
+      case TypeParamRef(binder: TypeLambda, _) => !contains(binder)
+      case _ => false
+
+    def checkClosedType(tp: Type, where: String) =
+      if tp != null then
+        assert(!tp.existsPart(isFreeTypeParamRef), i"unclosed constraint: $this refers to $tp in $where")
+
+    boundsMap.foreachBinding((_, tps) => tps.foreach(checkClosedType(_, "bounds")))
+    lowerMap.foreachBinding((_, paramss) => paramss.foreach(_.foreach(checkClosedType(_, "lower"))))
+    upperMap.foreachBinding((_, paramss) => paramss.foreach(_.foreach(checkClosedType(_, "upper"))))
+  end checkClosed
 
 // ---------- Invalidation -------------------------------------------
 
@@ -631,6 +636,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
 // ---------- toText -----------------------------------------------------
 
   override def toText(printer: Printer): Text = {
+    //Printer.debugPrintUnique = true
     def entryText(tp: Type) = tp match {
       case tp: TypeBounds =>
         tp.toText(printer)
@@ -663,6 +669,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
               Text(ups.map(_.toText(printer)), ", ")
         Text(deps, "\n")
       }
+    //Printer.debugPrintUnique = false
     Text.lines(List(header, uninstVarsText, constrainedText, boundsText, orderingText, ")"))
   }
 

From 5a57ae50d9debec79fde6f0e0db11bb66d7b516d Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Thu, 30 Apr 2020 16:00:59 +0200
Subject: [PATCH 13/22] Simplify adjust

Half of what it does is now handled in constraint itself
---
 .../tools/dotc/core/ConstraintHandling.scala  | 34 +++++--------------
 1 file changed, 8 insertions(+), 26 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
index 94026888e374..69fef0032ec2 100644
--- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
+++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
@@ -90,39 +90,22 @@ trait ConstraintHandling[AbstractContext] {
 
     /** Adjust the bound `tp` in the following ways:
      *
-     *   1. Any toplevel occurrences of the compared parameter `param` are
-     *      replaced by `Nothing` if bound is from below or `Any` otherwise.
-     *   2. Toplevel occurrences of TypeVars over TypeRefs in the current
-     *      constraint are dereferenced.
-     *   3. Toplevel occurrences of TypeRefs that are instantiated in the current
+     *   1. Toplevel occurrences of TypeRefs that are instantiated in the current
      *      constraint are also dereferenced.
-     *   4. Toplevel occurrences of ExprTypes lead to a `NoType` return, which
+     *   2. Toplevel occurrences of ExprTypes lead to a `NoType` return, which
      *      causes the addOneBound operation to fail.
      *
-     *   An occurrence is toplevel if it is the bound itself,
-     *   or the bound is a union or intersection, and the ocurrence is
-     *   toplevel in one of the operands of the `&` or `|`.
+     *   An occurrence is toplevel if it is the bound itself, or a term in some
+     *   combination of `&` or `|` types.
      */
     def adjust(tp: Type): Type = tp match
       case tp: AndOrType =>
         val p1 = adjust(tp.tp1)
         val p2 = adjust(tp.tp2)
         if p1.exists && p2.exists then tp.derivedAndOrType(p1, p2) else NoType
-      case tp: TypeParamRef =>
-        if constraint.contains(tp) then
-          constr.println(i"${if tp eq param then "stripping" else "keeping"} $tp from $rawBound, upper = $isUpper in $constraint")
-        if tp eq param then // (1)
-          if isUpper then defn.AnyType else defn.NothingType
-        else constraint.entry(tp) match  // (3)
-          case NoType => tp
-          case TypeBounds(lo, hi) => if lo eq hi then adjust(lo) else tp
-          case inst => adjust(inst)
-      case tp: TypeVar => // (2)
-        val underlying1 = adjust(tp.underlying)
-        if (underlying1 ne tp.underlying) || constraint.contains(tp.origin)
-        then underlying1
-        else tp
-      case tp: ExprType => // (4)
+      case tp: TypeVar if constraint.contains(tp.origin) =>
+        adjust(tp.underlying)
+      case tp: ExprType =>
         // ExprTypes are not value types, so type parameters should not
         // be instantiated to ExprTypes. A scenario where such an attempted
         // instantiation can happen is if we unify (=> T) => () with A => ()
@@ -468,7 +451,7 @@ trait ConstraintHandling[AbstractContext] {
      *  missing.
      */
     def avoidLambdaParams(tp: Type) =
-      if (comparedTypeLambdas.nonEmpty) {
+      if comparedTypeLambdas.nonEmpty then
         val approx = new ApproximatingTypeMap {
           if (!fromBelow) variance = -1
           def apply(t: Type): Type = t match {
@@ -480,7 +463,6 @@ trait ConstraintHandling[AbstractContext] {
           }
         }
         approx(tp)
-      }
       else tp
 
     def addParamBound(bound: TypeParamRef) =

From 677ae31ef48bcf596e93fb58e7b8f02c2e532916 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Thu, 30 Apr 2020 16:34:10 +0200
Subject: [PATCH 14/22] Include TypeVars in cyclic checking

Without the additional case, scalatest fails.
---
 .../src/dotty/tools/dotc/core/OrderingConstraint.scala    | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
index 13d476564b09..053238e31142 100644
--- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
+++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
@@ -323,7 +323,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
           case NoType => tp
           case TypeBounds(lo, hi) => if lo eq hi then recur(lo, fromBelow) else tp
           case inst => recur(inst, fromBelow)
-      case tp: TypeVar if false => // TODO: needed?
+      case tp: TypeVar =>
         val underlying1 = recur(tp.underlying, fromBelow)
         if underlying1 ne tp.underlying then underlying1 else tp
       case _ => tp
@@ -399,7 +399,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
    */
   def replace(param: TypeParamRef, tp: Type)(implicit ctx: Context): OrderingConstraint =
     val replacement = tp.dealiasKeepAnnots.stripTypeVar
-    if param == replacement then this
+    if param == replacement then this.checkNonCyclic()
     else
       assert(replacement.isValueTypeOrLambda)
       var current =
@@ -518,7 +518,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
       val TypeParamRef(binder, n) = tvar.origin
       if (binder eq tl) tvar.setOrigin(tl1.paramRefs(n))
     }
-    constr.println(i"renamd $this to $current")
+    constr.println(i"renamed $this to $current")
     current.checkNonCyclic()
   }
 
@@ -602,6 +602,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
           case NoType =>
           case TypeBounds(lo, hi) => if lo eq hi then recur(lo)
           case inst => recur(inst)
+      case tp: TypeVar =>
+        recur(tp.underlying)
       case TypeBounds(lo, hi) =>
         recur(lo)
         recur(hi)

From 498578e116942943445af705e454d38fed9b6a0d Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Thu, 30 Apr 2020 16:34:25 +0200
Subject: [PATCH 15/22] Revert "Disable three scalatest tests"

This reverts commit f8765379300ecf5a2626fbdb0445bbac584da69a.
---
 community-build/community-projects/scalatest | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/community-build/community-projects/scalatest b/community-build/community-projects/scalatest
index 4a2fe34ebba0..11be13672aa8 160000
--- a/community-build/community-projects/scalatest
+++ b/community-build/community-projects/scalatest
@@ -1 +1 @@
-Subproject commit 4a2fe34ebba038b7145248632f8523ce53d6c76e
+Subproject commit 11be13672aa8ac4e634cd396524275f3634e544f

From 16dec6bff4cfec1dd4dc8653e450cba00adb56ee Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Thu, 30 Apr 2020 16:39:06 +0200
Subject: [PATCH 16/22] Check for cyclic constraints

---
 compiler/src/dotty/tools/dotc/config/Config.scala | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala
index 6bad889890d4..53e99f084706 100644
--- a/compiler/src/dotty/tools/dotc/config/Config.scala
+++ b/compiler/src/dotty/tools/dotc/config/Config.scala
@@ -13,7 +13,7 @@ object Config {
   /** When updating a constraint bound, check that the constrained parameter
    *  does not appear at the top-level of either of its bounds.
    */
-  final val checkConstraintsNonCyclic = false
+  final val checkConstraintsNonCyclic = true
 
   /** Check that each constraint resulting from a subtype test
    *  is satisfiable.

From 33861855d92e447c08df93e117be0eeca54cd177 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Thu, 30 Apr 2020 18:10:01 +0200
Subject: [PATCH 17/22] Revert "Check for cyclic constraints"

This reverts commit 16dec6bff4cfec1dd4dc8653e450cba00adb56ee.
---
 compiler/src/dotty/tools/dotc/config/Config.scala | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala
index 53e99f084706..6bad889890d4 100644
--- a/compiler/src/dotty/tools/dotc/config/Config.scala
+++ b/compiler/src/dotty/tools/dotc/config/Config.scala
@@ -13,7 +13,7 @@ object Config {
   /** When updating a constraint bound, check that the constrained parameter
    *  does not appear at the top-level of either of its bounds.
    */
-  final val checkConstraintsNonCyclic = true
+  final val checkConstraintsNonCyclic = false
 
   /** Check that each constraint resulting from a subtype test
    *  is satisfiable.

From fa8e36b0927217f31db2aa3c12eab35d36764322 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Thu, 30 Apr 2020 23:47:13 +0200
Subject: [PATCH 18/22] Change top type to AnyKind

---
 compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
index 053238e31142..b933498dbf36 100644
--- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
+++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
@@ -263,7 +263,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
   private def normalizedType(tp: Type, paramBuf: mutable.ListBuffer[TypeParamRef],
       isUpper: Boolean)(implicit ctx: Context): Type =
     stripParams(tp, paramBuf, isUpper)
-      .orElse(if (isUpper) defn.AnyType else defn.NothingType)
+      .orElse(if (isUpper) defn.AnyKindType else defn.NothingType)
 
   def add(poly: TypeLambda, tvars: List[TypeVar])(implicit ctx: Context): This = {
     assert(!contains(poly))

From f4cbf5d87f5b46e7b3bf715b5c1971cc5a0402cd Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Fri, 1 May 2020 08:41:25 +0200
Subject: [PATCH 19/22] Make cycle checking complete

Also detect cycles containing aliases and annotated types.
---
 .../dotty/tools/dotc/core/OrderingConstraint.scala   | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
index b933498dbf36..2b286f05338a 100644
--- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
+++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
@@ -326,7 +326,15 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
       case tp: TypeVar =>
         val underlying1 = recur(tp.underlying, fromBelow)
         if underlying1 ne tp.underlying then underlying1 else tp
-      case _ => tp
+      case tp: AnnotatedType =>
+        val parent1 = recur(tp.parent, fromBelow)
+        if parent1 ne tp.parent then tp.derivedAnnotatedType(parent1, tp.annot) else tp
+      case _ =>
+        val tp1 = tp.dealiasKeepAnnots
+        if tp1 ne tp then
+          val tp2 = recur(tp1, fromBelow)
+          if tp2 ne tp1 then tp2 else tp
+        else tp
 
     inst match
       case bounds: TypeBounds =>
@@ -608,6 +616,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
         recur(lo)
         recur(hi)
       case _ =>
+        val tp1 = tp.dealias
+        if tp1 ne tp then recur(tp1)
 
     recur(entry(param))
   end checkNonCyclic

From f4de03f51a8f08cbf23d95808398316489872815 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Thu, 30 Apr 2020 16:39:06 +0200
Subject: [PATCH 20/22] Check for cyclic constraints

---
 compiler/src/dotty/tools/dotc/config/Config.scala | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala
index 6bad889890d4..53e99f084706 100644
--- a/compiler/src/dotty/tools/dotc/config/Config.scala
+++ b/compiler/src/dotty/tools/dotc/config/Config.scala
@@ -13,7 +13,7 @@ object Config {
   /** When updating a constraint bound, check that the constrained parameter
    *  does not appear at the top-level of either of its bounds.
    */
-  final val checkConstraintsNonCyclic = false
+  final val checkConstraintsNonCyclic = true
 
   /** Check that each constraint resulting from a subtype test
    *  is satisfiable.

From 0e4adb80764eb339fdefbc5ae1ef27f2ccc1ca76 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Fri, 1 May 2020 09:17:12 +0200
Subject: [PATCH 21/22] Simplify constraint handling

---
 .../tools/dotc/core/ConstraintHandling.scala  | 130 ++++++++----------
 .../tools/dotc/core/GadtConstraint.scala      |   5 +-
 2 files changed, 59 insertions(+), 76 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
index 69fef0032ec2..1cb30f2f71eb 100644
--- a/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
+++ b/compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
@@ -8,7 +8,7 @@ import Symbols._
 import Decorators._
 import Flags._
 import config.Config
-import config.Printers.{constr, typr}
+import config.Printers.typr
 import dotty.tools.dotc.reporting.trace
 
 /** Methods for adding constraints and solving them.
@@ -24,8 +24,7 @@ import dotty.tools.dotc.reporting.trace
  */
 trait ConstraintHandling[AbstractContext] {
 
-  def constr_println(msg: => String): Unit = constr.println(msg)
-  def typr_println(msg: => String): Unit = typr.println(msg)
+  def constr: config.Printers.Printer = config.Printers.constr
 
   implicit def ctx(implicit ac: AbstractContext): Context
 
@@ -86,7 +85,41 @@ trait ConstraintHandling[AbstractContext] {
   def fullBounds(param: TypeParamRef)(implicit actx: AbstractContext): TypeBounds =
     nonParamBounds(param).derivedTypeBounds(fullLowerBound(param), fullUpperBound(param))
 
-  protected def addOneBound(param: TypeParamRef, rawBound: Type, isUpper: Boolean)(implicit actx: AbstractContext): Boolean =
+  protected def addOneBound(param: TypeParamRef, bound: Type, isUpper: Boolean)(using AbstractContext): Boolean =
+    if !constraint.contains(param) then true
+    else
+      val oldBounds @ TypeBounds(lo, hi) = constraint.nonParamBounds(param)
+      val equalBounds = (if isUpper then lo else hi) eq bound
+      if equalBounds
+        && !bound.existsPart(bp => bp.isInstanceOf[WildcardType] || (bp eq param))
+      then
+        // The narrowed bounds are equal and do not contain wildcards,
+        // so we can remove `param` from the constraint.
+        // (Handling wildcards requires choosing a bound, but we don't know which
+        // bound to choose here, this is handled in `ConstraintHandling#approximation`)
+        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
+            if isUpper then oldBounds.derivedTypeBounds(lo, hi & bound)
+            else oldBounds.derivedTypeBounds(lo | bound, hi)
+          finally homogenizeArgs = saved
+        val c1 = constraint.updateEntry(param, narrowedBounds)
+        (c1 eq constraint)
+        || {
+          constraint = c1
+          val TypeBounds(lo, hi) = constraint.entry(param)
+          isSubType(lo, hi)
+        }
+  end addOneBound
+
+  protected def addBoundTransitively(param: TypeParamRef, rawBound: Type, isUpper: Boolean)(implicit actx: AbstractContext): Boolean =
 
     /** Adjust the bound `tp` in the following ways:
      *
@@ -119,69 +152,19 @@ trait ConstraintHandling[AbstractContext] {
       case _ =>
         tp
 
-    if !constraint.contains(param) then true
-    else
-      val bound = adjust(rawBound)
-
-      val oldBounds @ TypeBounds(lo, hi) = constraint.nonParamBounds(param)
-      val equalBounds = isUpper && (lo eq bound) || !isUpper && (bound eq hi)
-      if !bound.exists then false
-      else if equalBounds
-        && !bound.existsPart(bp => bp.isInstanceOf[WildcardType] || (bp eq param))
-      then
-        // The narrowed bounds are equal and do not contain wildcards,
-        // so we can remove `param` from the constraint.
-        // (Handling wildcards requires choosing a bound, but we don't know which
-        // bound to choose here, this is handled in `ConstraintHandling#approximation`)
-        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
-            if (isUpper) oldBounds.derivedTypeBounds(lo, hi & bound)
-            else oldBounds.derivedTypeBounds(lo | bound, hi)
-          finally homogenizeArgs = saved
-        val c1 = constraint.updateEntry(param, narrowedBounds)
-        (c1 eq constraint) || {
-          constraint = c1
-          val TypeBounds(lo, hi) = constraint.entry(param)
-          isSubType(lo, hi)
-        }
-  end addOneBound
-
-  private def location(implicit ctx: Context) = "" // i"in ${ctx.typerState.stateChainStr}" // use for debugging
-
-  protected def addUpperBound(param: TypeParamRef, bound: Type)(implicit actx: AbstractContext): Boolean = {
-    def description = i"constraint $param <: $bound to\n$constraint"
-    if (bound.isRef(defn.NothingClass) && ctx.typerState.isGlobalCommittable) {
-      def msg = s"!!! instantiated to Nothing: $param, constraint = ${constraint.show}"
-      if (Config.failOnInstantiationToNothing) assert(false, msg)
+    def description = i"constraint $param ${if isUpper then "<:" else ":>"} $rawBound to\n$constraint"
+    constr.println(i"adding $description$location")
+    if isUpper && rawBound.isRef(defn.NothingClass) && ctx.typerState.isGlobalCommittable then
+      def msg = i"!!! instantiated to Nothing: $param, constraint = $constraint"
+      if Config.failOnInstantiationToNothing
+      then assert(false, msg)
       else ctx.log(msg)
-    }
-    constr_println(i"adding $description$location")
-    val lower = constraint.lower(param)
-    val res =
-      addOneBound(param, bound, isUpper = true) &&
-      lower.forall(addOneBound(_, bound, isUpper = true))
-    constr_println(i"added $description = $res$location")
-    res
-  }
-
-  protected def addLowerBound(param: TypeParamRef, bound: Type)(implicit actx: AbstractContext): Boolean = {
-    def description = i"constraint $param >: $bound to\n$constraint"
-    constr_println(i"adding $description")
-    val upper = constraint.upper(param)
-    val res =
-      addOneBound(param, bound, isUpper = false) &&
-      upper.forall(addOneBound(_, bound, isUpper = false))
-    constr_println(i"added $description = $res$location")
-    res
-  }
+    def others = if isUpper then constraint.lower(param) else constraint.upper(param)
+    val bound = adjust(rawBound)
+    bound.exists
+    && addOneBound(param, bound, isUpper) && others.forall(addOneBound(_, bound, isUpper))
+        .reporting(i"added $description = $result$location", constr)
+  end addBoundTransitively
 
   protected def addLess(p1: TypeParamRef, p2: TypeParamRef)(implicit actx: AbstractContext): Boolean = {
     def description = i"ordering $p1 <: $p2 to\n$constraint"
@@ -192,20 +175,22 @@ trait ConstraintHandling[AbstractContext] {
         val up2 = p2 :: constraint.exclusiveUpper(p2, p1)
         val lo1 = constraint.nonParamBounds(p1).lo
         val hi2 = constraint.nonParamBounds(p2).hi
-        constr_println(i"adding $description down1 = $down1, up2 = $up2$location")
+        constr.println(i"adding $description down1 = $down1, up2 = $up2$location")
         constraint = constraint.addLess(p1, p2)
         down1.forall(addOneBound(_, hi2, isUpper = true)) &&
         up2.forall(addOneBound(_, lo1, isUpper = false))
       }
-    constr_println(i"added $description = $res$location")
+    constr.println(i"added $description = $res$location")
     res
   }
 
+  def location(implicit ctx: Context) = "" // i"in ${ctx.typerState.stateChainStr}" // use for debugging
+
   /** Make p2 = p1, transfer all bounds of p2 to p1
    *  @pre  less(p1)(p2)
    */
   private def unify(p1: TypeParamRef, p2: TypeParamRef)(implicit actx: AbstractContext): Boolean = {
-    constr_println(s"unifying $p1 $p2")
+    constr.println(s"unifying $p1 $p2")
     assert(constraint.isLess(p1, p2))
     val down = constraint.exclusiveLower(p2, p1)
     val up = constraint.exclusiveUpper(p1, p2)
@@ -301,7 +286,7 @@ trait ConstraintHandling[AbstractContext] {
       case _: TypeBounds =>
         val bound = if (fromBelow) fullLowerBound(param) else fullUpperBound(param)
         val inst = avoidParam(bound)
-        typr_println(s"approx ${param.show}, from below = $fromBelow, bound = ${bound.show}, inst = ${inst.show}")
+        typr.println(s"approx ${param.show}, from below = $fromBelow, bound = ${bound.show}, inst = ${inst.show}")
         inst
       case inst =>
         assert(inst.exists, i"param = $param\nconstraint = $constraint")
@@ -407,7 +392,7 @@ trait ConstraintHandling[AbstractContext] {
             val upper = constraint.upper(param)
             if lower.nonEmpty && !bounds.lo.isRef(defn.NothingClass)
                || upper.nonEmpty && !bounds.hi.isAny
-            then constr_println(i"INIT*** $tl")
+            then constr.println(i"INIT*** $tl")
             lower.forall(addOneBound(_, bounds.hi, isUpper = true)) &&
               upper.forall(addOneBound(_, bounds.lo, isUpper = false))
           case _ =>
@@ -491,8 +476,7 @@ trait ConstraintHandling[AbstractContext] {
           addParamBound(bound)
         case _ =>
           val pbound = avoidLambdaParams(bound)
-          kindCompatible(param, pbound)
-          && (if fromBelow then addLowerBound(param, pbound) else addUpperBound(param, pbound))
+          kindCompatible(param, pbound) && addBoundTransitively(param, pbound, !fromBelow)
       finally addConstraintInvocations -= 1
     }
   end addConstraint
diff --git a/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala b/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala
index 2faf721a9da8..929750b99ff8 100644
--- a/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala
+++ b/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala
@@ -155,8 +155,7 @@ final class ProperGadtConstraint private(
           else if (isUpper) addLess(symTvar.origin, boundTvar.origin)
           else addLess(boundTvar.origin, symTvar.origin)
         case bound =>
-          if (isUpper) addUpperBound(symTvar.origin, bound)
-          else addLowerBound(symTvar.origin, bound)
+          addBoundTransitively(symTvar.origin, bound, isUpper)
       }
     ).reporting({
       val descr = if (isUpper) "upper" else "lower"
@@ -271,7 +270,7 @@ final class ProperGadtConstraint private(
 
   // ---- Debug ------------------------------------------------------------
 
-  override def constr_println(msg: => String): Unit = gadtsConstr.println(msg)
+  override def constr = gadtsConstr
 
   override def toText(printer: Printer): Texts.Text = constraint.toText(printer)
 

From 031c69a335ba5a6a12ad4360ea49eb98f21eabb6 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Thu, 30 Apr 2020 18:10:01 +0200
Subject: [PATCH 22/22] Revert "Check for cyclic constraints"

This reverts commit 16dec6bff4cfec1dd4dc8653e450cba00adb56ee.
---
 compiler/src/dotty/tools/dotc/config/Config.scala | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala
index 53e99f084706..6bad889890d4 100644
--- a/compiler/src/dotty/tools/dotc/config/Config.scala
+++ b/compiler/src/dotty/tools/dotc/config/Config.scala
@@ -13,7 +13,7 @@ object Config {
   /** When updating a constraint bound, check that the constrained parameter
    *  does not appear at the top-level of either of its bounds.
    */
-  final val checkConstraintsNonCyclic = true
+  final val checkConstraintsNonCyclic = false
 
   /** Check that each constraint resulting from a subtype test
    *  is satisfiable.