Even more careful handling of tailcalls.

See i321 doc for description of problem and decision taken.

2 files changed, 44 insertions, 24 deletions

diff --git a/src/dotty/tools/dotc/transform/TailRec.scala b/src/dotty/tools/dotc/transform/TailRec.scala
index 15f4f9b4f..b747636c7 100644
--- a/src/dotty/tools/dotc/transform/TailRec.scala
+++ b/src/dotty/tools/dotc/transform/TailRec.scala
@@ -74,23 +74,25 @@ class TailRec extends MiniPhaseTransform with DenotTransformer with FullParamete
   final val labelPrefix = "tailLabel"
   final val labelFlags = Flags.Synthetic | Flags.Label
 
-  private def mkLabel(method: Symbol)(implicit c: Context): TermSymbol = {
+  private def mkLabel(method: Symbol, abstractOverClass: Boolean)(implicit c: Context): TermSymbol = {
     val name = c.freshName(labelPrefix)
 
-    c.newSymbol(method, name.toTermName, labelFlags, fullyParameterizedType(method.info, method.enclosingClass.asClass, abstractOverClass = false))
+    c.newSymbol(method, name.toTermName, labelFlags, fullyParameterizedType(method.info, method.enclosingClass.asClass, abstractOverClass))
   }
 
   override def transformDefDef(tree: tpd.DefDef)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = {
+    val sym = tree.symbol
     tree match {
       case dd@DefDef(name, tparams, vparamss0, tpt, rhs0)
-        if (dd.symbol.isEffectivelyFinal) && !((dd.symbol is Flags.Accessor) || (rhs0 eq EmptyTree) || (dd.symbol is Flags.Label)) =>
-        val mandatory = dd.symbol.hasAnnotation(defn.TailrecAnnotationClass)
+        if (sym.isEffectivelyFinal) && !((sym is Flags.Accessor) || (rhs0 eq EmptyTree) || (sym is Flags.Label)) =>
+        val mandatory = sym.hasAnnotation(defn.TailrecAnnotationClass)
         atGroupEnd { implicit ctx: Context =>
 
           cpy.DefDef(dd)(rhs = {
 
-            val origMeth = tree.symbol
-            val label = mkLabel(dd.symbol)
+            val defIsTopLevel = sym.owner.isClass
+            val origMeth = sym
+            val label = mkLabel(sym, abstractOverClass = defIsTopLevel)
             val owner = ctx.owner.enclosingClass.asClass
             val thisTpe = owner.thisType.widen
 
@@ -101,7 +103,7 @@ class TailRec extends MiniPhaseTransform with DenotTransformer with FullParamete
             // and second one will actually apply,
             // now this speculatively transforms tree and throws away result in many cases
             val rhsSemiTransformed = {
-              val transformer = new TailRecElimination(dd.symbol, owner, thisTpe, mandatory, label)
+              val transformer = new TailRecElimination(origMeth, owner, thisTpe, mandatory, label, abstractOverClass = defIsTopLevel)
               val rhs = atGroupEnd(transformer.transform(rhs0)(_))
               rewrote = transformer.rewrote
               rhs
@@ -109,8 +111,8 @@ class TailRec extends MiniPhaseTransform with DenotTransformer with FullParamete
 
             if (rewrote) {
               val dummyDefDef = cpy.DefDef(tree)(rhs = rhsSemiTransformed)
-              val res = fullyParameterizedDef(label, dummyDefDef, abstractOverClass = false)
-              val call = forwarder(label, dd, abstractOverClass = false)
+              val res = fullyParameterizedDef(label, dummyDefDef, abstractOverClass = defIsTopLevel)
+              val call = forwarder(label, dd, abstractOverClass = defIsTopLevel)
               Block(List(res), call)
             } else {
               if (mandatory)
@@ -130,7 +132,7 @@ class TailRec extends MiniPhaseTransform with DenotTransformer with FullParamete
 
   }
 
-  class TailRecElimination(method: Symbol, enclosingClass: Symbol, thisType: Type, isMandatory: Boolean, label: Symbol) extends tpd.TreeMap {
+  class TailRecElimination(method: Symbol, enclosingClass: Symbol, thisType: Type, isMandatory: Boolean, label: Symbol, abstractOverClass: Boolean) extends tpd.TreeMap {
 
     import dotty.tools.dotc.ast.tpd._
 
@@ -179,9 +181,11 @@ class TailRec extends MiniPhaseTransform with DenotTransformer with FullParamete
         val targs = typeArguments.map(noTailTransform)
         val argumentss = arguments.map(noTailTransforms)
 
-        val receiverIsSame = enclosingClass.typeRef.widen =:= recv.tpe.widen
-        val receiverIsSuper = (method.name eq sym) && enclosingClass.typeRef.widen <:< recv.tpe.widen
-        val receiverIsThis = recv.tpe.widen =:= thisType
+        val recvWiden = recv.tpe.widenDealias
+
+        val receiverIsSame = enclosingClass.typeRef.widenDealias =:= recvWiden
+        val receiverIsSuper = (method.name eq sym) && enclosingClass.typeRef.widen <:< recvWiden
+        val receiverIsThis = recv.tpe =:= thisType
 
         val isRecursiveCall = (method eq sym)
 
@@ -205,12 +209,12 @@ class TailRec extends MiniPhaseTransform with DenotTransformer with FullParamete
           rewrote = true
           val reciever = noTailTransform(recv)
 
-          /*
-                 handling changed type arguments in sound way is hard, see test `i321`
-                 val classTypeArgs = recv.tpe.baseTypeWithArgs(enclosingClass).argInfos
-                 val trz = classTypeArgs.map(x => ref(x.typeSymbol))
-          */
-          val callTargs: List[tpd.Tree] = targs
+          val callTargs: List[tpd.Tree] =
+            if(abstractOverClass) {
+              val classTypeArgs = recv.tpe.baseTypeWithArgs(enclosingClass).argInfos
+              targs ::: classTypeArgs.map(x => ref(x.typeSymbol))
+            } else targs
+
           val method = Apply(if (callTargs.nonEmpty) TypeApply(Ident(label.termRef), callTargs) else Ident(label.termRef),
             List(reciever))
 
diff --git a/tests/pos/tailcall/i321.scala b/tests/pos/tailcall/i321.scala
index 0d32dd625..33b3075de 100644
--- a/tests/pos/tailcall/i321.scala
+++ b/tests/pos/tailcall/i321.scala
@@ -1,10 +1,26 @@
+import scala.annotation.tailrec
+/**
+ * Illustrates that abstracting over type arguments without triggering Ycheck failure is tricky
+ *
+ * go1.loop refers to type parameter of i321, and captures value f
+ * if goo1.loop will abstract over T it will need to cast f or will trigger a Ycheck failure.
+ * One could decide to not abstract over type parameters in tail calls, but this leads us to go2 example
+ *
+ * In go2 we should abstract over i321.T, as we need to change it in recursive call.
+ *
+ * For now decision is such - we will abstract for top-level methods, but will not for inner ones.
+ */
+
 class i321[T >: Null <: AnyRef] {
 
-  def mapconserve(f: T => Int): Int = {
-    def loop(pending: T): Int = {
-        val head1 = f(pending)
-        loop(pending)
-      }
+  def go1(f: T => Int): Int = {
+    @tailrec def loop(pending: T): Int = {
+      val head1 = f(pending)
+      loop(pending)
+    }
     loop(null)
   }
+
+  final def go2[U >: Null <: AnyRef](t: i321[U]): Int = t.go2(this)
+
 }
\ No newline at end of file