Skip to content

Some smaller optimizations and refactorings #9617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Constraint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,13 @@ abstract class Constraint extends Showable {
/** A new constraint with entry `tl` renamed to a fresh type lambda */
def rename(tl: TypeLambda)(using Context): This

/** Gives for each instantiated type var that does not yet have its `inst` field
* set, the instance value stored in the constraint. Storing instances in constraints
* is done only in a temporary way for contexts that may be retracted
* without also retracting the type var as a whole.
*/
def instType(tvar: TypeVar): Type

/** The given `tl` in case it is not contained in this constraint,
* a fresh copy of `tl` otherwise.
*/
Expand Down
13 changes: 0 additions & 13 deletions compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,6 @@ trait ConstraintHandling {
assert(homogenizeArgs == false)
assert(comparedTypeLambdas == Set.empty)

/** Gives for each instantiated type var that does not yet have its `inst` field
* set, the instance value stored in the constraint. Storing instances in constraints
* is done only in a temporary way for contexts that may be retracted
* without also retracting the type var as a whole.
*/
def instType(tvar: TypeVar): Type = constraint.entry(tvar.origin) match {
case _: TypeBounds => NoType
case tp: TypeParamRef =>
var tvar1 = constraint.typeVarOfParam(tp)
if (tvar1.exists) tvar1 else tp
case tp => tp
}

def nonParamBounds(param: TypeParamRef)(using Context): TypeBounds = constraint.nonParamBounds(param)

def fullLowerBound(param: TypeParamRef)(using Context): Type =
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/GadtConstraint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ final class ProperGadtConstraint private(
override 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 = instType(tv)
val inst = constraint.instType(tv)
if (inst.exists) stripInternalTypeVar(inst) else tv
case _ => tp
}
Expand Down
5 changes: 5 additions & 0 deletions compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,11 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
current.checkNonCyclic()
}

def instType(tvar: TypeVar): Type = entry(tvar.origin) match
case _: TypeBounds => NoType
case tp: TypeParamRef => typeVarOfParam(tp).orElse(tp)
case tp => tp

