From 072d38e2359fe0960ca6ef89cb1bdc8c43f73f23 Mon Sep 17 00:00:00 2001
From: Dale Wijnand <dale.wijnand@gmail.com>
Date: Mon, 17 Jul 2023 08:39:43 +0100
Subject: [PATCH] Freeze constraints while calculating GADT full bounds

While studying a bug, I discovered that the code I'd written to avoid
bidirectionalily in GADT fullBounds was inadvertently recording GADT
constraints, assisted by the fact that, when running without
-Yno-deep-subtypes, TypeComparer's escape hatch for deep recursions
avoids it crashing.  For instance a GADT symbol was compared >: Nothing
and thus recorded as having Nothing as a lower bound.

So I added freezing, and while I was at it I updated the docs I hadn't
changed last time, and I made the subtyping check in the only direction
necessary.
---
 .../tools/dotc/core/GadtConstraint.scala      | 27 ++++++++++---------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala b/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala
index bb65cce84042..ab0611b89b22 100644
--- a/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala
+++ b/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala
@@ -73,28 +73,31 @@ class GadtConstraint private (
 
   def fullLowerBound(param: TypeParamRef)(using Context): Type =
     val self = externalize(param)
-    constraint.minLower(param).foldLeft(nonParamBounds(param).lo) { (acc, p) =>
-      externalize(p) match
+    constraint.minLower(param).foldLeft(nonParamBounds(param).lo) { (acc, loParam) =>
+      externalize(loParam) match
         // drop any lower param that is a GADT symbol
-        // and is upper-bounded by a non-Any super-type of the original parameter
-        // e.g. in pos/i14287.min
+        // and is upper-bounded by the original parameter
+        // e.g. in pos/i14287.min:
+        //   case Foo.Bar[B$1](Foo.Bar[B$2](x)) => Foo.Bar(x)
+        // after pattern type constraining:
         // B$1 had info <: X   and fullBounds >: B$2 <: X, and
         // B$2 had info <: B$1 and fullBounds <: B$1
-        // We can use the info of B$2 to drop the lower-bound of B$1
+        // If we keep these fullBounds, it would be a bidirectional definition.
+        // So instead we can use the info of B$2 to drop the lower-bound of B$1
         // and return non-bidirectional bounds B$1 <: X and B$2 <: B$1.
-        case tp: TypeRef if tp.symbol.isPatternBound && self =:= tp.info.hiBound => acc
-        case tp => acc | tp
+        case lo: TypeRef if lo.symbol.isPatternBound && lo.info.hiBound.frozen_<:<(self) => acc
+        case lo => acc | lo
     }
 
   def fullUpperBound(param: TypeParamRef)(using Context): Type =
     val self = externalize(param)
-    constraint.minUpper(param).foldLeft(nonParamBounds(param).hi) { (acc, u) =>
-      externalize(u) match
-        case tp: TypeRef if tp.symbol.isPatternBound && self =:= tp.info.loBound => acc // like fullLowerBound
-        case tp =>
+    constraint.minUpper(param).foldLeft(nonParamBounds(param).hi) { (acc, hiParam) =>
+      externalize(hiParam) match
+        case hi: TypeRef if hi.symbol.isPatternBound && self.frozen_<:<(hi.info.loBound) => acc // like fullLowerBound
+        case hi =>
           // Any as the upper bound means "no bound", but if F is higher-kinded,
           // Any & F = F[_]; this is wrong for us so we need to short-circuit
-          if acc.isAny then tp else acc & tp
+          if acc.isAny then hi else acc & hi
     }
 
   def externalize(tp: Type, theMap: TypeMap | Null = null)(using Context): Type = tp match