Skip to content

Fix #5948: Be more careful with capturing #5957

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 24 commits into from
Feb 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
51928fd
Change flags for Any_typeCast
odersky Feb 20, 2019
bdfc5a9
Systematically use `cast` in compiler
odersky Feb 20, 2019
237eba0
Replace TypeBound arguments by wildcards in wildApprox
odersky Feb 20, 2019
2b02931
Update decompiled files to account for new type tests
odersky Feb 21, 2019
086a218
Introduce capture wildcard adaptation
odersky Feb 21, 2019
c9be032
New method `substApprox` on types
odersky Feb 21, 2019
7410682
Drop capture conversion in type comparer
odersky Feb 21, 2019
f1ab7dc
Capture convert only if LHS is a path
odersky Feb 21, 2019
7165b98
Fix Definitions
odersky Feb 21, 2019
6d21081
Fix bytecode test
odersky Feb 21, 2019
545cfe7
Add missing library file
odersky Feb 21, 2019
3553f4e
Fix bytecode tests some more
odersky Feb 21, 2019
e04918e
Delete DottyByteCodeTest
odersky Feb 21, 2019
83ac988
Allow capture conversion after typer
odersky Feb 21, 2019
cee8162
Reinstantiate DottyByteCodeTest
odersky Feb 21, 2019
9e16a48
Drop dead code
odersky Feb 21, 2019
abb8146
Utility functions to test whether a type is a TypeBounds
odersky Feb 21, 2019
78939ca
Document capture conversion in TypeComparer and Typer
odersky Feb 22, 2019
79bfff4
Document TypeBox and move to library/src
odersky Feb 22, 2019
100bdeb
Address remaining review comments
odersky Feb 22, 2019
788fac8
Transform infos of SkolemTypes in TypeMap
odersky Feb 22, 2019
09b8754
Make SkolemTypes immutable and non-recursive.
odersky Feb 22, 2019
6e811b4
Print info of SkolemTypes under -Xprint-types
odersky Feb 22, 2019
d4306dd
Document two aspects suggested by review
odersky Feb 23, 2019
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
11 changes: 9 additions & 2 deletions compiler/src/dotty/tools/dotc/ast/tpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -885,13 +885,20 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
tree.select(defn.Any_asInstanceOf).appliedToType(tp)
}

/** `tree.asInstanceOf[tp]` (or its box/unbox/cast equivalent when after
/** cast tree to `tp`, assuming no exception is raised, i.e the operation is pure */
def cast(tp: Type)(implicit ctx: Context): Tree = {
assert(tp.isValueType, i"bad cast: $tree.asInstanceOf[$tp]")
tree.select(if (ctx.erasedTypes) defn.Any_asInstanceOf else defn.Any_typeCast)
.appliedToType(tp)
}

/** cast `tree` to `tp` (or its box/unbox/cast equivalent when after
* erasure and value and non-value types are mixed),
* unless tree's type already conforms to `tp`.
*/
def ensureConforms(tp: Type)(implicit ctx: Context): Tree =
if (tree.tpe <:< tp) tree
else if (!ctx.erasedTypes) asInstance(tp)
else if (!ctx.erasedTypes) cast(tp)
else Erasure.Boxing.adaptToType(tree, tp)