def ensureFresh(tl: TypeLambda)(using Context): TypeLambda =
if (contains(tl)) {
var paramInfos = tl.paramInfos
Expand Down
3 changes: 0 additions & 3 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2605,9 +2605,6 @@ object TypeComparer {
def subtypeCheckInProgress(using Context): Boolean =
comparing(_.subtypeCheckInProgress)

def instType(tvar: TypeVar)(using Context): Type =
comparing(_.instType(tvar))

def instanceType(param: TypeParamRef, fromBelow: Boolean)(using Context): Type =
comparing(_.instanceType(param, fromBelow))

Expand Down
121 changes: 63 additions & 58 deletions compiler/src/dotty/tools/dotc/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -529,74 +529,79 @@ object TypeOps:
val skolemizedArgTypes = skolemizeWildcardArgs(argTypes, app)
val violations = new mutable.ListBuffer[BoundsViolation]

for ((arg, bounds) <- args zip boundss) {
def checkOverlapsBounds(lo: Type, hi: Type): Unit = {
//println(i" = ${instantiate(bounds.hi, argTypes)}")

var checkCtx = ctx // the context to be used for bounds checking
if (argTypes ne skolemizedArgTypes) { // some of the arguments are wildcards

/** Is there a `LazyRef(TypeRef(_, sym))` reference in `tp`? */
def isLazyIn(sym: Symbol, tp: Type): Boolean = {
def isReference(tp: Type) = tp match {
case tp: LazyRef => tp.ref.isInstanceOf[TypeRef] && tp.ref.typeSymbol == sym
case _ => false
}
tp.existsPart(isReference, forceLazy = false)
}
def checkOverlapsBounds(lo: Type, hi: Type, arg: Tree, bounds: TypeBounds): Unit = {
//println(i" = ${instantiate(bounds.hi, argTypes)}")

/** The argument types of the form `TypeRef(_, sym)` which appear as a LazyRef in `bounds`.
* This indicates that the application is used as an F-bound for the symbol referred to in the LazyRef.
*/
val lazyRefs = skolemizedArgTypes collect {
case tp: TypeRef if isLazyIn(tp.symbol, bounds) => tp.symbol
}
var checkCtx = ctx // the context to be used for bounds checking
if (argTypes ne skolemizedArgTypes) { // some of the arguments are wildcards

for (sym <- lazyRefs) {

// If symbol `S` has an F-bound such as `C[?, S]` that contains wildcards,
// add a modifieed bound where wildcards are skolemized as a GADT bound for `S`.
// E.g. for `C[?, S]` we would add `C[C[?, S]#T0, S]` where `T0` is the first
// type parameter of `C`. The new bound is added as a GADT bound for `S` in
// `checkCtx`.
// This mirrors what we do for the bounds that are checked and allows us thus
// to bounds-check F-bounds with wildcards. A test case is pos/i6146.scala.

def massage(tp: Type): Type = tp match {
case tp @ AppliedType(tycon, args) =>
tp.derivedAppliedType(tycon, skolemizeWildcardArgs(args, tp))
case tp: AndOrType =>
tp.derivedAndOrType(massage(tp.tp1), massage(tp.tp2))
case _ => tp
}
def narrowBound(bound: Type, fromBelow: Boolean): Unit = {
val bound1 = massage(bound)
if (bound1 ne bound) {
if (checkCtx eq ctx) checkCtx = ctx.fresh.setFreshGADTBounds
if (!checkCtx.gadt.contains(sym)) checkCtx.gadt.addToConstraint(sym)
checkCtx.gadt.addBound(sym, bound1, fromBelow)
typr.println("install GADT bound $bound1 for when checking F-bounded $sym")
}
}
narrowBound(sym.info.loBound, fromBelow = true)
narrowBound(sym.info.hiBound, fromBelow = false)
/** Is there a `LazyRef(TypeRef(_, sym))` reference in `tp`? */
def isLazyIn(sym: Symbol, tp: Type): Boolean = {
def isReference(tp: Type) = tp match {
case tp: LazyRef => tp.ref.isInstanceOf[TypeRef] && tp.ref.typeSymbol == sym
case _ => false
}
tp.existsPart(isReference, forceLazy = false)
}

val hiBound = instantiate(bounds.hi, skolemizedArgTypes)
val loBound = instantiate(bounds.lo, skolemizedArgTypes)
/** The argument types of the form `TypeRef(_, sym)` which appear as a LazyRef in `bounds`.
* This indicates that the application is used as an F-bound for the symbol referred to in the LazyRef.
*/
val lazyRefs = skolemizedArgTypes collect {
case tp: TypeRef if isLazyIn(tp.symbol, bounds) => tp.symbol
}

def check(using Context) = {
if (!(lo <:< hiBound)) violations += ((arg, "upper", hiBound))
if (!(loBound <:< hi)) violations += ((arg, "lower", loBound))
for (sym <- lazyRefs) {

// If symbol `S` has an F-bound such as `C[?, S]` that contains wildcards,
// add a modifieed bound where wildcards are skolemized as a GADT bound for `S`.
// E.g. for `C[?, S]` we would add `C[C[?, S]#T0, S]` where `T0` is the first
// type parameter of `C`. The new bound is added as a GADT bound for `S` in
// `checkCtx`.
// This mirrors what we do for the bounds that are checked and allows us thus
// to bounds-check F-bounds with wildcards. A test case is pos/i6146.scala.

def massage(tp: Type): Type = tp match {
case tp @ AppliedType(tycon, args) =>
tp.derivedAppliedType(tycon, skolemizeWildcardArgs(args, tp))
case tp: AndOrType =>
tp.derivedAndOrType(massage(tp.tp1), massage(tp.tp2))
case _ => tp
}
def narrowBound(bound: Type, fromBelow: Boolean): Unit = {
val bound1 = massage(bound)
if (bound1 ne bound) {
if (checkCtx eq ctx) checkCtx = ctx.fresh.setFreshGADTBounds
if (!checkCtx.gadt.contains(sym)) checkCtx.gadt.addToConstraint(sym)
checkCtx.gadt.addBound(sym, bound1, fromBelow)
typr.println("install GADT bound $bound1 for when checking F-bounded $sym")
}
}
narrowBound(sym.info.loBound, fromBelow = true)
narrowBound(sym.info.hiBound, fromBelow = false)
}
check(using checkCtx)
}
arg.tpe match {
case TypeBounds(lo, hi) => checkOverlapsBounds(lo, hi)
case tp => checkOverlapsBounds(tp, tp)
val hiBound = instantiate(bounds.hi, skolemizedArgTypes)
val loBound = instantiate(bounds.lo, skolemizedArgTypes)

def check(using Context) = {
if (!(lo <:< hiBound)) violations += ((arg, "upper", hiBound))
if (!(loBound <:< hi)) violations += ((arg, "lower", loBound))
}
check(using checkCtx)
}

def loop(args: List[Tree], boundss: List[TypeBounds]): Unit = args match
case arg :: args1 => boundss match
case bounds :: boundss1 =>
arg.tpe match
case TypeBounds(lo, hi) => checkOverlapsBounds(lo, hi, arg, bounds)
case tp => checkOverlapsBounds(tp, tp, arg, bounds)
loop(args1, boundss1)
case _ =>
case _ =>

loop(args, boundss)
violations.toList
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/core/TyperState.scala
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,11 @@ class TyperState() {
* no-longer needed constraint entries.
*/
def gc()(using Context): Unit = {
Stats.record("typerState.gc")
val toCollect = new mutable.ListBuffer[TypeLambda]
constraint foreachTypeVar { tvar =>
if (!tvar.inst.exists) {
val inst = TypeComparer.instType(tvar)
val inst = constraint.instType(tvar)
if (inst.exists && (tvar.owningState.get eq this)) {
tvar.inst = inst
val lam = tvar.origin.binder
Expand Down
12 changes: 10 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3211,8 +3211,16 @@ object Types {
else newLikeThis(paramNames, paramInfos, resType)

def newLikeThis(paramNames: List[ThisName], paramInfos: List[PInfo], resType: Type)(using Context): This =
def substParams(pinfos: List[PInfo], to: This): List[PInfo] = pinfos match
case pinfo :: rest =>
val pinfo1 = pinfo.subst(this, to).asInstanceOf[PInfo]
val rest1 = substParams(rest, to)
if (pinfo1 eq pinfo) && (rest1 eq rest) then pinfos
else pinfo1 :: rest1
case nil =>
nil
companion(paramNames)(
x => paramInfos.mapConserve(_.subst(this, x).asInstanceOf[PInfo]),
x => substParams(paramInfos, x),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is in conflict with the changes in #9618.

x => resType.subst(this, x))

protected def prefixString: String
Expand Down Expand Up @@ -4184,7 +4192,7 @@ object Types {
* uninstantiated
*/
def instanceOpt(using Context): Type =
if (inst.exists) inst else TypeComparer.instType(this)
if (inst.exists) inst else ctx.typerState.constraint.instType(this)

/** Is the variable already instantiated? */
def isInstantiated(using Context): Boolean = instanceOpt.exists
Expand Down