Skip to content

Implement avoid in terms of AproximatingTypeMap #2945

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.

Sign up for GitHub

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
odersky merged 10 commits into from
Aug 21, 2017
Merged

Implement avoid in terms of AproximatingTypeMap #2945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1f04bba
Factor out variance manipulation in TypeMap and TypeAccumulator
odersky Aug 13, 2017
104516b
Fix derivedRefinedType in ApproximatingTypeMap
odersky Aug 13, 2017
8d17f57
Add comment explaining approximating derivedAppliedType
odersky Aug 13, 2017
6e08737
Polishings and more comments
odersky Aug 13, 2017
826c96f
Fix review comments re homogenizedArg
odersky Aug 13, 2017
46a462b
Address reviewers comments
odersky Aug 14, 2017
dbd2e2c
Drop unused field `unsafeNonvariant`
odersky Aug 14, 2017
bc5cbb0
Fix typo
odersky Aug 14, 2017
b27dadc
Fix derivedSelect in avoid
odersky Aug 15, 2017
491fe97
Avoid double avoidance when typing blocks
odersky Aug 15, 2017
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
6 changes: 0 additions & 6 deletions compiler/src/dotty/tools/dotc/core/Contexts.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,12 +631,6 @@ object Contexts {
* of underlying during a controlled operation exists. */
private[core] val pendingUnderlying = new mutable.HashSet[Type]

/** A flag that some unsafe nonvariant instantiation was encountered
* in this run. Used as a shortcut to a avoid scans of types in
* Typer.typedSelect.
*/
private[dotty] var unsafeNonvariant: RunId = NoRunId

/** A map from ErrorType to associated message computation. We use this map
* instead of storing message computations directly in ErrorTypes in order
* to avoid space leaks - the message computation usually captures a context.
Expand Down
18 changes: 6 additions & 12 deletions compiler/src/dotty/tools/dotc/core/TypeOps.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ trait TypeOps { this: Context => // TODO: Make standalone object.

def apply(tp: Type): Type = {

/** Map a `C.this` type to the right prefix. If the prefix is unstable and
* the `C.this` occurs in nonvariant or contravariant position, mark the map
* to be unstable.
*/
/** Map a `C.this` type to the right prefix. If the prefix is unstable, and
* the current variance is <= 0, return a range.
*/
def toPrefix(pre: Type, cls: Symbol, thiscls: ClassSymbol): Type = /*>|>*/ ctx.conditionalTraceIndented(TypeOps.track, s"toPrefix($pre, $cls, $thiscls)") /*<|<*/ {
if ((pre eq NoType) || (pre eq NoPrefix) || (cls is PackageClass))
tp
Expand All @@ -50,16 +49,11 @@ trait TypeOps { this: Context => // TODO: Make standalone object.
}

/*>|>*/ ctx.conditionalTraceIndented(TypeOps.track, s"asSeen ${tp.show} from (${pre.show}, ${cls.show})", show = true) /*<|<*/ { // !!! DEBUG
// One `case ThisType` is specific to asSeenFrom, all other cases are inlined for performance
tp match {
case tp: NamedType => // inlined for performance; TODO: factor out into inline method
case tp: NamedType =>
if (tp.symbol.isStatic) tp
else {
val saved = variance
variance = variance max 0
val prefix1 = this(tp.prefix)
variance = saved
derivedSelect(tp, prefix1)
}
else derivedSelect(tp, atVariance(variance max 0)(this(tp.prefix)))
case tp: ThisType =>
toPrefix(pre, cls, tp.cls)
case _: BoundType | NoPrefix =>
Expand Down
141 changes: 78 additions & 63 deletions compiler/src/dotty/tools/dotc/core/Types.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3679,14 +3679,26 @@ object Types {

// ----- TypeMaps --------------------------------------------------------------------

abstract class TypeMap(implicit protected val ctx: Context) extends (Type => Type) { thisMap =>
/** Common base class of TypeMap and TypeAccumulator */
abstract class VariantTraversal {
protected[core] var variance = 1

@inline protected def atVariance[T](v: Int)(op: => T): T = {
val saved = variance
variance = v
val res = op
variance = saved
res
}
}

abstract class TypeMap(implicit protected val ctx: Context)
extends VariantTraversal with (Type => Type) { thisMap =>

protected def stopAtStatic = true

def apply(tp: Type): Type

protected[core] var variance = 1

protected def derivedSelect(tp: NamedType, pre: Type): Type =
tp.derivedSelect(pre)
protected def derivedRefinedType(tp: RefinedType, parent: Type, info: Type): Type =
Expand Down Expand Up @@ -3724,16 +3736,13 @@ object Types {
case tp: NamedType =>
if (stopAtStatic && tp.symbol.isStatic) tp
else {
val saved = variance
variance = variance max 0
val prefix1 = atVariance(variance max 0)(this(tp.prefix))
// A prefix is never contravariant. Even if say `p.A` is used in a contravariant
// context, we cannot assume contravariance for `p` because `p`'s lower
// bound might not have a binding for `A` (e.g. the lower bound could be `Nothing`).
// By contrast, covariance does translate to the prefix, since we have that
// if `p <: q` then `p.A <: q.A`, and well-formedness requires that `A` is a member
// of `p`'s upper bound.
val prefix1 = this(tp.prefix)
variance = saved
derivedSelect(tp, prefix1)
}
case _: ThisType
Expand All @@ -3744,11 +3753,7 @@ object Types {
derivedRefinedType(tp, this(tp.parent), this(tp.refinedInfo))

case tp: TypeAlias =>
val saved = variance
variance *= tp.variance
val alias1 = this(tp.alias)
variance = saved
derivedTypeAlias(tp, alias1)
derivedTypeAlias(tp, atVariance(variance * tp.variance)(this(tp.alias)))

case tp: TypeBounds =>
variance = -variance
Expand All @@ -3764,12 +3769,8 @@ object Types {
if (inst.exists) apply(inst) else tp

case tp: HKApply =>
def mapArg(arg: Type, tparam: ParamInfo): Type = {
val saved = variance
variance *= tparam.paramVariance
try this(arg)
finally variance = saved
}
def mapArg(arg: Type, tparam: ParamInfo): Type =
atVariance(variance * tparam.paramVariance)(this(arg))
derivedAppliedType(tp, this(tp.tycon),
tp.args.zipWithConserve(tp.typeParams)(mapArg))

Expand Down Expand Up @@ -3894,20 +3895,14 @@ object Types {
case _ => tp
}

protected def atVariance[T](v: Int)(op: => T): T = {
val saved = variance
variance = v
try op finally variance = saved
}

/** Derived selection.
* @pre the (upper bound of) prefix `pre` has a member named `tp.name`.
/** Try to widen a named type to its info relative to given prefix `pre`, where possible.
* The possible cases are listed inline in the code. Return `default` if no widening is
* possible.
*/
override protected def derivedSelect(tp: NamedType, pre: Type) =
if (pre eq tp.prefix) tp
else pre match {
case Range(preLo, preHi) =>
preHi.member(tp.name).info.widenExpr match {
def tryWiden(tp: NamedType, pre: Type)(default: => Type): Type =
pre.member(tp.name) match {
case d: SingleDenotation =>
d.info match {
case TypeAlias(alias) =>
// if H#T = U, then for any x in L..H, x.T =:= U,
// hence we can replace with U under all variances
Expand All @@ -3921,9 +3916,21 @@ object Types {
// hence we can replace with y.type under all variances
reapply(info)
case _ =>
range(tp.derivedSelect(preLo), tp.derivedSelect(preHi))
default
}
case _ => tp.derivedSelect(pre)
case _ => default
}

/** Derived selection.
* @pre the (upper bound of) prefix `pre` has a member named `tp.name`.
*/
override protected def derivedSelect(tp: NamedType, pre: Type) =
if (pre eq tp.prefix) tp
else pre match {
case Range(preLo, preHi) =>
tryWiden(tp, preHi)(range(tp.derivedSelect(preLo), tp.derivedSelect(preHi)))
case _ =>
tp.derivedSelect(pre)
}

override protected def derivedRefinedType(tp: RefinedType, parent: Type, info: Type) =
Expand All @@ -3932,20 +3939,30 @@ object Types {
case Range(parentLo, parentHi) =>
range(derivedRefinedType(tp, parentLo, info), derivedRefinedType(tp, parentHi, info))
case _ =>
def propagate(lo: Type, hi: Type) =
range(derivedRefinedType(tp, parent, lo), derivedRefinedType(tp, parent, hi))
if (parent.isBottomType) parent
else info match {
case Range(infoLo: TypeBounds, infoHi: TypeBounds) =>
assert(variance == 0)
val v1 = infoLo.variance
val v2 = infoHi.variance
// There's some weirdness coming from the way aliases can have variance
// If infoLo and infoHi are both aliases with the same non-zero variance
// we can propagate to a range of the refined types. If they are both
// non-alias ranges we know that infoLo <:< infoHi and therefore we can
// propagate to refined types with infoLo and infoHi as bounds.
// In all other cases, Nothing..Any is the only interval that contains
// the range. i966.scala is a test case.
if (v1 > 0 && v2 > 0) propagate(infoLo, infoHi)
else if (v1 < 0 && v2 < 0) propagate(infoHi, infoLo)
else if (!infoLo.isAlias && !infoHi.isAlias) propagate(infoLo, infoHi)
else range(tp.bottomType, tp.topType)
Copy link
Member

@smarter smarter Aug 13, 2017

Choose a reason for hiding this comment

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

Could parent be used instead of tp.topType ?

Copy link
Contributor Author

@odersky odersky Aug 14, 2017

Choose a reason for hiding this comment

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

I added a comment explaining why not:

            // Using `parent` instead of `tp.topType` would be better for normal refinements,
            // but it would also turn *-types to a hk-types, which is not what we want.
            // We should revisit this point in case we represent applied types not as refinements anymore.

Copy link
Member

@smarter smarter Aug 14, 2017

Choose a reason for hiding this comment

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

it would also turn *-types to a hk-types

Would it? TypeApplications#isHK only returns true for HKTypeLambda, and before this PR avoid used to return the parent for refinement types and it worked fine.

Copy link
Member

@smarter smarter Aug 14, 2017

Choose a reason for hiding this comment

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

Alternatively we could return derivedRefinedType(tp, parent, WildcardType) I think

// Using `parent` instead of `tp.topType` would be better for normal refinements,
// but it would also turn *-types into hk-types, which is not what we want.
// We should revisit this point in case we represent applied types not as refinements anymore.
case Range(infoLo, infoHi) =>
def propagate(lo: Type, hi: Type) =
range(derivedRefinedType(tp, parent, lo), derivedRefinedType(tp, parent, hi))
tp.refinedInfo match {
case rinfo: TypeBounds =>
val v = if (rinfo.isAlias) rinfo.variance * variance else variance
if (v > 0) tp.derivedRefinedType(parent, tp.refinedName, rangeToBounds(info))
else if (v < 0) propagate(infoHi, infoLo)
else range(tp.bottomType, tp.topType)
case _ =>
propagate(infoLo, infoHi)
}
propagate(infoLo, infoHi)
case _ =>
tp.derivedRefinedType(parent, tp.refinedName, info)
}
Expand Down Expand Up @@ -3987,6 +4004,13 @@ object Types {
if (variance > 0) tp.derivedAppliedType(tycon, args.map(rangeToBounds))
else {
val loBuf, hiBuf = new mutable.ListBuffer[Type]
// Given `C[A1, ..., An]` where sone A's are ranges, try to find
// non-range arguments L1, ..., Ln and H1, ..., Hn such that
// C[L1, ..., Ln] <: C[H1, ..., Hn] by taking the right limits of
// ranges that appear in as co- or contravariant arguments.
// Fail for non-variant argument ranges.
// If successful, the L-arguments are in loBut, the H-arguments in hiBuf.
// @return operation succeeded for all arguments.
def distributeArgs(args: List[Type], tparams: List[ParamInfo]): Boolean = args match {
case Range(lo, hi) :: args1 =>
val v = tparams.head.paramVariance
Expand All @@ -4006,13 +4030,14 @@ object Types {
range(tp.derivedAppliedType(tycon, loBuf.toList),
tp.derivedAppliedType(tycon, hiBuf.toList))
else range(tp.bottomType, tp.topType)
// TODO: can we give a better bound than `topType`?
}
}
else tp.derivedAppliedType(tycon, args)
}

override protected def derivedAndOrType(tp: AndOrType, tp1: Type, tp2: Type) =
if (tp1.isInstanceOf[Range] || tp2.isInstanceOf[Range])
if (isRange(tp1) || isRange(tp2))
if (tp.isAnd) range(lower(tp1) & lower(tp2), upper(tp1) & upper(tp2))
else range(lower(tp1) | lower(tp2), upper(tp1) | upper(tp2))
else tp.derivedAndOrType(tp1, tp2)
Expand All @@ -4030,7 +4055,9 @@ object Types {
}

override protected def derivedClassInfo(tp: ClassInfo, pre: Type): Type = {
assert(!pre.isInstanceOf[Range])
assert(!isRange(pre))
// we don't know what to do here; this case has to be handled in subclasses
// (typically by handling ClassInfo's specially, in case they can be encountered).
tp.derivedClassInfo(pre)
}

Expand Down Expand Up @@ -4058,23 +4085,17 @@ object Types {

// ----- TypeAccumulators ----------------------------------------------------

abstract class TypeAccumulator[T](implicit protected val ctx: Context) extends ((T, Type) => T) {
abstract class TypeAccumulator[T](implicit protected val ctx: Context)
extends VariantTraversal with ((T, Type) => T) {

protected def stopAtStatic = true

def apply(x: T, tp: Type): T

protected def applyToAnnot(x: T, annot: Annotation): T = x // don't go into annotations

protected var variance = 1

protected final def applyToPrefix(x: T, tp: NamedType) = {
val saved = variance
variance = variance max 0 // see remark on NamedType case in TypeMap
val result = this(x, tp.prefix)
variance = saved
result
}
protected final def applyToPrefix(x: T, tp: NamedType) =
atVariance(variance max 0)(this(x, tp.prefix)) // see remark on NamedType case in TypeMap

def foldOver(x: T, tp: Type): T = tp match {
case tp: TypeRef =>
Expand All @@ -4095,13 +4116,7 @@ object Types {
this(this(x, tp.parent), tp.refinedInfo)

case bounds @ TypeBounds(lo, hi) =>
if (lo eq hi) {
val saved = variance
variance = variance * bounds.variance
val result = this(x, lo)
variance = saved
result
}
if (lo eq hi) atVariance(variance * bounds.variance)(this(x, lo))
else {
variance = -variance
val y = this(x, lo)
Expand Down
7 changes: 4 additions & 3 deletions compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import typer.ImportInfo
import config.Config
import java.lang.Integer.toOctalString
import config.Config.summarizeDepth
import scala.util.control.NonFatal
import scala.annotation.switch

class PlainPrinter(_ctx: Context) extends Printer {
Expand Down Expand Up @@ -69,11 +70,11 @@ class PlainPrinter(_ctx: Context) extends Printer {
else tp

private def sameBound(lo: Type, hi: Type): Boolean =
try lo =:= hi
catch { case ex: Throwable => false }
try ctx.typeComparer.isSameTypeWhenFrozen(lo, hi)
catch { case NonFatal(ex) => false }

private def homogenizeArg(tp: Type) = tp match {
case TypeBounds(lo, hi) if sameBound(lo, hi) => homogenize(hi)
case TypeBounds(lo, hi) if homogenizedView && sameBound(lo, hi) => homogenize(hi)
case _ => tp
}

Expand Down
31 changes: 19 additions & 12 deletions compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ trait TypeAssigner {
val parentType = info.parentsWithArgs.reduceLeft(ctx.typeComparer.andType(_, _))
def addRefinement(parent: Type, decl: Symbol) = {
val inherited =
parentType.findMember(decl.name, info.cls.thisType, Private)
parentType.findMember(decl.name, info.cls.thisType, excluded = Private)
.suchThat(decl.matches(_))
val inheritedInfo = inherited.info
if (inheritedInfo.exists && decl.info <:< inheritedInfo && !(inheritedInfo <:< decl.info)) {
Expand Down Expand Up @@ -88,7 +88,7 @@ trait TypeAssigner {
case info => range(tp.info.bottomType, apply(info))
}
case tp: TypeRef if toAvoid(tp.symbol) =>
val avoided = tp.info match {
tp.info match {
case TypeAlias(alias) =>
apply(alias)
case TypeBounds(lo, hi) =>
Expand All @@ -98,7 +98,6 @@ trait TypeAssigner {
case _ =>
range(tp.bottomType, tp.topType) // should happen only in error cases
}
avoided
case tp: ThisType if toAvoid(tp.cls) =>
range(tp.bottomType, apply(classBound(tp.cls.classInfo)))
case tp: TypeVar if ctx.typerState.constraint.contains(tp) =>
Expand All @@ -109,18 +108,26 @@ trait TypeAssigner {
mapOver(tp)
}

/** Two deviations from standard derivedSelect:
* 1. The teh approximation result is a singleton references C#x.type, we
/** Three deviations from standard derivedSelect:
* 1. We first try a widening conversion to the type's info with
* the original prefix. Since the original prefix is known to
* be a subtype of the returned prefix, this can improve results.
* 2. IThen, if the approximation result is a singleton reference C#x.type, we
Copy link
Member

@smarter smarter Aug 16, 2017

Choose a reason for hiding this comment

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

typo: IThen -> Then

* replace by the widened type, which is usually more natural.
* 2. We need to handle the case where the prefix type does not have a member
* named `tp.name` anymmore.
* 3. Finally, we need to handle the case where the prefix type does not have a member
* named `tp.name` anymmore. In that case, we need to fall back to Bot..Top.
*/
override def derivedSelect(tp: NamedType, pre: Type) =
if (pre eq tp.prefix) tp
else if (tp.isTerm && variance > 0 && !pre.isInstanceOf[SingletonType])
apply(tp.info.widenExpr)
else if (upper(pre).member(tp.name).exists) super.derivedSelect(tp, pre)
else range(tp.bottomType, tp.topType)
if (pre eq tp.prefix)
tp
else tryWiden(tp, tp.prefix) {
if (tp.isTerm && variance > 0 && !pre.isInstanceOf[SingletonType])
apply(tp.info.widenExpr)
else if (upper(pre).member(tp.name).exists)
super.derivedSelect(tp, pre)
else
range(tp.bottomType, tp.topType)
}
}

widenMap(tp)
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
else pt.notApplied
val expr1 = typedExpr(tree.expr, ept)(exprCtx)
ensureNoLocalRefs(
assignType(cpy.Block(tree)(stats1, expr1), stats1, expr1), pt, localSyms(stats1))
cpy.Block(tree)(stats1, expr1).withType(expr1.tpe), pt, localSyms(stats1))
}

def escapingRefs(block: Tree, localSyms: => List[Symbol])(implicit ctx: Context): collection.Set[NamedType] = {
Expand Down
Loading