From c36869d83d20749b7a26bb6c79f1e1b2907deb83 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 30 Dec 2015 16:06:34 +0100 Subject: [PATCH 1/4] Keep rhs of TypeDef in typed tree This is important for IDEs who want to see the full tree. The tree now gets replaced by a TypeTree in PostTyper. --- src/dotty/tools/dotc/transform/PostTyper.scala | 9 +++++++++ src/dotty/tools/dotc/typer/Typer.scala | 3 +-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/transform/PostTyper.scala b/src/dotty/tools/dotc/transform/PostTyper.scala index 3266d3a02bc5..edf97f5b8a20 100644 --- a/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/src/dotty/tools/dotc/transform/PostTyper.scala @@ -180,6 +180,15 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran case tree: DefDef => transformAnnots(tree) superAcc.wrapDefDef(tree)(super.transform(tree).asInstanceOf[DefDef]) + case tree: TypeDef => + transformAnnots(tree) + val sym = tree.symbol + val tree1 = + if (sym.isClass) tree + else { + cpy.TypeDef(tree)(rhs = TypeTree(tree.symbol.info)) + } + super.transform(tree1) case tree: MemberDef => transformAnnots(tree) super.transform(tree) diff --git a/src/dotty/tools/dotc/typer/Typer.scala b/src/dotty/tools/dotc/typer/Typer.scala index cb3bb0f205e9..43a89e59496e 100644 --- a/src/dotty/tools/dotc/typer/Typer.scala +++ b/src/dotty/tools/dotc/typer/Typer.scala @@ -961,8 +961,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit val TypeDef(name, rhs) = tdef checkLowerNotHK(sym, tdef.tparams.map(symbolOfTree), tdef.pos) completeAnnotations(tdef, sym) - val _ = typedType(rhs) // unused, typecheck only to remove from typedTree - assignType(cpy.TypeDef(tdef)(name, TypeTree(sym.info), Nil), sym) + assignType(cpy.TypeDef(tdef)(name, typedType(rhs), Nil), sym) } def typedClassDef(cdef: untpd.TypeDef, cls: ClassSymbol)(implicit ctx: Context) = track("typedClassDef") { From cc3b5309aed2d384cb2bc53271b774989b2617c1 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 30 Dec 2015 16:10:13 +0100 Subject: [PATCH 2/4] Check bounds everywhere Previously, bounds of a TypeDef tree were not checked. We now make sure bounds are checked everywhere in PostTyper. The previous partial check in Applications gets removed (it was not complete even for TypeApplications because sometimes bounds were not yet known when the test was performed.) --- .../tools/dotc/transform/PostTyper.scala | 51 +++++++++---------- src/dotty/tools/dotc/typer/Applications.scala | 1 - src/dotty/tools/dotc/typer/Checking.scala | 12 +++++ 3 files changed, 35 insertions(+), 29 deletions(-) diff --git a/src/dotty/tools/dotc/transform/PostTyper.scala b/src/dotty/tools/dotc/transform/PostTyper.scala index edf97f5b8a20..f9862bb95115 100644 --- a/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/src/dotty/tools/dotc/transform/PostTyper.scala @@ -68,7 +68,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran // TODO fill in } - /** Check bounds of AppliedTypeTrees and TypeApplys. + /** Check bounds of AppliedTypeTrees. * Replace type trees with TypeTree nodes. * Replace constant expressions with Literal nodes. * Note: Demanding idempotency instead of purity in literalize is strictly speaking too loose. @@ -97,29 +97,17 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran * Revisit this issue once we have implemented `inline`. Then we can demand * purity of the prefix unless the selection goes to an inline val. */ - private def normalizeTree(tree: Tree)(implicit ctx: Context): Tree = { - def literalize(tp: Type): Tree = tp.widenTermRefExpr match { - case ConstantType(value) if isIdempotentExpr(tree) => Literal(value) - case _ => tree - } - def norm(tree: Tree) = - if (tree.isType) TypeTree(tree.tpe).withPos(tree.pos) - else literalize(tree.tpe) - tree match { - case tree: TypeTree => - tree - case AppliedTypeTree(tycon, args) => - val tparams = tycon.tpe.typeSymbol.typeParams - val bounds = tparams.map(tparam => - tparam.info.asSeenFrom(tycon.tpe.normalizedPrefix, tparam.owner.owner).bounds) - Checking.checkBounds(args, bounds, _.substDealias(tparams, _)) - norm(tree) - case TypeApply(fn, args) => - Checking.checkBounds(args, fn.tpe.widen.asInstanceOf[PolyType]) - norm(tree) - case _ => - norm(tree) - } + private def normalizeTree(tree: Tree)(implicit ctx: Context): Tree = tree match { + case tree: TypeTree => tree + case _ => + if (tree.isType) { + Checking.boundsChecker.traverse(tree) + TypeTree(tree.tpe).withPos(tree.pos) + } + else tree.tpe.widenTermRefExpr match { + case ConstantType(value) if isIdempotentExpr(tree) => Literal(value) + case _ => tree + } } class PostTyperTransformer extends Transformer { @@ -161,10 +149,16 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran } case tree: Select => transformSelect(paramFwd.adaptRef(tree), Nil) - case tree @ TypeApply(sel: Select, args) => - val args1 = transform(args) - val sel1 = transformSelect(sel, args1) - if (superAcc.isProtectedAccessor(sel1)) sel1 else cpy.TypeApply(tree)(sel1, args1) + case tree @ TypeApply(fn, args) => + Checking.checkBounds(args, fn.tpe.widen.asInstanceOf[PolyType]) + fn match { + case sel: Select => + val args1 = transform(args) + val sel1 = transformSelect(sel, args1) + if (superAcc.isProtectedAccessor(sel1)) sel1 else cpy.TypeApply(tree)(sel1, args1) + case _ => + super.transform(tree) + } case tree @ Assign(sel: Select, _) => superAcc.transformAssign(super.transform(tree)) case tree: Template => @@ -186,6 +180,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran val tree1 = if (sym.isClass) tree else { + Checking.boundsChecker.traverse(tree.rhs) cpy.TypeDef(tree)(rhs = TypeTree(tree.symbol.info)) } super.transform(tree1) diff --git a/src/dotty/tools/dotc/typer/Applications.scala b/src/dotty/tools/dotc/typer/Applications.scala index b3cda20b822d..649b8088fe8d 100644 --- a/src/dotty/tools/dotc/typer/Applications.scala +++ b/src/dotty/tools/dotc/typer/Applications.scala @@ -607,7 +607,6 @@ trait Applications extends Compatibility { self: Typer => case pt: PolyType => if (typedArgs.length <= pt.paramBounds.length) typedArgs = typedArgs.zipWithConserve(pt.paramBounds)(adaptTypeArg) - Checking.checkBounds(typedArgs, pt) case _ => } assignType(cpy.TypeApply(tree)(typedFn, typedArgs), typedFn, typedArgs) diff --git a/src/dotty/tools/dotc/typer/Checking.scala b/src/dotty/tools/dotc/typer/Checking.scala index 57032c4d9ecb..5635c12a2b42 100644 --- a/src/dotty/tools/dotc/typer/Checking.scala +++ b/src/dotty/tools/dotc/typer/Checking.scala @@ -48,6 +48,18 @@ object Checking { def checkBounds(args: List[tpd.Tree], poly: PolyType)(implicit ctx: Context): Unit = checkBounds(args, poly.paramBounds, _.substParams(poly, _)) + /** Check all AppliedTypeTree nodes in this tree for legal bounds */ + val boundsChecker = new TreeTraverser { + def traverse(tree: Tree)(implicit ctx: Context) = tree match { + case AppliedTypeTree(tycon, args) => + val tparams = tycon.tpe.typeSymbol.typeParams + val bounds = tparams.map(tparam => + tparam.info.asSeenFrom(tycon.tpe.normalizedPrefix, tparam.owner.owner).bounds) + checkBounds(args, bounds, _.substDealias(tparams, _)) + case _ => traverseChildren(tree) + } + } + /** Check that `tp` refers to a nonAbstract class * and that the instance conforms to the self type of the created class. */ From 124227143e1bfecd2385df3a446222e3487acf31 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 30 Dec 2015 16:11:50 +0100 Subject: [PATCH 3/4] Avoid caching the wrong bounds in TypeRefs Checking bounds everywhere revealed a problem in compileStdLib, which this commit fixes. --- src/dotty/tools/dotc/typer/Namer.scala | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/typer/Namer.scala b/src/dotty/tools/dotc/typer/Namer.scala index b24916be8005..5ed388f4cd27 100644 --- a/src/dotty/tools/dotc/typer/Namer.scala +++ b/src/dotty/tools/dotc/typer/Namer.scala @@ -850,7 +850,9 @@ class Namer { typer: Typer => if (tparamSyms.nonEmpty && !isDerived) tp.LambdaAbstract(tparamSyms) //else if (toParameterize) tp.parameterizeWith(tparamSyms) else tp - sym.info = abstracted(TypeBounds.empty) + + val dummyInfo = abstracted(TypeBounds.empty) + sym.info = dummyInfo // Temporarily set info of defined type T to ` >: Nothing <: Any. // This is done to avoid cyclic reference errors for F-bounds. // This is subtle: `sym` has now an empty TypeBounds, but is not automatically @@ -871,6 +873,19 @@ class Namer { typer: Typer => sym.info = NoCompleter sym.info = checkNonCyclic(sym, unsafeInfo, reportErrors = true) } + + // Here we pay the price for the cavalier setting info to TypeBounds.empty above. + // We need to compensate by invalidating caches in references that might + // still contain the TypeBounds.empty. If we do not do this, stdlib factories + // fail with a bounds error in PostTyper. + def ensureUpToDate(tp: Type, outdated: Type) = tp match { + case tref: TypeRef if tref.info == outdated && sym.info != outdated => + tref.uncheckedSetSym(null) + case _ => + } + ensureUpToDate(sym.typeRef, dummyInfo) + ensureUpToDate(sym.typeRef.appliedTo(tparamSyms.map(_.typeRef)), TypeBounds.empty) + etaExpandArgs.apply(sym.info) } From cc6af2609b72f5276b1abb6297d911ca766bf8d3 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 30 Dec 2015 16:12:38 +0100 Subject: [PATCH 4/4] Adapt and add tests New test that exhibited the problem is ski.scala. Previously this did not fail with a bounds violation. --- test/dotc/tests.scala | 7 +- tests/neg/bounds.scala | 4 ++ tests/neg/ski.scala | 136 +++++++++++++++++++++++++++++++++++++ tests/neg/typedapply.scala | 10 +-- 4 files changed, 147 insertions(+), 10 deletions(-) create mode 100644 tests/neg/ski.scala diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index db5fa85446b5..b4cac4a7628f 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -107,8 +107,8 @@ class tests extends CompilerTest { @Test def neg_abstractOverride() = compileFile(negDir, "abstract-override", xerrors = 2) @Test def neg_blockescapes() = compileFile(negDir, "blockescapesNeg", xerrors = 1) - @Test def neg_bounds() = compileFile(negDir, "bounds", xerrors = 2) - @Test def neg_typedapply() = compileFile(negDir, "typedapply", xerrors = 4) + @Test def neg_bounds() = compileFile(negDir, "bounds", xerrors = 3) + @Test def neg_typedapply() = compileFile(negDir, "typedapply", xerrors = 3) @Test def neg_typedIdents() = compileDir(negDir, "typedIdents", xerrors = 2) @Test def neg_assignments() = compileFile(negDir, "assignments", xerrors = 3) @Test def neg_typers() = compileFile(negDir, "typers", xerrors = 14)(allowDoubleBindings) @@ -165,6 +165,7 @@ class tests extends CompilerTest { @Test def neg_selfreq = compileFile(negDir, "selfreq", xerrors = 2) @Test def neg_singletons = compileFile(negDir, "singletons", xerrors = 8) @Test def neg_shadowedImplicits = compileFile(negDir, "arrayclone-new", xerrors = 2) + @Test def neg_ski = compileFile(negDir, "ski", xerrors = 2) @Test def neg_traitParamsTyper = compileFile(negDir, "traitParamsTyper", xerrors = 5) @Test def neg_traitParamsMixin = compileFile(negDir, "traitParamsMixin", xerrors = 2) @Test def neg_firstError = compileFile(negDir, "firstError", xerrors = 3) @@ -182,7 +183,7 @@ class tests extends CompilerTest { .filter(_.nonEmpty) .toList - @Test def compileStdLib = compileList("compileStdLib", stdlibFiles, "-migration" :: scala2mode) + @Test def compileStdLib = compileList("compileStdLib", stdlibFiles, "-migration" :: scala2mode)(allowDeepSubtypes) @Test def dotty = compileDir(dottyDir, ".", List("-deep", "-Ycheck-reentrant"))(allowDeepSubtypes) // note the -deep argument @Test def dotc_ast = compileDir(dotcDir, "ast") diff --git a/tests/neg/bounds.scala b/tests/neg/bounds.scala index 8d2cd825995f..55eec6941b10 100644 --- a/tests/neg/bounds.scala +++ b/tests/neg/bounds.scala @@ -5,4 +5,8 @@ object Test { g("foo") new C("bar") } + def baz[X >: Y, Y <: String](x: X, y: Y) = (x, y) + + baz[Int, String](1, "abc") + } diff --git a/tests/neg/ski.scala b/tests/neg/ski.scala new file mode 100644 index 000000000000..6510e66aeefe --- /dev/null +++ b/tests/neg/ski.scala @@ -0,0 +1,136 @@ +trait Term { + type ap[x <: Term] <: Term + type eval <: Term +} + +// The S combinator +trait S extends Term { + type ap[x <: Term] = S1[x] + type eval = S +} +trait S1[x <: Term] extends Term { + type ap[y <: Term] = S2[x, y] + type eval = S1[x] +} +trait S2[x <: Term, y <: Term] extends Term { + type ap[z <: Term] = S3[x, y, z] + type eval = S2[x, y] +} +trait S3[x <: Term, y <: Term, z <: Term] extends Term { + type ap[v <: Term] = eval#ap[v] + type eval = x#ap[z]#ap[y#ap[z]]#eval +} + +// The K combinator +trait K extends Term { + type ap[x <: Term] = K1[x] + type eval = K +} +trait K1[x <: Term] extends Term { + type ap[y <: Term] = K2[x, y] + type eval = K1[x] +} +trait K2[x <: Term, y <: Term] extends Term { + type ap[z <: Term] = eval#ap[z] + type eval = x#eval +} + +// The I combinator +trait I extends Term { + type ap[x <: Term] = I1[x] + type eval = I +} +trait I1[x <: Term] extends Term { + type ap[y <: Term] = eval#ap[y] + type eval = x#eval +} + +// Constants + +trait c extends Term { + type ap[x <: Term] = c + type eval = c +} +trait d extends Term { + type ap[x <: Term] = d + type eval = d +} +trait e extends Term { + type ap[x <: Term] = e + type eval = e +} + +case class Equals[A >: B <:B , B]() + +object Test { + type T1 = Equals[Int, Int] // compiles fine + type T2 = Equals[String, Int] // error + type T3 = Equals[I#ap[c]#eval, c] + type T3a = Equals[I#ap[c]#eval, d]// error + + // Ic -> c + type T4 = Equals[I#ap[c]#eval, c] + + // Kcd -> c + type T5 = Equals[K#ap[c]#ap[d]#eval, c] + + // KKcde -> d + type T6 = Equals[K#ap[K]#ap[c]#ap[d]#ap[e]#eval, d] + + // SIIIc -> Ic + type T7 = Equals[S#ap[I]#ap[I]#ap[I]#ap[c]#eval, c] + + // SKKc -> Ic + type T8 = Equals[S#ap[K]#ap[K]#ap[c]#eval, c] + + // SIIKc -> KKc + type T9 = Equals[S#ap[I]#ap[I]#ap[K]#ap[c]#eval, K#ap[K]#ap[c]#eval] + + // SIKKc -> K(KK)c + type T10 = Equals[S#ap[I]#ap[K]#ap[K]#ap[c]#eval, K#ap[K#ap[K]]#ap[c]#eval] + + // SIKIc -> KIc + type T11 = Equals[S#ap[I]#ap[K]#ap[I]#ap[c]#eval, K#ap[I]#ap[c]#eval] + + // SKIc -> Ic + type T12 = Equals[S#ap[K]#ap[I]#ap[c]#eval, c] + + // R = S(K(SI))K (reverse) + type R = S#ap[K#ap[S#ap[I]]]#ap[K] + type T13 = Equals[R#ap[c]#ap[d]#eval, d#ap[c]#eval] + + type b[a <: Term] = S#ap[K#ap[a]]#ap[S#ap[I]#ap[I]] + + trait A0 extends Term { + type ap[x <: Term] = c + type eval = A0 + } + trait A1 extends Term { + type ap[x <: Term] = x#ap[A0]#eval + type eval = A1 + } + trait A2 extends Term { + type ap[x <: Term] = x#ap[A1]#eval + type eval = A2 + } + + type NN1 = b[R]#ap[b[R]]#ap[A0] + type T13a = Equals[NN1#eval, c] + + // Double iteration + type NN2 = b[R]#ap[b[R]]#ap[A1] + type T14 = Equals[NN2#eval, c] + + // Triple iteration + type NN3 = b[R]#ap[b[R]]#ap[A2] + type T15 = Equals[NN3#eval, c] + + trait An extends Term { + type ap[x <: Term] = x#ap[An]#eval + type eval = An + } + +// Infinite iteration: Smashes scalac's stack + type NNn = b[R]#ap[b[R]]#ap[An] + // type X = Equals[NNn#eval, c] +} diff --git a/tests/neg/typedapply.scala b/tests/neg/typedapply.scala index b80281c9ff67..706b05da3592 100644 --- a/tests/neg/typedapply.scala +++ b/tests/neg/typedapply.scala @@ -2,16 +2,12 @@ object typedapply { def foo[X, Y](x: X, y: Y) = (x, y) - foo[Int](1, "abc") + foo[Int](1, "abc") // error: wrong number of type parameters - foo[Int, String, String](1, "abc") + foo[Int, String, String](1, "abc") // error: wrong number of type parameters def bar(x: Int) = x - bar[Int](1) - - def baz[X >: Y, Y <: String](x: X, y: Y) = (x, y) - - baz[Int, String](1, "abc") + bar[Int](1) // error: does not take parameters }