Skip to content

Commit 3af49b9

Browse files
committed
Remove GadtConstraint#remove
Pattern bound symbols infer their bounds which then get installed as their info (in indexPattern). So I want to remove the duplication of info, in the persisted pickle. However, we have a mode in TypeComparer where we don't trust type bounds, to pessimistically protect against bad bounds. So removing the info from the GADT bounds made that code broke, which is why I had looked to handling that in TypeComparer. However, in the body of the match the pattern bound symbols can gain more GADT bounds, so perhaps using `isPatternBound` is too naive? And, in general, it's a change that would need to be studied for its soundness impact. So, let's leave the algorithm as it is and just keep the potential duplication. At least for right now.
1 parent efd8b68 commit 3af49b9

File tree

3 files changed

+5
-30
lines changed

3 files changed

+5
-30
lines changed

compiler/src/dotty/tools/dotc/core/GadtConstraint.scala

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,6 @@ sealed abstract class GadtConstraint extends Showable {
5050
/** See [[ConstraintHandling.approximation]] */
5151
def approximation(sym: Symbol, fromBelow: Boolean, maxLevel: Int = Int.MaxValue)(using Context): Type
5252

53-
def remove(sym: Symbol)(using Context): Unit
54-
5553
def symbols: List[Symbol]
5654
def inputs: List[(List[Symbol], Int)]
5755

@@ -223,14 +221,6 @@ final class ProperGadtConstraint private(
223221
res
224222
}
225223

226-
override def remove(sym: Symbol)(using Context): Unit =
227-
mapping(sym) match
228-
case tv: TypeVar =>
229-
mapping = mapping.remove(sym)
230-
reverseMapping = reverseMapping.remove(tv.origin)
231-
constraint = constraint.replace(tv.origin, sym.typeRef)
232-
case null =>
233-
234224
override def symbols: List[Symbol] = mapping.keys
235225

236226
override def inputs: List[(List[Symbol], Int)] =
@@ -335,8 +325,6 @@ final class ProperGadtConstraint private(
335325

336326
override def approximation(sym: Symbol, fromBelow: Boolean, maxLevel: Int)(using Context): Type = unsupported("EmptyGadtConstraint.approximation")
337327

338-
override def remove(sym: Symbol)(using Context): Unit = ()
339-
340328
override def symbols: List[Symbol] = Nil
341329
override def inputs: List[(List[Symbol], Int)] = Nil
342330

compiler/src/dotty/tools/dotc/core/TypeComparer.scala

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -837,14 +837,9 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
837837
isSubTypeWhenFrozen(gbounds1.hi, tp2)
838838
|| narrowGADTBounds(tp1, tp2, approx, isUpper = true))
839839

840-
def comparePatBoundInfo =
841-
tp1.symbol.isPatternBound
842-
&& isSubTypeWhenFrozen(hi1, tp2)
843-
844840
(!caseLambda.exists || canWidenAbstract)
845841
&& isSubType(hi1.boxedIfTypeParam(tp1.symbol), tp2, approx.addLow) && (trustBounds || isSubType(lo1, tp2, approx.addLow))
846842
|| compareGADT
847-
|| comparePatBoundInfo
848843
|| tryLiftedToThis1
849844
case _ =>
850845
// `Mode.RelaxedOverriding` is only enabled when checking Java overriding

compiler/src/dotty/tools/dotc/typer/Typer.scala

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1693,13 +1693,6 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
16931693
}
16941694
}
16951695

1696-
private object patBindingsAcc extends TreeAccumulator[List[Symbol]] {
1697-
def run(tree: Tree)(using Context) = apply(Nil, tree).reverse
1698-
override def apply(x: List[Symbol], tree: Tree)(using Context) = tree match
1699-
case b: Bind if ctx.gadt.contains(b.symbol) => b.symbol :: x
1700-
case _ => super.foldOver(x, tree)
1701-
}
1702-
17031696
/** If the prototype `pt` is the type lambda (when doing a dependent
17041697
* typing of a match), instantiate that type lambda with the pattern
17051698
* variables found in the pattern `pat`.
@@ -1715,10 +1708,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
17151708
/** Type a case. */
17161709
def typedCase(tree: untpd.CaseDef, sel: Tree, wideSelType: Type, pt: Type)(using Context): CaseDef = {
17171710
val originalCtx = ctx
1718-
val gadt = tree.body match
1711+
val previousGadt = tree.body match
17191712
case GadtExpr(gadt, _) => gadt
1720-
case _ => ctx.gadt
1721-
val gadtCtx: Context = ctx.fresh.setGadt(gadt.fresh).setNewScope
1713+
case _ => originalCtx.gadt
1714+
val gadtCtx: Context = ctx.fresh.setGadt(previousGadt.fresh).setNewScope
17221715

17231716
def caseRest(pat: Tree)(using Context) = {
17241717
val pt1 = instantiateMatchTypeProto(pat, pt) match {
@@ -1728,10 +1721,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
17281721
val pat1 = indexPattern(tree).transform(pat)
17291722
val guard1 = typedExpr(tree.guard, defn.BooleanType)
17301723
var body1 = ensureNoLocalRefs(typedExpr(tree.body, pt1), pt1, ctx.scope.toList)
1731-
for sym <- patBindingsAcc.run(pat1) do ctx.gadt.remove(sym)
17321724
if (pt1.isValueType) // insert a cast if body does not conform to expected type if we disregard gadt bounds
17331725
body1 = body1.ensureConforms(pt1)(using originalCtx)
1734-
if ctx.gadt.isNarrowing && !gadt.eql(ctx.gadt) then
1726+
if (previousGadt ne originalCtx.gadt) || !ctx.gadt.eql(previousGadt) then
17351727
body1 = GadtExpr(ctx.gadt, body1)
17361728
assignType(cpy.CaseDef(tree)(pat1, guard1, body1), pat1, body1)
17371729
}
@@ -2950,7 +2942,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
29502942
|| tree.isDef // ... unless tree is a definition
29512943
then
29522944
interpolateTypeVars(tree, pt, locked)
2953-
tree.overwriteType(tree.tpe.simplified)
2945+
tree.overwriteType(tree.tpe.simplified(using ctx.withGadt(EmptyGadtConstraint)))
29542946
tree
29552947

29562948
protected def makeContextualFunction(tree: untpd.Tree, pt: Type)(using Context): Tree = {

0 commit comments

Comments
 (0)