From c72e06224980915ef1953d3932e8aca20d1675cc Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Sat, 15 Oct 2022 11:29:20 +0100 Subject: [PATCH 1/3] Eliminate class hierarchy in GadtConstraint --- .../src/dotty/tools/dotc/core/Contexts.scala | 2 +- .../tools/dotc/core/GadtConstraint.scala | 144 ++++++------------ .../dotty/tools/dotc/core/TypeComparer.scala | 6 +- .../tools/dotc/printing/PlainPrinter.scala | 12 +- .../src/dotty/tools/dotc/typer/Typer.scala | 2 +- 5 files changed, 51 insertions(+), 115 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 919598c41d6e..d2a88a422b2e 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -814,7 +814,7 @@ object Contexts { .updated(notNullInfosLoc, Nil) .updated(compilationUnitLoc, NoCompilationUnit) searchHistory = new SearchRoot - gadt = EmptyGadtConstraint + gadt = GadtConstraint.empty } @sharable object NoContext extends Context((null: ContextBase | Null).uncheckedNN) { diff --git a/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala b/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala index f3a5e740e07c..c961baa01b97 100644 --- a/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala @@ -12,60 +12,17 @@ import printing._ import scala.annotation.internal.sharable -/** Represents GADT constraints currently in scope */ -sealed abstract class GadtConstraint extends Showable { - /** Immediate bounds of `sym`. Does not contain lower/upper symbols (see [[fullBounds]]). */ - def bounds(sym: Symbol)(using Context): TypeBounds | Null - - /** Full bounds of `sym`, including TypeRefs to other lower/upper symbols. - * - * @note this performs subtype checks between ordered symbols. - * Using this in isSubType can lead to infinite recursion. Consider `bounds` instead. - */ - def fullBounds(sym: Symbol)(using Context): TypeBounds | Null - - /** Is `sym1` ordered to be less than `sym2`? */ - def isLess(sym1: Symbol, sym2: Symbol)(using Context): Boolean - - /** Add symbols to constraint, correctly handling inter-dependencies. - * - * @see [[ConstraintHandling.addToConstraint]] - */ - def addToConstraint(syms: List[Symbol])(using Context): Boolean - def addToConstraint(sym: Symbol)(using Context): Boolean = addToConstraint(sym :: Nil) - - /** Further constrain a symbol already present in the constraint. */ - def addBound(sym: Symbol, bound: Type, isUpper: Boolean)(using Context): Boolean +object GadtConstraint: + @sharable val empty = + new GadtConstraint(OrderingConstraint.empty, SimpleIdentityMap.empty, SimpleIdentityMap.empty, false) - /** Is the symbol registered in the constraint? - * - * @note this is true even if the symbol is constrained to be equal to another type, unlike [[Constraint.contains]]. - */ - def contains(sym: Symbol)(using Context): Boolean - - /** GADT constraint narrows bounds of at least one variable */ - def isNarrowing: Boolean - - /** See [[ConstraintHandling.approximation]] */ - def approximation(sym: Symbol, fromBelow: Boolean, maxLevel: Int = Int.MaxValue)(using Context): Type - - def symbols: List[Symbol] - - def fresh: GadtConstraint - - /** Restore the state from other [[GadtConstraint]], probably copied using [[fresh]] */ - def restore(other: GadtConstraint): Unit - - /** Provides more information than toText, by showing the underlying Constraint details. */ - def debugBoundsDescription(using Context): String -} - -final class ProperGadtConstraint private( +/** Represents GADT constraints currently in scope */ +final class GadtConstraint private( private var myConstraint: Constraint, private var mapping: SimpleIdentityMap[Symbol, TypeVar], private var reverseMapping: SimpleIdentityMap[TypeParamRef, Symbol], private var wasConstrained: Boolean -) extends GadtConstraint with ConstraintHandling { +) extends ConstraintHandling with Showable { import dotty.tools.dotc.config.Printers.{gadts, gadtsConstr} def this() = this( @@ -77,10 +34,7 @@ final class ProperGadtConstraint private( /** Exposes ConstraintHandling.subsumes */ def subsumes(left: GadtConstraint, right: GadtConstraint, pre: GadtConstraint)(using Context): Boolean = { - def extractConstraint(g: GadtConstraint) = g match { - case s: ProperGadtConstraint => s.constraint - case EmptyGadtConstraint => OrderingConstraint.empty - } + def extractConstraint(g: GadtConstraint) = g.constraint subsumes(extractConstraint(left), extractConstraint(right), extractConstraint(pre)) } @@ -89,7 +43,12 @@ final class ProperGadtConstraint private( // the case where they're valid, so no approximating is needed. rawBound - override def addToConstraint(params: List[Symbol])(using Context): Boolean = { + /** Add symbols to constraint, correctly handling inter-dependencies. + * + * @see [[ConstraintHandling.addToConstraint]] + */ + def addToConstraint(sym: Symbol)(using Context): Boolean = addToConstraint(sym :: Nil) + def addToConstraint(params: List[Symbol])(using Context): Boolean = { import NameKinds.DepParamName val poly1 = PolyType(params.map { sym => DepParamName.fresh(sym.name.toTypeName) })( @@ -138,7 +97,8 @@ final class ProperGadtConstraint private( .showing(i"added to constraint: [$poly1] $params%, % gadt = $this", gadts) } - override def addBound(sym: Symbol, bound: Type, isUpper: Boolean)(using Context): Boolean = { + /** Further constrain a symbol already present in the constraint. */ + def addBound(sym: Symbol, bound: Type, isUpper: Boolean)(using Context): Boolean = { @annotation.tailrec def stripInternalTypeVar(tp: Type): Type = tp match { case tv: TypeVar => val inst = constraint.instType(tv) @@ -179,10 +139,16 @@ final class ProperGadtConstraint private( result } - override def isLess(sym1: Symbol, sym2: Symbol)(using Context): Boolean = + /** Is `sym1` ordered to be less than `sym2`? */ + def isLess(sym1: Symbol, sym2: Symbol)(using Context): Boolean = constraint.isLess(tvarOrError(sym1).origin, tvarOrError(sym2).origin) - override def fullBounds(sym: Symbol)(using Context): TypeBounds | Null = + /** Full bounds of `sym`, including TypeRefs to other lower/upper symbols. + * + * @note this performs subtype checks between ordered symbols. + * Using this in isSubType can lead to infinite recursion. Consider `bounds` instead. + */ + def fullBounds(sym: Symbol)(using Context): TypeBounds | Null = mapping(sym) match { case null => null // TODO: Improve flow typing so that ascription becomes redundant @@ -191,7 +157,8 @@ final class ProperGadtConstraint private( // .ensuring(containsNoInternalTypes(_)) } - override def bounds(sym: Symbol)(using Context): TypeBounds | Null = + /** Immediate bounds of `sym`. Does not contain lower/upper symbols (see [[fullBounds]]). */ + def bounds(sym: Symbol)(using Context): TypeBounds | Null = mapping(sym) match { case null => null // TODO: Improve flow typing so that ascription becomes redundant @@ -202,11 +169,17 @@ final class ProperGadtConstraint private( //.ensuring(containsNoInternalTypes(_)) } - override def contains(sym: Symbol)(using Context): Boolean = mapping(sym) != null + /** Is the symbol registered in the constraint? + * + * @note this is true even if the symbol is constrained to be equal to another type, unlike [[Constraint.contains]]. + */ + def contains(sym: Symbol)(using Context): Boolean = mapping(sym) != null + /** GADT constraint narrows bounds of at least one variable */ def isNarrowing: Boolean = wasConstrained - override def approximation(sym: Symbol, fromBelow: Boolean, maxLevel: Int)(using Context): Type = { + /** See [[ConstraintHandling.approximation]] */ + def approximation(sym: Symbol, fromBelow: Boolean, maxLevel: Int = Int.MaxValue)(using Context): Type = { val res = approximation(tvarOrError(sym).origin, fromBelow, maxLevel) match case tpr: TypeParamRef => @@ -220,23 +193,16 @@ final class ProperGadtConstraint private( res } - override def symbols: List[Symbol] = mapping.keys + def symbols: List[Symbol] = mapping.keys - override def fresh: GadtConstraint = new ProperGadtConstraint( - myConstraint, - mapping, - reverseMapping, - wasConstrained - ) + def fresh: GadtConstraint = new GadtConstraint(myConstraint, mapping, reverseMapping, wasConstrained) - def restore(other: GadtConstraint): Unit = other match { - case other: ProperGadtConstraint => - this.myConstraint = other.myConstraint - this.mapping = other.mapping - this.reverseMapping = other.reverseMapping - this.wasConstrained = other.wasConstrained - case _ => ; - } + /** Restore the state from other [[GadtConstraint]], probably copied using [[fresh]] */ + def restore(other: GadtConstraint): Unit = + this.myConstraint = other.myConstraint + this.mapping = other.mapping + this.reverseMapping = other.reverseMapping + this.wasConstrained = other.wasConstrained // ---- Protected/internal ----------------------------------------------- @@ -294,30 +260,6 @@ final class ProperGadtConstraint private( override def toText(printer: Printer): Texts.Text = printer.toText(this) - override def debugBoundsDescription(using Context): String = i"$this\n$constraint" -} - -@sharable object EmptyGadtConstraint extends GadtConstraint { - override def bounds(sym: Symbol)(using Context): TypeBounds | Null = null - override def fullBounds(sym: Symbol)(using Context): TypeBounds | Null = null - - override def isLess(sym1: Symbol, sym2: Symbol)(using Context): Boolean = unsupported("EmptyGadtConstraint.isLess") - - override def isNarrowing: Boolean = false - - override def contains(sym: Symbol)(using Context) = false - - override def addToConstraint(params: List[Symbol])(using Context): Boolean = unsupported("EmptyGadtConstraint.addToConstraint") - override def addBound(sym: Symbol, bound: Type, isUpper: Boolean)(using Context): Boolean = unsupported("EmptyGadtConstraint.addBound") - - override def approximation(sym: Symbol, fromBelow: Boolean, maxLevel: Int)(using Context): Type = unsupported("EmptyGadtConstraint.approximation") - - override def symbols: List[Symbol] = Nil - - override def fresh = new ProperGadtConstraint - override def restore(other: GadtConstraint): Unit = - assert(!other.isNarrowing, "cannot restore a non-empty GADTMap") - - override def toText(printer: Printer): Texts.Text = printer.toText(this) - override def debugBoundsDescription(using Context): String = i"$this" + /** Provides more information than toText, by showing the underlying Constraint details. */ + def debugBoundsDescription(using Context): String = i"$this\n$constraint" } diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 9365bafd8282..126e169d8ff1 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -1830,11 +1830,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling val preGadt = ctx.gadt.fresh def allSubsumes(leftGadt: GadtConstraint, rightGadt: GadtConstraint, left: Constraint, right: Constraint): Boolean = - subsumes(left, right, preConstraint) && preGadt.match - case preGadt: ProperGadtConstraint => - preGadt.subsumes(leftGadt, rightGadt, preGadt) - case _ => - true + subsumes(left, right, preConstraint) && preGadt.subsumes(leftGadt, rightGadt, preGadt) if op1 then val op1Constraint = constraint diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index b428a117d6a6..b98b66087689 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -693,13 +693,11 @@ class PlainPrinter(_ctx: Context) extends Printer { finally ctx.typerState.constraint = savedConstraint - def toText(g: GadtConstraint): Text = g match - case EmptyGadtConstraint => "EmptyGadtConstraint" - case g: ProperGadtConstraint => - val deps = for sym <- g.symbols yield - val bound = g.fullBounds(sym).nn - (typeText(toText(sym.typeRef)) ~ toText(bound)).close - ("GadtConstraint(" ~ Text(deps, ", ") ~ ")").close + def toText(g: GadtConstraint): Text = + val deps = for sym <- g.symbols yield + val bound = g.fullBounds(sym).nn + (typeText(toText(sym.typeRef)) ~ toText(bound)).close + ("GadtConstraint(" ~ Text(deps, ", ") ~ ")").close def plain: PlainPrinter = this diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index f97a2d183ce5..4af144a03d98 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3774,7 +3774,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer adaptToSubType(wtp) case CompareResult.OKwithGADTUsed if pt.isValueType - && !inContext(ctx.fresh.setGadt(EmptyGadtConstraint)) { + && !inContext(ctx.fresh.setGadt(GadtConstraint.empty)) { val res = (tree.tpe.widenExpr frozen_<:< pt) if res then // we overshot; a cast is not needed, after all. From 4e8a1a641ccd76f92f5c6281dcf1f8a2a9b968f0 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 17 Oct 2022 08:27:50 +0200 Subject: [PATCH 2/3] Hide ConstraintHandling within GadtConstraint --- .../tools/dotc/core/GadtConstraint.scala | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala b/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala index c961baa01b97..f1e644fbe5d8 100644 --- a/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala @@ -13,24 +13,19 @@ import printing._ import scala.annotation.internal.sharable object GadtConstraint: - @sharable val empty = - new GadtConstraint(OrderingConstraint.empty, SimpleIdentityMap.empty, SimpleIdentityMap.empty, false) + @sharable val empty: GadtConstraint = + new ProperGadtConstraint(OrderingConstraint.empty, SimpleIdentityMap.empty, SimpleIdentityMap.empty, false) /** Represents GADT constraints currently in scope */ -final class GadtConstraint private( +sealed trait GadtConstraint ( private var myConstraint: Constraint, private var mapping: SimpleIdentityMap[Symbol, TypeVar], private var reverseMapping: SimpleIdentityMap[TypeParamRef, Symbol], private var wasConstrained: Boolean -) extends ConstraintHandling with Showable { - import dotty.tools.dotc.config.Printers.{gadts, gadtsConstr} +) extends Showable { + this: ConstraintHandling => - def this() = this( - myConstraint = new OrderingConstraint(SimpleIdentityMap.empty, SimpleIdentityMap.empty, SimpleIdentityMap.empty, SimpleIdentitySet.empty), - mapping = SimpleIdentityMap.empty, - reverseMapping = SimpleIdentityMap.empty, - wasConstrained = false - ) + import dotty.tools.dotc.config.Printers.{gadts, gadtsConstr} /** Exposes ConstraintHandling.subsumes */ def subsumes(left: GadtConstraint, right: GadtConstraint, pre: GadtConstraint)(using Context): Boolean = { @@ -195,7 +190,7 @@ final class GadtConstraint private( def symbols: List[Symbol] = mapping.keys - def fresh: GadtConstraint = new GadtConstraint(myConstraint, mapping, reverseMapping, wasConstrained) + def fresh: GadtConstraint = new ProperGadtConstraint(myConstraint, mapping, reverseMapping, wasConstrained) /** Restore the state from other [[GadtConstraint]], probably copied using [[fresh]] */ def restore(other: GadtConstraint): Unit = @@ -263,3 +258,10 @@ final class GadtConstraint private( /** Provides more information than toText, by showing the underlying Constraint details. */ def debugBoundsDescription(using Context): String = i"$this\n$constraint" } + +private class ProperGadtConstraint ( + myConstraint: Constraint, + mapping: SimpleIdentityMap[Symbol, TypeVar], + reverseMapping: SimpleIdentityMap[TypeParamRef, Symbol], + wasConstrained: Boolean, +) extends ConstraintHandling with GadtConstraint(myConstraint, mapping, reverseMapping, wasConstrained) From e6a9ffda5463243e7852466b438fbe628961a9fb Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 17 Oct 2022 17:13:07 +0200 Subject: [PATCH 3/3] Avoid sharing a mutable empty GadtConstraint! --- compiler/src/dotty/tools/dotc/core/GadtConstraint.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala b/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala index f1e644fbe5d8..53fc58595472 100644 --- a/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala @@ -10,10 +10,9 @@ import util.{SimpleIdentitySet, SimpleIdentityMap} import collection.mutable import printing._ -import scala.annotation.internal.sharable - object GadtConstraint: - @sharable val empty: GadtConstraint = + def apply(): GadtConstraint = empty + def empty: GadtConstraint = new ProperGadtConstraint(OrderingConstraint.empty, SimpleIdentityMap.empty, SimpleIdentityMap.empty, false) /** Represents GADT constraints currently in scope */