From 01dbaeb6c6f5ac0155af5a76c690ae1adb88b854 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 22 Mar 2019 16:33:30 +0100 Subject: [PATCH 1/5] Fix #6146: Fix bounds check involving wildcards and f-bounds Don't map arguments to their upper bounds before doing the bound check. There was a comment justifiying that by talking about non fully-determined arguments, but this check is done in PostTyper when all types should already be fully-determined. Given that this comment was written in 2014 I'm assuming it's no longer relevant. --- compiler/src/dotty/tools/dotc/core/TypeOps.scala | 6 ++---- tests/pos/i6146.scala | 13 +++++++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 tests/pos/i6146.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeOps.scala b/compiler/src/dotty/tools/dotc/core/TypeOps.scala index 6ba033df4bdf..c7983eb8a26a 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeOps.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeOps.scala @@ -265,10 +265,8 @@ trait TypeOps { this: Context => // TODO: Make standalone object. 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. + val hiBound = instantiate(bounds.hi, argTypes) + val loBound = instantiate(bounds.lo, argTypes) if (!(lo <:< hiBound)) violations += ((arg, "upper", hiBound)) if (!(loBound <:< hi)) violations += ((arg, "lower", bounds.lo)) } diff --git a/tests/pos/i6146.scala b/tests/pos/i6146.scala new file mode 100644 index 000000000000..c0a98c910018 --- /dev/null +++ b/tests/pos/i6146.scala @@ -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)] +} From 48f7ed677d9256ee19b09377b839a5292438dd2d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 23 Mar 2019 17:22:19 +0100 Subject: [PATCH 2/5] Use substApprox for F-bounds instantiation Use substApprox for F-bounds instantiation if there are wildcards. This is the simplest checking algorithm I could find that is not obviously wrong and that accepts i6146.scala. I can't prove it's correct, though. --- compiler/src/dotty/tools/dotc/typer/Checking.scala | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 96bf79df302e..3d14149b5933 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -75,8 +75,12 @@ object Checking { }.getOrElse(TypeTree(tparam.paramRef)) val orderedArgs = if (hasNamedArg(args)) tparams.map(argNamed) else args val bounds = tparams.map(_.paramInfoAsSeenFrom(tree.tpe).bounds) - def instantiate(bound: Type, args: List[Type]) = - HKTypeLambda.fromParams(tparams, bound).appliedTo(args) + def instantiate(bound: Type, args: List[Type]) = tparams match { + case (_: Symbol) :: _ if args.exists(_.isInstanceOf[TypeBounds]) => + bound.substApprox(tparams.asInstanceOf[List[Symbol]], args) + case _ => + HKTypeLambda.fromParams(tparams, bound).appliedTo(args) + } if (boundsCheck) checkBounds(orderedArgs, bounds, instantiate) def checkWildcardApply(tp: Type): Unit = tp match { From 861e59bf51144e272de3598ca76691f1563b2c75 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 23 Mar 2019 18:25:55 +0100 Subject: [PATCH 3/5] Revert "Use substApprox for F-bounds instantiation" This reverts commit 48f7ed677d9256ee19b09377b839a5292438dd2d. --- compiler/src/dotty/tools/dotc/typer/Checking.scala | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 3d14149b5933..96bf79df302e 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -75,12 +75,8 @@ object Checking { }.getOrElse(TypeTree(tparam.paramRef)) val orderedArgs = if (hasNamedArg(args)) tparams.map(argNamed) else args val bounds = tparams.map(_.paramInfoAsSeenFrom(tree.tpe).bounds) - def instantiate(bound: Type, args: List[Type]) = tparams match { - case (_: Symbol) :: _ if args.exists(_.isInstanceOf[TypeBounds]) => - bound.substApprox(tparams.asInstanceOf[List[Symbol]], args) - case _ => - HKTypeLambda.fromParams(tparams, bound).appliedTo(args) - } + def instantiate(bound: Type, args: List[Type]) = + HKTypeLambda.fromParams(tparams, bound).appliedTo(args) if (boundsCheck) checkBounds(orderedArgs, bounds, instantiate) def checkWildcardApply(tp: Type): Unit = tp match { From 841975be0bede67b04f0599243023f9cd858f614 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 23 Mar 2019 18:38:05 +0100 Subject: [PATCH 4/5] Revised scheme Revised scheme for checking applied types containing wildcards in F-bounds. --- .../src/dotty/tools/dotc/core/TypeOps.scala | 88 +++++++++++++++++-- .../src/dotty/tools/dotc/typer/Checking.scala | 9 +- 2 files changed, 87 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeOps.scala b/compiler/src/dotty/tools/dotc/core/TypeOps.scala index c7983eb8a26a..fdb16c61d99f 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeOps.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeOps.scala @@ -257,18 +257,94 @@ 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 `#` where `` 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 => + 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) - val loBound = instantiate(bounds.lo, argTypes) - 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 + } + 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) diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 96bf79df302e..157fa39f98ec 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -36,15 +36,16 @@ 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) @@ -52,7 +53,7 @@ object Checking { /** 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, _)) @@ -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, _) => From 8f57adec9bcd7ebd24f8af9d29b0d86474f8bcb4 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 23 Mar 2019 19:42:25 +0100 Subject: [PATCH 5/5] Turn off F-bound wildcard skolemizatiuon under Scala-2 mode With skolemization we fail compiling the standard lib with: ``` [E057] Type Mismatch Error: /Users/odersky/workspace/dotty/tests/scala2-library/src/library/scala/collection/generic/ParMapFactory.scala:28:76 28 |abstract class ParMapFactory[CC[X, Y] <: ParMap[X, Y] with ParMapLike[X, Y, CC[X, Y], _]] | ^ |Type argument CC[X, Y] does not conform to upper bound scala.collection.parallel.ParMapLike[X, Y, LazyRef(CC[X, Y]), | collection.parallel.ParMapLike[X, Y, CC[X, Y], _]#Sequential |] & scala.collection.parallel.ParMap[X, Y] -- ``` I have no idea whether this bound is correct or not, and am not inclined to find out. But we clearly have to support it anyway under Scala-2 mode. --- compiler/src/dotty/tools/dotc/core/TypeOps.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeOps.scala b/compiler/src/dotty/tools/dotc/core/TypeOps.scala index fdb16c61d99f..3d5482701ce8 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeOps.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeOps.scala @@ -271,7 +271,7 @@ trait TypeOps { this: Context => // TODO: Make standalone object. * type parameter corresponding to the wildcard. */ def skolemizeWildcardArgs(tps: List[Type], app: Type) = app match { - case AppliedType(tycon, args) if tycon.typeSymbol.isClass => + case AppliedType(tycon, args) if tycon.typeSymbol.isClass && !scala2Mode => tps.zipWithConserve(tycon.typeSymbol.typeParams) { (tp, tparam) => tp match { case _: TypeBounds => app.select(tparam) @@ -321,6 +321,8 @@ trait TypeOps { this: Context => // TODO: Make standalone object. 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 = {