From ac226f26d8f54c79c642ed88bc5c48916afeb61b Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 4 Aug 2015 18:34:10 -0700 Subject: [PATCH 01/15] Implement non-local returns Non-local returns are now implemented. --- src/dotty/tools/dotc/Compiler.scala | 1 + src/dotty/tools/dotc/ast/tpd.scala | 4 + src/dotty/tools/dotc/core/Definitions.scala | 1 + .../dotc/transform/NonLocalReturns.scala | 88 +++++++++++++++++++ tests/run/nonLocalReturns.scala | 32 +++++++ 5 files changed, 126 insertions(+) create mode 100644 src/dotty/tools/dotc/transform/NonLocalReturns.scala create mode 100644 tests/run/nonLocalReturns.scala diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index 76cf10428fe5..1742fb1b8c5d 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -68,6 +68,7 @@ class Compiler { new LazyVals, new Memoize, new LinkScala2ImplClasses, + new NonLocalReturns, new CapturedVars, // capturedVars has a transformUnit: no phases should introduce local mutable vars here new Constructors, new FunctionalInterfaces, diff --git a/src/dotty/tools/dotc/ast/tpd.scala b/src/dotty/tools/dotc/ast/tpd.scala index b05d23107755..4e9940ac4b52 100644 --- a/src/dotty/tools/dotc/ast/tpd.scala +++ b/src/dotty/tools/dotc/ast/tpd.scala @@ -161,6 +161,10 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { def Bind(sym: TermSymbol, body: Tree)(implicit ctx: Context): Bind = ta.assignType(untpd.Bind(sym.name, body), sym) + /** A pattern corrsponding to `sym: tpe` */ + def BindTyped(sym: TermSymbol, tpe: Type)(implicit ctx: Context): Bind = + Bind(sym, Typed(Underscore(tpe), TypeTree(tpe))) + def Alternative(trees: List[Tree])(implicit ctx: Context): Alternative = ta.assignType(untpd.Alternative(trees), trees) diff --git a/src/dotty/tools/dotc/core/Definitions.scala b/src/dotty/tools/dotc/core/Definitions.scala index 7ed0a26e018c..502b42987982 100644 --- a/src/dotty/tools/dotc/core/Definitions.scala +++ b/src/dotty/tools/dotc/core/Definitions.scala @@ -326,6 +326,7 @@ class Definitions { lazy val Product_productArity = ProductClass.requiredMethod(nme.productArity) lazy val Product_productPrefix = ProductClass.requiredMethod(nme.productPrefix) lazy val LanguageModuleClass = ctx.requiredModule("dotty.language").moduleClass.asClass + lazy val NonLocalReturnControlClass = ctx.requiredClass("scala.runtime.NonLocalReturnControl") // Annotation base classes lazy val AnnotationClass = ctx.requiredClass("scala.annotation.Annotation") diff --git a/src/dotty/tools/dotc/transform/NonLocalReturns.scala b/src/dotty/tools/dotc/transform/NonLocalReturns.scala new file mode 100644 index 000000000000..d0a2f2ca78bf --- /dev/null +++ b/src/dotty/tools/dotc/transform/NonLocalReturns.scala @@ -0,0 +1,88 @@ +package dotty.tools.dotc +package transform + +import core._ +import Contexts._, Symbols._, Types._, Flags._, Decorators._, StdNames._, Constants._, Phases._ +import TreeTransforms._ +import ast.Trees._ +import collection.mutable + +/** Implement non-local returns using NonLocalReturnControl exceptions. + */ +class NonLocalReturns extends MiniPhaseTransform { thisTransformer => + override def phaseName = "nonLocalReturns" + + import ast.tpd._ + + override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[ElimByName]) + + private def ensureConforms(tree: Tree, pt: Type)(implicit ctx: Context) = + if (tree.tpe <:< pt) tree + else Erasure.Boxing.adaptToType(tree, pt) + + /** The type of a non-local return expression with given argument type */ + private def nonLocalReturnExceptionType(argtype: Type)(implicit ctx: Context) = + defn.NonLocalReturnControlClass.typeRef.appliedTo(argtype) + + /** A hashmap from method symbols to non-local return keys */ + private val nonLocalReturnKeys = mutable.Map[Symbol, TermSymbol]() + + /** Return non-local return key for given method */ + private def nonLocalReturnKey(meth: Symbol)(implicit ctx: Context) = + nonLocalReturnKeys.getOrElseUpdate(meth, + ctx.newSymbol( + meth, ctx.freshName("nonLocalReturnKey").toTermName, Synthetic, defn.ObjectType, coord = meth.pos)) + + /** Generate a non-local return throw with given return expression from given method. + * I.e. for the method's non-local return key, generate: + * + * throw new NonLocalReturnControl(key, expr) + * todo: maybe clone a pre-existing exception instead? + * (but what to do about exceptions that miss their targets?) + */ + private def nonLocalReturnThrow(expr: Tree, meth: Symbol)(implicit ctx: Context) = + Throw( + New( + defn.NonLocalReturnControlClass.typeRef, + ref(nonLocalReturnKey(meth)) :: ensureConforms(expr, defn.ObjectType) :: Nil)) + + /** Transform (body, key) to: + * + * { + * val key = new Object() + * try { + * body + * } catch { + * case ex: NonLocalReturnControl => + * if (ex.key().eq(key)) ex.value().asInstanceOf[T] + * else throw ex + * } + * } + */ + private def nonLocalReturnTry(body: Tree, key: TermSymbol, meth: Symbol)(implicit ctx: Context) = { + val keyDef = ValDef(key, New(defn.ObjectType, Nil)) + val nonLocalReturnControl = defn.NonLocalReturnControlClass.typeRef + val ex = ctx.newSymbol(meth, nme.ex, EmptyFlags, nonLocalReturnControl, coord = body.pos) + val pat = BindTyped(ex, nonLocalReturnControl) + val rhs = If( + ref(ex).select(nme.key).appliedToNone.select(nme.eq).appliedTo(ref(key)), + ensureConforms(ref(ex).select(nme.value), meth.info.finalResultType), + Throw(ref(ex))) + val catches = CaseDef(pat, EmptyTree, rhs) :: Nil + val tryCatch = Try(body, catches, EmptyTree) + Block(keyDef :: Nil, tryCatch) + } + + def isNonLocalReturn(ret: Return)(implicit ctx: Context) = + ret.from.symbol != ctx.owner.enclosingMethod || ctx.owner.is(Lazy) // Lazy needed? + + override def transformDefDef(tree: DefDef)(implicit ctx: Context, info: TransformerInfo): Tree = + nonLocalReturnKeys.remove(tree.symbol) match { + case Some(key) => cpy.DefDef(tree)(rhs = nonLocalReturnTry(tree.rhs, key, tree.symbol)) + case _ => tree + } + + override def transformReturn(tree: Return)(implicit ctx: Context, info: TransformerInfo): Tree = + if (isNonLocalReturn(tree)) nonLocalReturnThrow(tree.expr, tree.from.symbol).withPos(tree.pos) + else tree +} diff --git a/tests/run/nonLocalReturns.scala b/tests/run/nonLocalReturns.scala new file mode 100644 index 000000000000..a425ac723e2b --- /dev/null +++ b/tests/run/nonLocalReturns.scala @@ -0,0 +1,32 @@ +object Test { + + def foo(xs: List[Int]): Int = { + xs.foreach(x => return x) + 0 + } + + def bar(xs: List[Int]): Int = { + lazy val y = if (xs.isEmpty) return -1 else xs.head + y + } + + def baz(x: Int): Int = + byName { return -2; 3 } + + def byName(x: => Int): Int = x + + def bam(): Int = { // no non-local return needed here + val foo = { + return -3 + 3 + } + foo + } + + def main(args: Array[String]) = { + assert(foo(List(1, 2, 3)) == 1) + assert(bar(Nil) == -1) + assert(baz(3) == -2) + assert(bam() == -3) + } +} From c8afd79b4c7f145ba090a2d936d627c3ab35b1c2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 4 Aug 2015 18:34:58 -0700 Subject: [PATCH 02/15] Disable -Ycheck:labelDefs Disable the check because if fails for desugar.scala and also in some dotty files. This test failed before the addition of NonLocalReturns. --- test/dotc/tests.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index 0284a8714326..c5d93b861127 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -24,7 +24,7 @@ class tests extends CompilerTest { "-Yno-deep-subtypes", "-Yno-double-bindings", "-d", defaultOutputDir) ++ { if (isRunByJenkins) List("-Ycheck:-Ycheck:tailrec,resolveSuper,mixin,restoreScopes,labelDef") // should be Ycheck:all, but #725 - else List("-Ycheck:tailrec,resolveSuper,mixin,restoreScopes,labelDef") + else List("-Ycheck:tailrec,resolveSuper,mixin,restoreScopes") } From 2a306ddcfe78589310d462bbf67fb893ce8702aa Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 5 Aug 2015 12:26:07 -0700 Subject: [PATCH 03/15] Make ensureConforms behave gracefully fter erasure After erasure, if required conformance is between value and non-value types, one should perform boxing and unboxing operations automatically, instead of just issuing a cast, which would be illegal at that point. Also: make isNonLocalReturn available as part of a global object, because we'll need it in LiftTry. --- src/dotty/tools/dotc/ast/tpd.scala | 12 +++++++++--- .../tools/dotc/transform/NonLocalReturns.scala | 14 +++++++++----- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/dotty/tools/dotc/ast/tpd.scala b/src/dotty/tools/dotc/ast/tpd.scala index 4e9940ac4b52..989cae2168f8 100644 --- a/src/dotty/tools/dotc/ast/tpd.scala +++ b/src/dotty/tools/dotc/ast/tpd.scala @@ -13,6 +13,7 @@ import config.Printers._ import typer.Mode import collection.mutable import typer.ErrorReporting._ +import transform.Erasure import scala.annotation.tailrec @@ -161,7 +162,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { def Bind(sym: TermSymbol, body: Tree)(implicit ctx: Context): Bind = ta.assignType(untpd.Bind(sym.name, body), sym) - /** A pattern corrsponding to `sym: tpe` */ + /** A pattern corresponding to `sym: tpe` */ def BindTyped(sym: TermSymbol, tpe: Type)(implicit ctx: Context): Bind = Bind(sym, Typed(Underscore(tpe), TypeTree(tpe))) @@ -737,9 +738,14 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { tree.select(defn.Any_asInstanceOf).appliedToType(tp) } - /** `tree.asInstanceOf[tp]` unless tree's type already conforms to `tp` */ + /** `tree.asInstanceOf[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 asInstance(tp) + if (tree.tpe <:< tp) tree + else if (!ctx.erasedTypes) asInstance(tp) + else Erasure.Boxing.adaptToType(tree, tp) /** If inititializer tree is `_', the default value of its type, * otherwise the tree itself. diff --git a/src/dotty/tools/dotc/transform/NonLocalReturns.scala b/src/dotty/tools/dotc/transform/NonLocalReturns.scala index d0a2f2ca78bf..1f18a7318d24 100644 --- a/src/dotty/tools/dotc/transform/NonLocalReturns.scala +++ b/src/dotty/tools/dotc/transform/NonLocalReturns.scala @@ -7,11 +7,18 @@ import TreeTransforms._ import ast.Trees._ import collection.mutable +object NonLocalReturns { + import ast.tpd._ + def isNonLocalReturn(ret: Return)(implicit ctx: Context) = + ret.from.symbol != ctx.owner.enclosingMethod || ctx.owner.is(Lazy) +} + /** Implement non-local returns using NonLocalReturnControl exceptions. */ class NonLocalReturns extends MiniPhaseTransform { thisTransformer => override def phaseName = "nonLocalReturns" + import NonLocalReturns._ import ast.tpd._ override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[ElimByName]) @@ -44,7 +51,7 @@ class NonLocalReturns extends MiniPhaseTransform { thisTransformer => Throw( New( defn.NonLocalReturnControlClass.typeRef, - ref(nonLocalReturnKey(meth)) :: ensureConforms(expr, defn.ObjectType) :: Nil)) + ref(nonLocalReturnKey(meth)) :: expr.ensureConforms(defn.ObjectType) :: Nil)) /** Transform (body, key) to: * @@ -66,16 +73,13 @@ class NonLocalReturns extends MiniPhaseTransform { thisTransformer => val pat = BindTyped(ex, nonLocalReturnControl) val rhs = If( ref(ex).select(nme.key).appliedToNone.select(nme.eq).appliedTo(ref(key)), - ensureConforms(ref(ex).select(nme.value), meth.info.finalResultType), + ref(ex).select(nme.value).ensureConforms(meth.info.finalResultType), Throw(ref(ex))) val catches = CaseDef(pat, EmptyTree, rhs) :: Nil val tryCatch = Try(body, catches, EmptyTree) Block(keyDef :: Nil, tryCatch) } - def isNonLocalReturn(ret: Return)(implicit ctx: Context) = - ret.from.symbol != ctx.owner.enclosingMethod || ctx.owner.is(Lazy) // Lazy needed? - override def transformDefDef(tree: DefDef)(implicit ctx: Context, info: TransformerInfo): Tree = nonLocalReturnKeys.remove(tree.symbol) match { case Some(key) => cpy.DefDef(tree)(rhs = nonLocalReturnTry(tree.rhs, key, tree.symbol)) From 3f46a7506454f01f1b3d79b60d1b48845bde8d4a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 5 Aug 2015 12:35:34 -0700 Subject: [PATCH 04/15] Add LiftTry phase Phase lifts tries that would be illegal because they execute on non-empty expression stacks. --- src/dotty/tools/dotc/Compiler.scala | 1 + src/dotty/tools/dotc/transform/LiftTry.scala | 66 ++++++++++++++++++++ test/dotc/tests.scala | 2 +- tests/run/StackMap.scala | 7 +++ tests/run/liftedTry.scala | 21 +++++++ 5 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 src/dotty/tools/dotc/transform/LiftTry.scala create mode 100644 tests/run/StackMap.scala create mode 100644 tests/run/liftedTry.scala diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index 1742fb1b8c5d..f753b7614a45 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -48,6 +48,7 @@ class Compiler { new ExtensionMethods, new ExpandSAMs, new TailRec, + new LiftTry, new ClassOf), List(new PatternMatcher, new ExplicitOuter, diff --git a/src/dotty/tools/dotc/transform/LiftTry.scala b/src/dotty/tools/dotc/transform/LiftTry.scala new file mode 100644 index 000000000000..6a273b91e6cb --- /dev/null +++ b/src/dotty/tools/dotc/transform/LiftTry.scala @@ -0,0 +1,66 @@ +package dotty.tools.dotc +package transform + +import TreeTransforms._ +import core.DenotTransformers._ +import core.Symbols._ +import core.Contexts._ +import core.Types._ +import core.Flags._ +import core.Decorators._ +import NonLocalReturns._ + +/** Lifts try's that might be executed on non-empty expression stacks + * to their own methods. I.e. + * + * try body catch handler + * + * is lifted to + * + * { def liftedTree$n() = try body catch handler; liftedTree$n() } + */ +class LiftTry extends MiniPhase with IdentityDenotTransformer { thisTransform => + import ast.tpd._ + + /** the following two members override abstract members in Transform */ + val phaseName: String = "liftTry" + + val treeTransform = new Transform(needLift = false) + val liftingTransform = new Transform(needLift = true) + + class Transform(needLift: Boolean) extends TreeTransform { + def phase = thisTransform + + override def prepareForApply(tree: Apply)(implicit ctx: Context) = + if (tree.fun.symbol.is(Label)) this + else liftingTransform + + override def prepareForValDef(tree: ValDef)(implicit ctx: Context) = + if (!tree.symbol.exists || + tree.symbol.isSelfSym || + tree.symbol.owner == ctx.owner.enclosingMethod) this + else liftingTransform + + override def prepareForAssign(tree: Assign)(implicit ctx: Context) = + if (tree.lhs.symbol.maybeOwner == ctx.owner.enclosingMethod) this + else liftingTransform + + override def prepareForReturn(tree: Return)(implicit ctx: Context) = + if (!isNonLocalReturn(tree)) this + else liftingTransform + + override def prepareForTemplate(tree: Template)(implicit ctx: Context) = + treeTransform + + override def transformTry(tree: Try)(implicit ctx: Context, info: TransformerInfo): Tree = + if (needLift) { + ctx.debuglog(i"lifting tree at ${tree.pos}, current owner = ${ctx.owner}") + val fn = ctx.newSymbol( + ctx.owner, ctx.freshName("liftedTree").toTermName, Synthetic | Method, + MethodType(Nil, tree.tpe), coord = tree.pos) + tree.changeOwnerAfter(ctx.owner, fn, thisTransform) + Block(DefDef(fn, tree) :: Nil, ref(fn).appliedToNone) + } + else tree + } +} diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index c5d93b861127..3c0f883ce4fd 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -115,7 +115,7 @@ class tests extends CompilerTest { @Test def neg_zoo = compileFile(negDir, "zoo", xerrors = 12) val negTailcallDir = negDir + "tailcall/" - @Test def neg_tailcall_t1672b = compileFile(negTailcallDir, "t1672b", xerrors = 6) + @Test def neg_tailcall_t1672b = compileFile(negTailcallDir, "t1672b", xerrors = 5) @Test def neg_tailcall_t3275 = compileFile(negTailcallDir, "t3275", xerrors = 1) @Test def neg_tailcall_t6574 = compileFile(negTailcallDir, "t6574", xerrors = 2) @Test def neg_tailcall = compileFile(negTailcallDir, "tailrec", xerrors = 7) diff --git a/tests/run/StackMap.scala b/tests/run/StackMap.scala new file mode 100644 index 000000000000..5220758fe923 --- /dev/null +++ b/tests/run/StackMap.scala @@ -0,0 +1,7 @@ +object Test { + var implicitsCache = null + + def main(args: Array[String]): Unit = { + implicitsCache = try{null} catch { case ex: Exception => null } + } +} diff --git a/tests/run/liftedTry.scala b/tests/run/liftedTry.scala new file mode 100644 index 000000000000..2e4c25c2b690 --- /dev/null +++ b/tests/run/liftedTry.scala @@ -0,0 +1,21 @@ +object Test { + + def raise(x: Int) = { throw new Exception(s"$x"); 0 } + def handle: Throwable => Int = { case ex: Exception => ex.getMessage().toInt } + + val x = try raise(1) catch handle + + def foo(x: Int) = { + val y = try raise(x) catch handle + y + } + + foo(try 3 catch handle) + + def main(args: Array[String]) = { + assert(x == 1) + assert(foo(2) == 2) + assert(foo(try raise(3) catch handle) == 3) + } +} + From 70ad514131f7e4e30b59027c6e3da3b763a2b8e9 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 5 Aug 2015 13:40:10 -0700 Subject: [PATCH 05/15] Revert "Workaround #742 and add a test for it." This reverts commit a43d39ad719978fbb36663f336c1c7cd2c4da1e0. --- src/dotty/tools/dotc/core/Contexts.scala | 37 ++++++++++++------------ tests/pending/run/StackMap.scala | 7 ----- 2 files changed, 18 insertions(+), 26 deletions(-) delete mode 100644 tests/pending/run/StackMap.scala diff --git a/src/dotty/tools/dotc/core/Contexts.scala b/src/dotty/tools/dotc/core/Contexts.scala index 15c7b4b115b5..206ef9d8b9d4 100644 --- a/src/dotty/tools/dotc/core/Contexts.scala +++ b/src/dotty/tools/dotc/core/Contexts.scala @@ -177,26 +177,25 @@ object Contexts { /** The new implicit references that are introduced by this scope */ private var implicitsCache: ContextualImplicits = null def implicits: ContextualImplicits = { - if (implicitsCache == null ) { - val outerImplicits = - if (isImportContext && importInfo.hiddenRoot.exists) - outer.implicits exclude importInfo.hiddenRoot - else - outer.implicits - try - implicitsCache = { - val implicitRefs: List[TermRef] = - if (isClassDefContext) owner.thisType.implicitMembers - else if (isImportContext) importInfo.importedImplicits - else if (isNonEmptyScopeContext) scope.implicitDecls - else Nil - if (implicitRefs.isEmpty) outerImplicits - else new ContextualImplicits(implicitRefs, outerImplicits)(this) - } - catch { - case ex: CyclicReference => implicitsCache = outerImplicits + if (implicitsCache == null ) + implicitsCache = { + val implicitRefs: List[TermRef] = + if (isClassDefContext) + try owner.thisType.implicitMembers + catch { + case ex: CyclicReference => Nil + } + else if (isImportContext) importInfo.importedImplicits + else if (isNonEmptyScopeContext) scope.implicitDecls + else Nil + val outerImplicits = + if (isImportContext && importInfo.hiddenRoot.exists) + outer.implicits exclude importInfo.hiddenRoot + else + outer.implicits + if (implicitRefs.isEmpty) outerImplicits + else new ContextualImplicits(implicitRefs, outerImplicits)(this) } - } implicitsCache } diff --git a/tests/pending/run/StackMap.scala b/tests/pending/run/StackMap.scala deleted file mode 100644 index 5220758fe923..000000000000 --- a/tests/pending/run/StackMap.scala +++ /dev/null @@ -1,7 +0,0 @@ -object Test { - var implicitsCache = null - - def main(args: Array[String]): Unit = { - implicitsCache = try{null} catch { case ex: Exception => null } - } -} From c4680c87d453c59f5874980984305b202c2f7b62 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 5 Aug 2015 14:26:37 -0700 Subject: [PATCH 06/15] Make sure expanded method names are legal according to JVM spec --- src/dotty/tools/dotc/core/SymDenotations.scala | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/core/SymDenotations.scala b/src/dotty/tools/dotc/core/SymDenotations.scala index 985d1a9c713d..2ce2b8d20db3 100644 --- a/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/src/dotty/tools/dotc/core/SymDenotations.scala @@ -299,7 +299,11 @@ object SymDenotations { /** The expanded name of this denotation. */ final def expandedName(implicit ctx: Context) = if (is(ExpandedName) || isConstructor) name - else name.expandedName(initial.asSymDenotation.owner) + else { + def legalize(name: Name): Name = // JVM method names may not contain `<' or `>' characters + if (is(Method)) name.replace('<', '(').replace('>', ')') else name + legalize(name.expandedName(initial.asSymDenotation.owner)) + } // need to use initial owner to disambiguate, as multiple private symbols with the same name // might have been moved from different origins into the same class From 694aabd5caa2a67721f82db4027d28815af90275 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 5 Aug 2015 14:27:05 -0700 Subject: [PATCH 07/15] Revert "Disable -Ycheck:labelDefs" This reverts commit c8afd79b4c7f145ba090a2d936d627c3ab35b1c2. --- test/dotc/tests.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index 3c0f883ce4fd..b39d0e928951 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -24,7 +24,7 @@ class tests extends CompilerTest { "-Yno-deep-subtypes", "-Yno-double-bindings", "-d", defaultOutputDir) ++ { if (isRunByJenkins) List("-Ycheck:-Ycheck:tailrec,resolveSuper,mixin,restoreScopes,labelDef") // should be Ycheck:all, but #725 - else List("-Ycheck:tailrec,resolveSuper,mixin,restoreScopes") + else List("-Ycheck:tailrec,resolveSuper,mixin,restoreScopes,labelDef") } From 2a57aabfac03086417dffc49a43c9f36450ef39a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 6 Aug 2015 13:09:07 -0700 Subject: [PATCH 08/15] Add test case --- tests/run/liftedTry.scala | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/run/liftedTry.scala b/tests/run/liftedTry.scala index 2e4c25c2b690..f10389557115 100644 --- a/tests/run/liftedTry.scala +++ b/tests/run/liftedTry.scala @@ -19,3 +19,11 @@ object Test { } } +object Tr { + def fun(a: Int => Unit) = a(2) + def foo: Int = { + var s = 1 + s = try {fun(s = _); 3} catch{ case ex: Throwable => s = 4; 5 } + s + } +} From 8e996503d2662dc4bf24453a97e702f382a55741 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 9 Aug 2015 15:13:07 -0700 Subject: [PATCH 09/15] Fixes #739 by adding the following rule: Before typing an implicit parameter list of a method m, instantiate all type parameters of m that occur in the type of some preceding value parameter of m. --- src/dotty/tools/dotc/typer/Inferencing.scala | 45 +++++++++++++++++--- src/dotty/tools/dotc/typer/Typer.scala | 2 + 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/dotty/tools/dotc/typer/Inferencing.scala b/src/dotty/tools/dotc/typer/Inferencing.scala index 8df544dd62c6..421459fd7f63 100644 --- a/src/dotty/tools/dotc/typer/Inferencing.scala +++ b/src/dotty/tools/dotc/typer/Inferencing.scala @@ -43,6 +43,11 @@ trait Inferencing { this: Checking => if (isFullyDefined(tp, ForceDegree.all)) tp else throw new Error(i"internal error: type of $what $tp is not fully defined, pos = $pos") // !!! DEBUG + + /** Instantiate selected type variables `tvars` in type `tp` */ + def instantiateSelected(tp: Type, tvars: List[Type])(implicit ctx: Context): Unit = + new IsFullyDefinedAccumulator(new ForceDegree.Value(tvars.contains)).process(tp) + /** The accumulator which forces type variables using the policy encoded in `force` * and returns whether the type is fully defined. Two phases: * 1st Phase: Try to instantiate covariant and non-variant type variables to @@ -61,8 +66,7 @@ trait Inferencing { this: Checking => case _: WildcardType | _: ProtoType => false case tvar: TypeVar if !tvar.isInstantiated => - if (force == ForceDegree.none) false - else { + force.appliesTo(tvar) && { val minimize = variance >= 0 && !( force == ForceDegree.noBottom && @@ -93,6 +97,33 @@ trait Inferencing { this: Checking => } } + /** If `tree`'s type is of the form + * + * e [T1, ..., Tn] (ps1)...(psn) + * + * the list of uninstantiated type variables matching one of `T1`, ..., `Tn` + * which also appear in one of the parameter sections `ps1`, ..., `psn`, otherwise Nil. + */ + def tvarsInParams(tree: Tree)(implicit ctx: Context): List[TypeVar] = { + def occursInParam(mtp: Type, tvar: TypeVar, secCount: Int): Boolean = mtp match { + case mtp: MethodType => + secCount > 0 && ( + mtp.paramTypes.exists(tvar.occursIn) || + occursInParam(mtp.resultType, tvar, secCount - 1)) + case _ => false + } + def collect(tree: Tree, secCount: Int): List[TypeVar] = tree match { + case Apply(fn, _) => collect(fn, secCount + 1) + case TypeApply(_, targs) => + targs.tpes.collect { + case tvar: TypeVar + if !tvar.isInstantiated && occursInParam(tree.tpe, tvar, secCount) => tvar + } + case _ => Nil + } + collect(tree, 0) + } + def isBottomType(tp: Type)(implicit ctx: Context) = tp == defn.NothingType || tp == defn.NullType @@ -257,9 +288,11 @@ trait Inferencing { this: Checking => } /** An enumeration controlling the degree of forcing in "is-dully-defined" checks. */ -@sharable object ForceDegree extends Enumeration { - val none, // don't force type variables - noBottom, // force type variables, fail if forced to Nothing or Null - all = Value // force type variables, don't fail +@sharable object ForceDegree { + class Value(val appliesTo: TypeVar => Boolean) + val none = new Value(_ => false) + val all = new Value(_ => true) + val noBottom = new Value(_ => true) } + diff --git a/src/dotty/tools/dotc/typer/Typer.scala b/src/dotty/tools/dotc/typer/Typer.scala index 33ec156a1d72..ca5bd6ac1dc8 100644 --- a/src/dotty/tools/dotc/typer/Typer.scala +++ b/src/dotty/tools/dotc/typer/Typer.scala @@ -1309,6 +1309,8 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit case wtp: ExprType => adaptInterpolated(tree.withType(wtp.resultType), pt, original) case wtp: ImplicitMethodType if constrainResult(wtp, followAlias(pt)) => + val tvarsToInstantiate = tvarsInParams(tree) + wtp.paramTypes.foreach(instantiateSelected(_, tvarsToInstantiate)) val constr = ctx.typerState.constraint def addImplicitArgs = { def implicitArgError(msg: => String): Tree = { From 7cbc5904056e99ec920bbf9ceaeeead84fdec5da Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 9 Aug 2015 15:22:42 -0700 Subject: [PATCH 10/15] Add test case --- tests/pos/i739.scala | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 tests/pos/i739.scala diff --git a/tests/pos/i739.scala b/tests/pos/i739.scala new file mode 100644 index 000000000000..340478f4fa5c --- /dev/null +++ b/tests/pos/i739.scala @@ -0,0 +1,10 @@ +class Foo + +object Test { + def foo[T](x: T)(implicit ev: T): T = ??? + + def test: Unit = { + implicit val evidence: Foo = new Foo + foo(new Foo) + } +} From bb1c8335b7ffb07f1865a47bcb39fc954cad83d9 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 9 Aug 2015 15:29:20 -0700 Subject: [PATCH 11/15] Added neg test (scalac and dotty both produce an error here) --- tests/neg/i739.scala | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 tests/neg/i739.scala diff --git a/tests/neg/i739.scala b/tests/neg/i739.scala new file mode 100644 index 000000000000..5385fa42cced --- /dev/null +++ b/tests/neg/i739.scala @@ -0,0 +1,7 @@ +class Foo[A, B] +class Test { + implicit val f: Foo[Int, String] = ??? + def t[A, B >: A](a: A)(implicit f: Foo[A, B]) = ??? + t(1) // error +} + From 3d09c687f9e135dbbf4a899996da88cacf39d561 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 10 Aug 2015 12:56:08 -0700 Subject: [PATCH 12/15] Generalize set of typevars instantiated before implicit search We now also consider type variables in a selection prefix of the application. The test case was augmented to include a snippet which only succeeds under the generalization. --- src/dotty/tools/dotc/typer/Inferencing.scala | 55 ++++++++++++-------- tests/pos/i739.scala | 7 +++ 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/src/dotty/tools/dotc/typer/Inferencing.scala b/src/dotty/tools/dotc/typer/Inferencing.scala index 421459fd7f63..d60057ecfa25 100644 --- a/src/dotty/tools/dotc/typer/Inferencing.scala +++ b/src/dotty/tools/dotc/typer/Inferencing.scala @@ -17,6 +17,7 @@ import Decorators._ import Uniques._ import ErrorReporting.{errorType, DiagnosticString} import config.Printers._ +import annotation.tailrec import collection.mutable trait Inferencing { this: Checking => @@ -97,31 +98,43 @@ trait Inferencing { this: Checking => } } - /** If `tree`'s type is of the form - * - * e [T1, ..., Tn] (ps1)...(psn) - * - * the list of uninstantiated type variables matching one of `T1`, ..., `Tn` - * which also appear in one of the parameter sections `ps1`, ..., `psn`, otherwise Nil. + /** The list of uninstantiated type variables bound by some prefix of type `T` which + * occur in at least one formal parameter type of a prefix application. + * Considered prefixes are: + * - The function `f` of an application node `f(e1, .., en)` + * - The function `f` of a type application node `f[T1, ..., Tn]` + * - The prefix `p` of a selection `p.f`. + * - The result expression `e` of a block `{s1; .. sn; e}`. */ def tvarsInParams(tree: Tree)(implicit ctx: Context): List[TypeVar] = { - def occursInParam(mtp: Type, tvar: TypeVar, secCount: Int): Boolean = mtp match { - case mtp: MethodType => - secCount > 0 && ( - mtp.paramTypes.exists(tvar.occursIn) || - occursInParam(mtp.resultType, tvar, secCount - 1)) - case _ => false - } - def collect(tree: Tree, secCount: Int): List[TypeVar] = tree match { - case Apply(fn, _) => collect(fn, secCount + 1) - case TypeApply(_, targs) => - targs.tpes.collect { - case tvar: TypeVar - if !tvar.isInstantiated && occursInParam(tree.tpe, tvar, secCount) => tvar + @tailrec def boundVars(tree: Tree, acc: List[TypeVar]): List[TypeVar] = tree match { + case Apply(fn, _) => boundVars(fn, acc) + case TypeApply(fn, targs) => + val tvars = targs.tpes.collect { + case tvar: TypeVar if !tvar.isInstantiated => tvar } - case _ => Nil + boundVars(fn, acc ::: tvars) + case Select(pre, _) => boundVars(pre, acc) + case Block(_, expr) => boundVars(expr, acc) + case _ => acc } - collect(tree, 0) + @tailrec def occurring(tree: Tree, toTest: List[TypeVar], acc: List[TypeVar]): List[TypeVar] = + if (toTest.isEmpty) acc + else tree match { + case Apply(fn, _) => + fn.tpe match { + case mtp: MethodType => + val (occ, nocc) = toTest.partition(tvar => mtp.paramTypes.exists(tvar.occursIn)) + occurring(fn, nocc, occ ::: acc) + case _ => + occurring(fn, toTest, acc) + } + case TypeApply(fn, targs) => occurring(fn, toTest, acc) + case Select(pre, _) => occurring(pre, toTest, acc) + case Block(_, expr) => occurring(expr, toTest, acc) + case _ => acc + } + occurring(tree, boundVars(tree, Nil), Nil) } def isBottomType(tp: Type)(implicit ctx: Context) = diff --git a/tests/pos/i739.scala b/tests/pos/i739.scala index 340478f4fa5c..61fed4e5dee4 100644 --- a/tests/pos/i739.scala +++ b/tests/pos/i739.scala @@ -3,8 +3,15 @@ class Foo object Test { def foo[T](x: T)(implicit ev: T): T = ??? + class Fn[T] { + def invoke(implicit ev: T): T = ??? + } + + def bar[T](x: T): Fn[T] = ??? + def test: Unit = { implicit val evidence: Foo = new Foo foo(new Foo) + bar(new Foo).invoke } } From d6b9fcee550e54bbfa544839c2dd1a896809e4b4 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 10 Aug 2015 13:02:10 -0700 Subject: [PATCH 13/15] Ignore implicit parameters when interpolating type variables. Fixes #738. The previous scheme interpolated type variables according to the full variance of the result type. This means that the interpolation direction is downwards if a type varaible occurs like this: (implicit x: T)T but it is upwards if it occurs like this (implicit x: T)Unit For a normal method this makes sense, since the upper bound of T gives you the most general function. But if the method is implicit, it makes no sense, since upward instantiation means that we likely get ambiguity errors. Interestingly the fix causes a different problem (exemplified by failures in run/stream-stack-overflow.scala, and also in dotc itself), which will be fixed in the next commit. --- src/dotty/tools/dotc/typer/Inferencing.scala | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/dotty/tools/dotc/typer/Inferencing.scala b/src/dotty/tools/dotc/typer/Inferencing.scala index d60057ecfa25..aeb052aff990 100644 --- a/src/dotty/tools/dotc/typer/Inferencing.scala +++ b/src/dotty/tools/dotc/typer/Inferencing.scala @@ -290,6 +290,14 @@ trait Inferencing { this: Checking => if (v == null) vmap.updated(t, variance) else if (v == variance) vmap else vmap.updated(t, 0) + case t: ImplicitMethodType => + // Assume type vars in implicit parameters are non-variant, in order not + // to widen them prematurely to upper bound if they don't occur in the result type. + val saved = variance + variance = 0 + val vmap1 = foldOver(vmap, t.paramTypes) + variance = saved + this(vmap1, t.resultType) case _ => foldOver(vmap, t) } From 686b6e4cd458a16fcfba239b68493703ff560a43 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 10 Aug 2015 13:14:13 -0700 Subject: [PATCH 14/15] Instantiate all type vars occurring in formal implicit parameters downwards. With the previous commit, we had an interesting failure in relation to CanBuildFrom. Say we have expr: (implicit cbf: CanBuildFrom[Repr, Elem, That]): That for some type vars Repr >: R, Elem >: R, That. CanBuildFrom is defined as trait CanBuildFrom[-Repr, -Elem, +That] Since Repr and Elem appear doubly contravariantly - that is covariantly - in (implicit cbf: CanBuildFrom[Repr, Elem, That]): That it means they are minimized. With the change of the last commit, implicit parameters are ignored for interpolation, so Repr, and Elem are not interpolated. But they will be instantiated in `adaptNoArgs` as type variables occurring in an implicit type. Only now the instantiation is according to the variance of the argument, so the type variables are maximized to Any! The same problem would have occurred earlier, if `Repr` or `Elem` would be part of the result type of a method, because then they would also have to be maxmimized. But by soem coincidence that code never was seen in the wild in the collection framework. The fix minimizes all type variables in formal implicit parameters, independently of their variance in the parameter type. This follows a suggestion of @extempore a while ago. There's another half to it that still needs to be implemented: when comparing multiple implicits for which one is best, we also need to ignore variance. --- src/dotty/tools/dotc/typer/Inferencing.scala | 14 +++++++++----- src/dotty/tools/dotc/typer/Typer.scala | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/dotty/tools/dotc/typer/Inferencing.scala b/src/dotty/tools/dotc/typer/Inferencing.scala index aeb052aff990..e443b6b290af 100644 --- a/src/dotty/tools/dotc/typer/Inferencing.scala +++ b/src/dotty/tools/dotc/typer/Inferencing.scala @@ -45,9 +45,13 @@ trait Inferencing { this: Checking => else throw new Error(i"internal error: type of $what $tp is not fully defined, pos = $pos") // !!! DEBUG - /** Instantiate selected type variables `tvars` in type `tp` */ - def instantiateSelected(tp: Type, tvars: List[Type])(implicit ctx: Context): Unit = - new IsFullyDefinedAccumulator(new ForceDegree.Value(tvars.contains)).process(tp) + /** Minimize selected type variables `tvars` if they appear in type `tp`. + * Instantiation is always to lower bound, independently of the variance in + * which the type variable occurs in `tp`. + */ + def minimizeSelected(tp: Type, tvars: List[Type])(implicit ctx: Context): Unit = { + new IsFullyDefinedAccumulator(new ForceDegree.Value(tvars.contains), alwaysDown = true).process(tp) + } /** The accumulator which forces type variables using the policy encoded in `force` * and returns whether the type is fully defined. Two phases: @@ -56,7 +60,7 @@ trait Inferencing { this: Checking => * 2nd Phase: If first phase was successful, instantiate all remaining type variables * to their upper bound. */ - private class IsFullyDefinedAccumulator(force: ForceDegree.Value)(implicit ctx: Context) extends TypeAccumulator[Boolean] { + private class IsFullyDefinedAccumulator(force: ForceDegree.Value, alwaysDown: Boolean = false)(implicit ctx: Context) extends TypeAccumulator[Boolean] { private def instantiate(tvar: TypeVar, fromBelow: Boolean): Type = { val inst = tvar.instantiate(fromBelow) typr.println(i"forced instantiation of ${tvar.origin} = $inst") @@ -69,7 +73,7 @@ trait Inferencing { this: Checking => case tvar: TypeVar if !tvar.isInstantiated => force.appliesTo(tvar) && { val minimize = - variance >= 0 && !( + alwaysDown || variance >= 0 && !( force == ForceDegree.noBottom && isBottomType(ctx.typeComparer.approximation(tvar.origin, fromBelow = true))) if (minimize) instantiate(tvar, fromBelow = true) diff --git a/src/dotty/tools/dotc/typer/Typer.scala b/src/dotty/tools/dotc/typer/Typer.scala index ca5bd6ac1dc8..11d2158a67e8 100644 --- a/src/dotty/tools/dotc/typer/Typer.scala +++ b/src/dotty/tools/dotc/typer/Typer.scala @@ -1310,7 +1310,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit adaptInterpolated(tree.withType(wtp.resultType), pt, original) case wtp: ImplicitMethodType if constrainResult(wtp, followAlias(pt)) => val tvarsToInstantiate = tvarsInParams(tree) - wtp.paramTypes.foreach(instantiateSelected(_, tvarsToInstantiate)) + wtp.paramTypes.foreach(minimizeSelected(_, tvarsToInstantiate)) val constr = ctx.typerState.constraint def addImplicitArgs = { def implicitArgError(msg: => String): Tree = { From c734216e70fd1dc4be349a57514004acf6aea995 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 10 Aug 2015 14:10:12 -0700 Subject: [PATCH 15/15] Add test case demonstrating that #738 is fixed. --- tests/pos/i738.scala | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 tests/pos/i738.scala diff --git a/tests/pos/i738.scala b/tests/pos/i738.scala new file mode 100644 index 000000000000..262f64637023 --- /dev/null +++ b/tests/pos/i738.scala @@ -0,0 +1,11 @@ +object Test { + def twoN[T](x: T)(implicit ev: T): Nothing = ??? + def twoT[T](x: T)(implicit ev: T): T = ??? + + def test = { + implicit val i: Int = 1 + + twoN(42) // dotty with #738: T=Any, implicit search fails; scalac: T=Int, implicit search succeeds + twoT(42) // dotty with #738 and scalac: T=Int, implicit search succeeds + } +}