/** `tree ne null` (might need a cast to be type correct) */
Expand Down
8 changes: 6 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ class Definitions {
lazy val Any_isInstanceOf: TermSymbol = enterT1ParameterlessMethod(AnyClass, nme.isInstanceOf_, _ => BooleanType, Final)
lazy val Any_asInstanceOf: TermSymbol = enterT1ParameterlessMethod(AnyClass, nme.asInstanceOf_, _.paramRefs(0), Final)
lazy val Any_typeTest: TermSymbol = enterT1ParameterlessMethod(AnyClass, nme.isInstanceOfPM, _ => BooleanType, Final | Synthetic | Artifact)
lazy val Any_typeCast: TermSymbol = enterT1ParameterlessMethod(AnyClass, nme.asInstanceOfPM, _.paramRefs(0), Final | Synthetic | Artifact | Erased)
lazy val Any_typeCast: TermSymbol = enterT1ParameterlessMethod(AnyClass, nme.asInstanceOfPM, _.paramRefs(0), Final | Synthetic | Artifact | StableRealizable)
// generated by pattern matcher, eliminated by erasure

def AnyMethods: List[TermSymbol] = List(Any_==, Any_!=, Any_equals, Any_hashCode,
Expand Down Expand Up @@ -680,7 +680,7 @@ class Definitions {
lazy val ModuleSerializationProxyType: TypeRef = ctx.requiredClassRef("scala.runtime.ModuleSerializationProxy")
def ModuleSerializationProxyClass(implicit ctx: Context): ClassSymbol = ModuleSerializationProxyType.symbol.asClass
lazy val ModuleSerializationProxyConstructor: TermSymbol =
ModuleSerializationProxyClass.requiredMethod(nme.CONSTRUCTOR, List(ClassType(WildcardType)))
ModuleSerializationProxyClass.requiredMethod(nme.CONSTRUCTOR, List(ClassType(TypeBounds.empty)))

lazy val GenericType: TypeRef = ctx.requiredClassRef("scala.reflect.Generic")
def GenericClass(implicit ctx: Context): ClassSymbol = GenericType.symbol.asClass
Expand Down Expand Up @@ -755,6 +755,10 @@ class Definitions {

def Eql_eqlAny(implicit ctx: Context): TermSymbol = EqlModule.requiredMethod(nme.eqlAny)

lazy val TypeBoxType: TypeRef = ctx.requiredClassRef("scala.internal.TypeBox")

lazy val TypeBox_CAP: TypeSymbol = TypeBoxType.symbol.requiredType(tpnme.CAP)

lazy val NotType: TypeRef = ctx.requiredClassRef("scala.implicits.Not")
def NotClass(implicit ctx: Context): ClassSymbol = NotType.symbol.asClass
def NotModule(implicit ctx: Context): Symbol = NotClass.companionModule
Expand Down
2 changes: 0 additions & 2 deletions compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,6 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
entries != null && isBounds(entries(pnum)) && (typeVar(entries, pnum) eq tvar)
}

private def isBounds(tp: Type) = tp.isInstanceOf[TypeBounds]

// ---------- Dependency handling ----------------------------------------------

def lower(param: TypeParamRef): List[TypeParamRef] = lowerLens(this, param.binder, param.paramNum)
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/core/StdNames.scala
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ object StdNames {
val AnnotatedType: N = "AnnotatedType"
val AppliedTypeTree: N = "AppliedTypeTree"
val ArrayAnnotArg: N = "ArrayAnnotArg"
val CAP: N = "CAP"
val Constant: N = "Constant"
val ConstantType: N = "ConstantType"
val doubleHash: N = "doubleHash"
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1790,7 +1790,7 @@ object SymDenotations {
case LambdaParam(_, _) :: _ =>
recur(tp.superType)
case tparams: List[Symbol @unchecked] =>
new ctx.SubstApproxMap(tparams, args).apply(recur(tycon))
recur(tycon).substApprox(tparams, args)
}
record(tp, baseTp)
baseTp
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/core/TypeApplications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ object TypeApplications {
private[this] var available = (0 until args.length).toSet
var allReplaced: Boolean = true
def hasWildcardArg(p: TypeParamRef): Boolean =
p.binder == tycon && args(p.paramNum).isInstanceOf[TypeBounds]
p.binder == tycon && isBounds(args(p.paramNum))
def canReduceWildcard(p: TypeParamRef): Boolean =
!ctx.mode.is(Mode.AllowLambdaWildcardApply) || available.contains(p.paramNum)
def atNestedLevel(op: => Type): Type = {
Expand Down Expand Up @@ -356,7 +356,7 @@ class TypeApplications(val self: Type) extends AnyVal {
else dealiased match {
case dealiased: HKTypeLambda =>
def tryReduce =
if (!args.exists(_.isInstanceOf[TypeBounds])) {
if (!args.exists(isBounds)) {
val followAlias = Config.simplifyApplications && {
dealiased.resType match {
case AppliedType(tyconBody, dealiasedArgs) =>
Expand Down
183 changes: 137 additions & 46 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -130,21 +130,51 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] {
}
}

private[this] var approx: ApproxState = NoApprox
/** The current approximation state. See `ApproxState`. */
private[this] var approx: ApproxState = FreshApprox
protected def approxState: ApproxState = approx

/** The original left-hand type of the comparison. Gets reset
* everytime we compare components of the previous pair of types.
* This type is used for capture conversion in `isSubArgs`.
*/
private [this] var leftRoot: Type = _

protected def isSubType(tp1: Type, tp2: Type, a: ApproxState): Boolean = {
val saved = approx
this.approx = a
val savedApprox = approx
val savedLeftRoot = leftRoot
if (a == FreshApprox) {
this.approx = NoApprox
this.leftRoot = tp1
}
else this.approx = a
try recur(tp1, tp2)
catch {
case ex: Throwable => handleRecursive("subtype", i"$tp1 <:< $tp2", ex, weight = 2)
}
finally this.approx = saved
finally {
this.approx = savedApprox
this.leftRoot = savedLeftRoot
}
}

def isSubType(tp1: Type, tp2: Type)(implicit nc: AbsentContext): Boolean = isSubType(tp1, tp2, NoApprox)

def isSubType(tp1: Type, tp2: Type)(implicit nc: AbsentContext): Boolean = isSubType(tp1, tp2, FreshApprox)

/** The inner loop of the isSubType comparison.
* Recursive calls from recur should go to recur directly if the two types
* compared in the callee are essentially the same as the types compared in the
* caller. "The same" means: represent essentially the same sets of values.
* `recur` should not be used to compare components of types. In this case
* one should use `isSubType(_, _)`.
* `recur` should also not be used to compare approximated versions of the original
* types (as when we go from an abstract type to one of its bounds). In that case
* one should use `isSubType(_, _, a)` where `a` defines the kind of approximation.
*
* Note: Logicaly, `recur` could be nested in `isSubType`, which would avoid
* the instance state consisting `approx` and `leftRoot`. But then the implemented
* code would have two extra parameters for each of the many calls that go from
* one sub-part of isSubType to another.
*/
protected def recur(tp1: Type, tp2: Type): Boolean = trace(s"isSubType ${traceInfo(tp1, tp2)} $approx", subtyping) {

def monitoredIsSubType = {
Expand Down Expand Up @@ -277,7 +307,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] {
case tp2: SuperType =>
def compareSuper = tp1 match {
case tp1: SuperType =>
isSubType(tp1.thistpe, tp2.thistpe) &&
recur(tp1.thistpe, tp2.thistpe) &&
isSameType(tp1.supertpe, tp2.supertpe)
case _ =>
secondTry
Expand Down Expand Up @@ -355,7 +385,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] {
}
case tp1: SkolemType =>
tp2 match {
case tp2: SkolemType if !ctx.phase.isTyper && isSubType(tp1.info, tp2.info) => true
case tp2: SkolemType if !ctx.phase.isTyper && recur(tp1.info, tp2.info) => true
case _ => thirdTry
}
case tp1: TypeVar =>
Expand Down Expand Up @@ -449,7 +479,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] {
// So if the constraint is not yet frozen, we do the same comparison again
// with a frozen constraint, which means that we get a chance to do the
// widening in `fourthTry` before adding to the constraint.
if (frozenConstraint) isSubType(tp1, bounds(tp2).lo)
if (frozenConstraint) recur(tp1, bounds(tp2).lo)
else isSubTypeWhenFrozen(tp1, tp2)
alwaysTrue || {
if (canConstrain(tp2) && !approx.low)
Expand Down Expand Up @@ -879,7 +909,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] {
compareLower(info2, tyconIsTypeRef = true)
case info2: ClassInfo =>
tycon2.name.toString.startsWith("Tuple") &&
defn.isTupleType(tp2) && isSubType(tp1, tp2.toNestedPairs) ||
defn.isTupleType(tp2) && recur(tp1, tp2.toNestedPairs) ||
tryBaseType(info2.cls)
case _ =>
fourthTry
Expand Down Expand Up @@ -1001,44 +1031,93 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] {
}

/** Subtype test for corresponding arguments in `args1`, `args2` according to
* variances in type parameters `tparams`.
* variances in type parameters `tparams2`.
* @param tp1 The applied type containing `args1`
* @param tparams2 The type parameters of the type constructor applied to `args2`
*/
def isSubArgs(args1: List[Type], args2: List[Type], tp1: Type, tparams: List[ParamInfo]): Boolean =
if (args1.isEmpty) args2.isEmpty
else args2.nonEmpty && {
val tparam = tparams.head
val v = tparam.paramVariance

def compareCaptured(arg1: Type, arg2: Type): Boolean = arg1 match {
case arg1: TypeBounds =>
val captured = TypeRef(tp1, tparam.asInstanceOf[TypeSymbol])
isSubArg(captured, arg2)
case _ =>
false
}
def isSubArgs(args1: List[Type], args2: List[Type], tp1: Type, tparams2: List[ParamInfo]): Boolean = {

def isSubArg(arg1: Type, arg2: Type): Boolean = arg2 match {
case arg2: TypeBounds =>
arg2.contains(arg1) || compareCaptured(arg1, arg2)
case _ =>
arg1 match {
case arg1: TypeBounds =>
compareCaptured(arg1, arg2)
case _ =>
(v > 0 || isSubType(arg2, arg1)) &&
(v < 0 || isSubType(arg1, arg2))
}
}
/** The bounds of parameter `tparam`, where all references to type paramneters
* are replaced by corresponding arguments (or their approximations in the case of
* wildcard arguments).
*/
def paramBounds(tparam: Symbol): TypeBounds =
tparam.info.substApprox(tparams2.asInstanceOf[List[Symbol]], args2).bounds

val arg1 = args1.head
val arg2 = args2.head
isSubArg(arg1, arg2) || {
// last effort: try to adapt variances of higher-kinded types if this is sound.
// TODO: Move this to eta-expansion?
val adapted2 = arg2.adaptHkVariances(tparam.paramInfo)
adapted2.ne(arg2) && isSubArg(arg1, adapted2)
}
} && isSubArgs(args1.tail, args2.tail, tp1, tparams.tail)
def recurArgs(args1: List[Type], args2: List[Type], tparams2: List[ParamInfo]): Boolean =
if (args1.isEmpty) args2.isEmpty
else args2.nonEmpty && {
val tparam = tparams2.head
val v = tparam.paramVariance

/** Try a capture conversion:
* If the original left-hand type `leftRoot` is a path `p.type`,
* and the current widened left type is an application with wildcard arguments
* such as `C[_]`, where `X` is `C`'s type parameter corresponding to the `_` argument,
* compare with `C[p.X]` instead. Otherwise return `false`.
* Also do a capture conversion in either of the following cases:
*
* - If we are after typer. We generally relax soundness requirements then.
* We need the relaxed condition to correctly compute overriding relationships.
* Missing this case led to AbstractMethod errors in the bootstrap.
*
* - If we are in mode TypevarsMissContext, which means we test implicits
* for eligibility. In this case, we can be more permissive, since it's
* just a pre-check. This relaxation is needed since the full
* implicit typing might perform an adaptation that skolemizes the
* type of a synthesized tree before comparing it with an expected type.
* But no such adaptation is applied for implicit eligibility
* testing, so we have to compensate.
*
* Note: Doing the capture conversion on path types is actually not necessary
* since we can already deal with the situation through skolemization in Typer#captureWildcards.
* But performance tests indicate that it's better to do it, since we avoid
* skolemizations, which are more expensive . And, besides, capture conversion on
* paths is less intrusive than skolemization.
*/
def compareCaptured(arg1: TypeBounds, arg2: Type) = tparam match {
case tparam: Symbol
if leftRoot.isStable || ctx.isAfterTyper || ctx.mode.is(Mode.TypevarsMissContext) =>
val captured = TypeRef(leftRoot, tparam)
assert(captured.exists, i"$leftRoot has no member $tparam in isSubArgs($args1, $args2, $tp1, $tparams2)")
isSubArg(captured, arg2)
case _ =>
false
}

def isSubArg(arg1: Type, arg2: Type): Boolean = arg2 match {
case arg2: TypeBounds =>
val arg1norm = arg1 match {
case arg1: TypeBounds =>
tparam match {
case tparam: Symbol => arg1 & paramBounds(tparam)
case _ => arg1 // This case can only arise when a hk-type is illegally instantiated with a wildcard
}
case _ => arg1
}
arg2.contains(arg1norm)
case _ =>
arg1 match {
case arg1: TypeBounds =>
compareCaptured(arg1, arg2)
case _ =>
(v > 0 || isSubType(arg2, arg1)) &&
(v < 0 || isSubType(arg1, arg2))
}
}

val arg1 = args1.head
val arg2 = args2.head
isSubArg(arg1, arg2) || {
// last effort: try to adapt variances of higher-kinded types if this is sound.
// TODO: Move this to eta-expansion?
val adapted2 = arg2.adaptHkVariances(tparam.paramInfo)
adapted2.ne(arg2) && isSubArg(arg1, adapted2)
}
} && recurArgs(args1.tail, args2.tail, tparams2.tail)

recurArgs(args1, args2, tparams2)
}

/** Test whether `tp1` has a base type of the form `B[T1, ..., Tn]` where
* - `B` derives from one of the class symbols of `tp2`,
Expand Down Expand Up @@ -1493,7 +1572,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] {
if (common.exists) common
else if (v > 0) glb(arg1.hiBound, arg2.hiBound)
else if (v < 0) lub(arg1.loBound, arg2.loBound)
else if (arg1.isInstanceOf[TypeBounds] || arg2.isInstanceOf[TypeBounds])
else if (isBounds(arg1) || isBounds(arg2))
TypeBounds(lub(arg1.loBound, arg2.loBound),
glb(arg1.hiBound, arg2.hiBound))
else if (homogenizeArgs && !frozenConstraint && isSameType(arg1, arg2)) arg1
Expand Down Expand Up @@ -1813,6 +1892,12 @@ object TypeComparer {
private val LoApprox = 1
private val HiApprox = 2

/** The approximation state indicates how the pair of types currently compared
* relates to the types compared originally.
* - `NoApprox`: They are still the same types
* - `LoApprox`: The left type is approximated (i.e widened)"
* - `HiApprox`: The right type is approximated (i.e narrowed)"
*/
class ApproxState(private val bits: Int) extends AnyVal {
override def toString: String = {
val lo = if ((bits & LoApprox) != 0) "LoApprox" else ""
Expand All @@ -1827,6 +1912,12 @@ object TypeComparer {

val NoApprox: ApproxState = new ApproxState(0)

/** A special approximation state to indicate that this is the first time we
* compare (approximations of) this pair of types. It's converted to `NoApprox`
* in `isSubType`, but also leads to `leftRoot` being set there.
*/
val FreshApprox: ApproxState = new ApproxState(4)

/** Show trace of comparison operations when performing `op` as result string */
def explaining[T](say: String => Unit)(op: Context => T)(implicit ctx: Context): T = {
val nestedCtx = ctx.fresh.setTypeComparerFn(new ExplainingTypeComparer(_))
Expand Down
Loading