Skip to content

Fix #6146: Fix bounds check involving wildcards and f-bounds #6147

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 5 commits into from
Mar 25, 2019
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
92 changes: 84 additions & 8 deletions compiler/src/dotty/tools/dotc/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -257,20 +257,96 @@ trait TypeOps { this: Context => // TODO: Make standalone object.
* @param boundss The list of type bounds
* @param instantiate A function that maps a bound type and the list of argument types to a resulting type.
* Needed to handle bounds that refer to other bounds.
* @param app The applied type whose arguments are checked, or NoType if
* arguments are for a TypeApply.
*
* This is particularly difficult for F-bounds that also contain wildcard arguments (see below).
* In fact the current treatment for this sitiuation can so far only be classified as "not obviously wrong",
* (maybe it still needs to be revised).
*/
def boundsViolations(args: List[Tree], boundss: List[TypeBounds], instantiate: (Type, List[Type]) => Type)(implicit ctx: Context): List[BoundsViolation] = {
def boundsViolations(args: List[Tree], boundss: List[TypeBounds], instantiate: (Type, List[Type]) => Type, app: Type)(implicit ctx: Context): List[BoundsViolation] = {
val argTypes = args.tpes

/** Replace all wildcards in `tps` with `<app>#<tparam>` where `<tparam>` is the
* type parameter corresponding to the wildcard.
*/
def skolemizeWildcardArgs(tps: List[Type], app: Type) = app match {
case AppliedType(tycon, args) if tycon.typeSymbol.isClass && !scala2Mode =>
tps.zipWithConserve(tycon.typeSymbol.typeParams) {
(tp, tparam) => tp match {
case _: TypeBounds => app.select(tparam)
case _ => tp
}
}
case _ => tps
}

// Skolemized argument types are used to substitute in F-bounds.
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"instantiating ${bounds.hi} with $argTypes")
//println(i" = ${instantiate(bounds.hi, argTypes)}")
val hiBound = instantiate(bounds.hi, argTypes.mapConserve(_.bounds.hi))
val loBound = instantiate(bounds.lo, argTypes.mapConserve(_.bounds.lo))
// Note that argTypes can contain a TypeBounds type for arguments that are
// not fully determined. In that case we need to check against the hi bound of the argument.
if (!(lo <:< hiBound)) violations += ((arg, "upper", hiBound))
if (!(loBound <:< hi)) violations += ((arg, "lower", bounds.lo))

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)
}

/** 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
}

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.addEmptyBounds(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)
}
}

val hiBound = instantiate(bounds.hi, skolemizedArgTypes)
val loBound = instantiate(bounds.lo, skolemizedArgTypes)

def check(implicit ctx: Context) = {
if (!(lo <:< hiBound)) violations += ((arg, "upper", hiBound))
if (!(loBound <:< hi)) violations += ((arg, "lower", loBound))
}
check(checkCtx)
}
arg.tpe match {
case TypeBounds(lo, hi) => checkOverlapsBounds(lo, hi)
Expand Down
9 changes: 5 additions & 4 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,24 @@ object Checking {
/** A general checkBounds method that can be used for TypeApply nodes as
* well as for AppliedTypeTree nodes. Also checks that type arguments to
* *-type parameters are fully applied.
* See TypeOps.boundsViolations for an explanation of the parameters.
*/
def checkBounds(args: List[tpd.Tree], boundss: List[TypeBounds], instantiate: (Type, List[Type]) => Type)(implicit ctx: Context): Unit = {
def checkBounds(args: List[tpd.Tree], boundss: List[TypeBounds], instantiate: (Type, List[Type]) => Type, app: Type = NoType)(implicit ctx: Context): Unit = {
(args, boundss).zipped.foreach { (arg, bound) =>
if (!bound.isLambdaSub && !arg.tpe.hasSimpleKind) {
// see MissingTypeParameterFor
ctx.error(ex"missing type parameter(s) for $arg", arg.sourcePos)
}
}
for ((arg, which, bound) <- ctx.boundsViolations(args, boundss, instantiate))
for ((arg, which, bound) <- ctx.boundsViolations(args, boundss, instantiate, app))
ctx.error(
DoesNotConformToBound(arg.tpe, which, bound)(err),
arg.sourcePos.focus)
}

/** Check that type arguments `args` conform to corresponding bounds in `tl`
* Note: This does not check the bounds of AppliedTypeTrees. These
* are handled by method checkBounds in FirstTransform
* are handled by method checkAppliedType below.
*/
def checkBounds(args: List[tpd.Tree], tl: TypeLambda)(implicit ctx: Context): Unit =
checkBounds(args, tl.paramInfos, _.substParams(tl, _))
Expand All @@ -77,7 +78,7 @@ object Checking {
val bounds = tparams.map(_.paramInfoAsSeenFrom(tree.tpe).bounds)
def instantiate(bound: Type, args: List[Type]) =
HKTypeLambda.fromParams(tparams, bound).appliedTo(args)
if (boundsCheck) checkBounds(orderedArgs, bounds, instantiate)
if (boundsCheck) checkBounds(orderedArgs, bounds, instantiate, tree.tpe)

def checkWildcardApply(tp: Type): Unit = tp match {
case tp @ AppliedType(tycon, _) =>
Expand Down
13 changes: 13 additions & 0 deletions tests/pos/i6146.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
trait BS[T, S <: BS[T, S]]
trait IS extends BS[Int, IS]

sealed trait BSElem[T, S <: BS[_, S]]
// old error: Type argument S does not conform to upper bound BS[Any, LazyRef(S)]

object BSElem {
implicit val intStreamShape: BSElem[Int, IS] = ???
}
class Ops[A] {
def asJavaSeqStream[S <: BS[_, S]](implicit s: BSElem[A, S]): S = ???
// old error: Type argument S does not conform to upper bound BS[Any, LazyRef(S)]
}