Skip to content

Commit f75f58f

Browse files
abgruszeckiabgruszecki
committed
Address review comments
1 parent 6cca0a6 commit f75f58f

File tree

3 files changed

+43
-57
lines changed

3 files changed

+43
-57
lines changed

‎compiler/src/dotty/tools/dotc/core/Contexts.scala‎

Lines changed: 34 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -745,64 +745,55 @@ object Contexts {
745745
override def addBound(sym: Symbol, bound: Type, isUpper: Boolean)(implicit ctx: Context): Boolean = try {
746746
boundCache = SimpleIdentityMap.Empty
747747
boundAdditionInProgress = true
748-
@annotation.tailrec def stripInst(tp: Type): Type = tp match {
748+
@annotation.tailrec def stripInternalTypeVar(tp: Type): Type = tp match {
749749
case tv: TypeVar =>
750750
val inst = instType(tv)
751-
if (inst.exists) stripInst(inst) else tv
751+
if (inst.exists) stripInternalTypeVar(inst) else tv
752752
case _ => tp
753753
}
754754

755-
def cautiousSubtype(tp1: Type, tp2: Type, isSubtype: Boolean): Boolean = {
755+
def externalizedSubtype(tp1: Type, tp2: Type, isSubtype: Boolean): Boolean = {
756756
val externalizedTp1 = removeTypeVars(tp1)
757757
val externalizedTp2 = removeTypeVars(tp2)
758758

759-
def descr = {
760-
def op = s"frozen_${if (isSubtype) "<:<" else ">:>"}"
761-
i"$tp1 $op $tp2\n\t$externalizedTp1 $op $externalizedTp2"
762-
}
763-
// gadts.println(descr)
764-
765-
val res =
766-
// TypeComparer.explain[Boolean](gadts.println) { implicit ctx =>
759+
(
767760
if (isSubtype) externalizedTp1 frozen_<:< externalizedTp2
768761
else externalizedTp2 frozen_<:< externalizedTp1
769-
// }
770-
771-
gadts.println(i"$descr = $res")
772-
res
762+
).reporting({ res =>
763+
val descr = i"$externalizedTp1 frozen_${if (isSubtype) "<:<" else ">:>"} $externalizedTp2"
764+
i"$descr = $res"
765+
}, gadts)
773766
}
774767

775-
def unify(tv: TypeVar, tp: Type): Unit = {
776-
gadts.println(i"manually unifying $tv with $tp")
777-
constraint = constraint.updateEntry(tv.origin, tp)
778-
}
779-
780-
val symTvar: TypeVar = stripInst(tvar(sym)) match {
768+
val symTvar: TypeVar = stripInternalTypeVar(tvar(sym)) match {
781769
case tv: TypeVar => tv
782770
case inst =>
783-
gadts.println(i"instantiated: $sym -> $inst")
784-
// this is wrong in general, but "correct" due to a subtype check in TypeComparer#narrowGadtBounds
785-
return true
771+
val externalizedInst = removeTypeVars(inst)
772+
gadts.println(i"instantiated: $sym -> $externalizedInst")
773+
return if (isUpper) isSubType(externalizedInst , bound) else isSubType(bound, externalizedInst)
786774
}
787775

788776
val internalizedBound = insertTypeVars(bound)
789-
val res = stripInst(internalizedBound) match {
790-
case boundTvar: TypeVar =>
791-
if (boundTvar eq symTvar) true
792-
else if (isUpper) addLess(symTvar.origin, boundTvar.origin)
793-
else addLess(boundTvar.origin, symTvar.origin)
794-
case bound =>
795-
if (cautiousSubtype(symTvar, bound, isSubtype = !isUpper)) { unify(symTvar, bound); true }
796-
else if (isUpper) addUpperBound(symTvar.origin, bound)
797-
else addLowerBound(symTvar.origin, bound)
798-
}
799-
800-
gadts.println {
777+
(
778+
stripInternalTypeVar(internalizedBound) match {
779+
case boundTvar: TypeVar =>
780+
if (boundTvar eq symTvar) true
781+
else if (isUpper) addLess(symTvar.origin, boundTvar.origin)
782+
else addLess(boundTvar.origin, symTvar.origin)
783+
case bound =>
784+
if (externalizedSubtype(symTvar, bound, isSubtype = !isUpper)) {
785+
gadts.println(i"manually unifying $symTvar with $bound")
786+
constraint = constraint.updateEntry(symTvar.origin, bound)
787+
true
788+
}
789+
else if (isUpper) addUpperBound(symTvar.origin, bound)
790+
else addLowerBound(symTvar.origin, bound)
791+
}
792+
).reporting({ res =>
801793
val descr = if (isUpper) "upper" else "lower"
802794
val op = if (isUpper) "<:" else ">:"
803795
i"adding $descr bound $sym $op $bound = $res\t( $symTvar $op $internalizedBound )"
804-
}
805-
res
796+
}, gadts)
806797
} finally boundAdditionInProgress = false
807798

808799
override def bounds(sym: Symbol)(implicit ctx: Context): TypeBounds = {
@@ -813,7 +804,7 @@ object Contexts {
813804
val tb = constraint.fullBounds(tv.origin)
814805
removeTypeVars(tb).asInstanceOf[TypeBounds]
815806
}
816-
val res =
807+
(
817808
if (boundAdditionInProgress || ctx.mode.is(Mode.GADTflexible)) retrieveBounds
818809
else boundCache(sym) match {
819810
case tb: TypeBounds => tb
@@ -822,8 +813,10 @@ object Contexts {
822813
boundCache = boundCache.updated(sym, bounds)
823814
bounds
824815
}
825-
// gadts.println(i"gadt bounds $sym: $res")
826-
res
816+
).reporting({ res =>
817+
// i"gadt bounds $sym: $res"
818+
""
819+
}, gadts)
827820
}
828821
}
829822

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

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,14 +1237,8 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] {
12371237
val tparam = tr.symbol
12381238
gadts.println(i"narrow gadt bound of $tparam: ${tparam.info} from ${if (isUpper) "above" else "below"} to $bound ${bound.toString} ${bound.isRef(tparam)}")
12391239
if (bound.isRef(tparam)) false
1240-
else {
1241-
val oldBounds = gadtBounds(tparam)
1242-
val newBounds =
1243-
if (isUpper) TypeBounds(oldBounds.lo, oldBounds.hi & bound)
1244-
else TypeBounds(oldBounds.lo | bound, oldBounds.hi)
1245-
isSubType(newBounds.lo, newBounds.hi) &&
1246-
(if (isUpper) gadtAddUpperBound(tparam, bound) else gadtAddLowerBound(tparam, bound))
1247-
}
1240+
else if (isUpper) gadtAddUpperBound(tparam, bound)
1241+
else gadtAddLowerBound(tparam, bound)
12481242
}
12491243
}
12501244

@@ -1826,13 +1820,14 @@ object TypeComparer {
18261820

18271821
val NoApprox: ApproxState = new ApproxState(0)
18281822

1829-
def explain[T](say: String => Unit)(op: Context => T)(implicit ctx: Context): T = {
1823+
/** Show trace of comparison operations when performing `op` as result string */
1824+
def explaining[T](say: String => Unit)(op: Context => T)(implicit ctx: Context): T = {
18301825
val (res, explanation) = underlyingExplained(op)
18311826
say(explanation)
18321827
res
18331828
}
18341829

1835-
/** Show trace of comparison operations when performing `op` as result string */
1830+
/** Like [[explaining]], but returns the trace instead */
18361831
def explained[T](op: Context => T)(implicit ctx: Context): String = {
18371832
underlyingExplained(op)._2
18381833
}

‎compiler/src/dotty/tools/dotc/typer/Inferencing.scala‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import NameKinds.UniqueName
1111
import util.Positions._
1212
import util.{Stats, SimpleIdentityMap}
1313
import Decorators._
14-
import config.Printers.typr
14+
import config.Printers.{gadts, typr}
1515
import annotation.tailrec
1616
import reporting._
1717
import collection.mutable
@@ -206,11 +206,9 @@ object Inferencing {
206206
}
207207

208208
val widePt = if (ctx.scala2Mode || refinementIsInvariant(tp)) pt else widenVariantParams(pt)
209-
import dotty.tools.dotc.config.Printers.gadts
210-
gadts.println(i"constraining pattern type $tp <:< $widePt")
211-
val res = tp <:< widePt
212-
gadts.println(i"constrained pattern type $tp <:< $widePt = $res\n${ctx.gadt.debugBoundsDescription}")
213-
res
209+
trace(i"constraining pattern type $tp <:< $widePt", gadts, res => s"$res\n${ctx.gadt.debugBoundsDescription}") {
210+
tp <:< widePt
211+
}
214212
}
215213

216214
/** The list of uninstantiated type variables bound by some prefix of type `T` which

0 commit comments

Comments
 (0)