Use `Labeled` blocks in `TailRec`, instead of label-defs. #5115

sjrd · 2018-09-17T19:00:22Z

This PR depends on #5113 and #5112.

It's easier to first explain on an example. Consider the following tail-recursive method:

  def fact(n: Int, acc: Int): Int =
    if (n == 0) acc
    else fact(n - 1, n * acc)

It is now translated as follows by the tailrec transform:

  def fact(n: Int, acc: Int): Int = {
    var n$tailLocal1: Int = n
    var acc$tailLocal1: Int = acc
    while (true) {
      tailLabel1[Unit]: {
        return {
          if (n$tailLocal1 == 0) {
            acc
          } else {
            val n$tailLocal1$tmp: Int = n$tailLocal1 - 1
            val acc$tailLocal1$tmp: Int = n$tailLocal1 * acc$tailLocal1
            n$tailLocal1 = n$tailLocal1$tmp
            acc$tailLocal1 = acc$tailLocal1$tmp
            return[tailLabel1] ()
          }
        }
      }
    }
    throw null // unreachable code
  }

First, we allocate local vars for every parameter, as well as this if necessary.

When we find a tail-recursive call, we evaluate the arguments into temporaries, then assign them to the vars. It is necessary to use temporaries in order not to use the new contents of a param local when computing the new value of another param local.

We avoid reassigning param locals if their rhs (i.e., the actual argument to the recursive call) is itself, which does happen quite often in practice. In particular, we thus avoid reassigning the local var for this if the prefix is empty. We could further optimize this by avoiding the reassignment if the prefix is non-empty but equivalent to this.

If only one parameter ends up changing value in any particular tail-recursive call, we can avoid the temporaries and directly assign it. This is also a fairly common situation, especially after discarding useless assignments to the local for this.

After all that, we return from a labeled block, which is right inside an infinite while loop. The net result is to loop back to the beginning, implementing the jump.

For control flows that do not end up in a tail-recursive call, the result value is given to an explicit return out of the enclosing method, which prevents the looping.

There is one pretty ugly artifact: after the while loop, we must insert a throw null for the body to still typecheck as an Int (the result type of the def). This could be avoided if we dared type a WhileDo(Literal(Constant(true)), body) as having type Nothing rather than Unit. This is probably dangerous, though, as we have no guarantee that further transformations will leave the true alone, especially in the presence of compiler plugins. If the true gets wrapped in any way, the type of the WhileDo will be altered, and chaos will ensue.

In the future, we could enhance the codegen to avoid emitting that dead code. This should not be too difficult:

emitting a WhileDo whose argument is true would set the generated BType to Nothing.
then, when emitting a Block, we would drop any statements and expr following a statement whose generated BType was Nothing.

This commit does not go to such lengths, however.

This change removes the last source of label-defs in the compiler. After this commit, we will be able to entirely remove label-defs.

dottybot

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Commit Messages

We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).

Please stick to these guidelines for commit messages:

Separate subject from body with a blank line

When fixing an issue, start your commit message with Fix #<ISSUE-NBR>:

Limit the subject line to 72 characters

Capitalize the subject line

Do not end the subject line with a period

Use the imperative mood in the subject line ("Add" instead of "Added")

Wrap the body at 80 characters

Use the body to explain what and why vs. how

adapted from https://chris.beams.io/posts/git-commit

Have an awesome day! ☀️

allanrenucci · 2018-09-17T19:29:59Z

Can we avoid the throw null by translating to something like?

  def fact(n: Int, acc: Int): Int = {
    var n$tailLocal1: Int = n
    var acc$tailLocal1: Int = acc
    var result$tailLocal1: Int = _
    tailLabel1[Unit]: {
      while (true) {
        if (n$tailLocal1 == 0) {
          result$tailLocal1 = acc$tailLocal1
          return[tailLabel1] ()
        } else {
          val n$tailLocal1$tmp: Int = n$tailLocal1 - 1
          val acc$tailLocal1$tmp: Int = n$tailLocal1 * acc$tailLocal1
          n$tailLocal1 = n$tailLocal1$tmp
          acc$tailLocal1 = acc$tailLocal1$tmp
        }
      }
    }
    result$tailLocal1
  }

EDIT: Introduced a new local variable for the result

sjrd · 2018-09-17T19:42:52Z

No, in general we cannot do that, because the right-hand-side to the inner return can refer to values that are not in scope at the top-level of the method. Besides, if you have several exit points, it doesn't really work either.

It would be possible to do something like this:

def fact(n: Int, acc: Int): Int = {
  var tailResult: Int = 0
  var n$tailLocal1: Int = n
  var acc$tailLocal1: Int = acc
  tailReturn1[Unit]: {
    while (true) {
      tailLabel1[Unit]: {
        tailResult = {
          if (n$tailLocal1 == 0) {
            acc
          } else {
            val n$tailLocal1$tmp: Int = n$tailLocal1 - 1
            val acc$tailLocal1$tmp: Int = n$tailLocal1 * acc$tailLocal1
            n$tailLocal1 = n$tailLocal1$tmp
            acc$tailLocal1 = acc$tailLocal1$tmp
            return[tailLabel1] ()
          }
        }
        return[tailReturn1] ()
      }
    }
  }
  tailResult
}

but that is actually less efficient than leaving the dead code, even if less "ugly".

I also thought that instead of generating throw null, I could put 0 there (or, in general, the zero of the result type). I haven't found a ready-made way to conjure up the appropriate zero Constant for a Type, though. It's certainly possible to write by hand, but tedious. It's still dead code, though (but slightly less ugly than a throw).

sjrd · 2018-09-17T22:26:22Z

There is one pretty ugly artifact: after the while loop, we must insert a throw null for the body to still typecheck as an Int (the result type of the def). This could be avoided if we dared type a WhileDo(Literal(Constant(true)), body) as having type Nothing rather than Unit. This is probably dangerous, though, as we have no guarantee that further transformations will leave the true alone, especially in the presence of compiler plugins. If the true gets wrapped in any way, the type of the WhileDo will be altered, and chaos will ensue.

A more reliable alternative would be to allow EmptyTree as the cond of WhileDo. The EmptyTree would mean "guaranteed infinite loop", and would allow the WhileDo to have type Nothing.

sjrd · 2018-09-26T09:27:11Z

Rebased. Now ready for full review.

allanrenucci · 2018-09-27T16:03:55Z

I think there is alternate translation that we may want to consider:

def fact(n: Int, acc: Int): Int = {
  var n$tailLocal1: Int = n
  var acc$tailLocal1: Int = acc

  tailReturn1[Int]: {
    while (true) {
      tailLabel1[Unit]: {
        return tailReturn1[Int] {
          if (n$tailLocal1 == 0) {
            acc
          } else {
            val n$tailLocal1$tmp: Int = n$tailLocal1 - 1
            val acc$tailLocal1$tmp: Int = n$tailLocal1 * acc$tailLocal1
            n$tailLocal1 = n$tailLocal1$tmp
            acc$tailLocal1 = acc$tailLocal1$tmp
            return[tailLabel1] ()
          }
        }
      }
    }
  }

@sjrd Can you clarify why we need temporaries for arguments of tail recursive call? I.e. why

val n$tailLocal1$tmp: Int = n$tailLocal1 - 1
val acc$tailLocal1$tmp: Int = n$tailLocal1 * acc$tailLocal1
n$tailLocal1 = n$tailLocal1$tmp
acc$tailLocal1 = acc$tailLocal1$tmp

instead of:

n$tailLocal1 = n$tailLocal1 - 1
acc$tailLocal1 = n$tailLocal1 * acc$tailLocal1

sjrd · 2018-09-27T16:11:53Z

I think there is alternate translation that we may want to consider:

That does not work. You'll still need a dead code throw after the while, inside the tailReturn1 label, because the body of tailReturn1 must typecheck as an Int.

Can you clarify why we need temporaries for arguments of tail recursive call?

In your alternative translation without temporaries, the computation of the new value for acc$tailLocal1 is wrong: it uses the new value of n$tailLocal1 instead of the old one. So you're multiplying by n - 1 instead of multiplying by n.

sjrd · 2018-09-27T17:11:47Z

I added a commit with my own alternative proposal to use WhileDo(EmptyTree, body) to represent infinite loops.

sjrd · 2018-10-03T09:27:17Z

test performance please

sjrd · 2018-10-03T10:38:19Z

Looks like the performance bot doesn't like me :p

smarter · 2018-10-03T10:39:02Z

test performance please

dottybot · 2018-10-03T10:39:32Z

performance test scheduled: 1 job(s) in queue, 0 running.

smarter · 2018-10-03T10:39:59Z

@liufengyun can you add @sjrd to the whitelist for running benchmarks ?

dottybot · 2018-10-03T12:19:16Z

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/5115/ to see the changes.

Benchmarks is based on merging with master (fd93b46)

liufengyun · 2018-10-03T12:23:36Z

Now the bot is for your service @sjrd .

sjrd · 2018-10-03T12:25:17Z

@liufengyun Thank you :)

Since you're there, it seems the performance test launched above had an issue: the page it points to is a 404 for me :s (http://dotty-bench.epfl.ch/5115/)

liufengyun · 2018-10-03T12:28:46Z

It is an issue with Github pages. Sometimes you can use the URL http://dotty-bench.epfl.ch/5115/index.html , sometimes you need to wait a little while before it's available. Le Wed, Oct 3, 2018 à 2:25 PM, Sébastien Doeraene <[email protected]> a écrit :

…

@liufengyun <https://github.com/liufengyun> Thank you :) Since you're there, it seems the performance test launched above had an issue: the page it points to is a 404 for me :s ( http://dotty-bench.epfl.ch/5115/) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5115 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAuDyQecAanMXxY1FubJLpuynatKoX_Vks5uhKyygaJpZM4WshD9> .

sjrd · 2018-10-11T14:49:45Z

Rebased.

odersky · 2018-10-12T09:19:58Z

What's the status here. Can this be merged?

sjrd · 2018-10-12T10:08:29Z

It needs a review.

allanrenucci

We should also add the missing case for While in the tree traversal:

case tree @ While(cond, body) =>
  tpd.cpy.While(tree)(noTailTransform(cond), noTailTransform(body))

And a test that exercises it

allanrenucci · 2018-10-23T09:24:30Z

compiler/src/dotty/tools/dotc/transform/TailRec.scala

+            val varsForRewrittenParamSyms = transformer.varsForRewrittenParamSyms
+
+            val initialValDefs = {
+              val initialParamValDefs = for ((param, local) <- rewrittenParamSyms.zip(varsForRewrittenParamSyms)) yield {


(rewrittenParamSyms, varsForRewrittenParamSyms).zipped to avoid intermediate collection?

I did not even know about this thing 🤣

allanrenucci · 2018-10-23T09:25:54Z

compiler/src/dotty/tools/dotc/transform/TailRec.scala

+            val rewrittenParamSyms = transformer.rewrittenParamSyms
+            val varsForRewrittenParamSyms = transformer.varsForRewrittenParamSyms
+
+            val initialValDefs = {


initialVarDefs?

Good question. Depends on what the coding style in Dotty is. There are var definitions, but they are still tpd.ValDefs because there is no such thing as a VarDef. That's why I called them initialValDefs. Is it customary to refer to tpd.ValDefs that represents vars as VarDefs?

Quickly going through the codebase, I cannot infer a coding style. No strong feeling about this one

allanrenucci · 2018-10-23T09:28:06Z

compiler/src/dotty/tools/dotc/transform/TailRec.scala

+
+            val rhsFullyTransformed = varForRewrittenThis match {
+              case Some(localThisSym) =>
+                val classSym = tree.symbol.owner.asClass


I think this is equivalent to owner defined on line 99

allanrenucci · 2018-10-23T12:03:45Z

compiler/src/dotty/tools/dotc/transform/TailRec.scala

+    var continueLabel: Option[Symbol] = None
+    var varForRewrittenThis: Option[Symbol] = None
+    var rewrittenParamSyms: List[Symbol] = Nil
+    var varsForRewrittenParamSyms: List[Symbol] = Nil


I think you could simplify the implementation here. If I understand correctly rewrittenParamSyms is just paramSyms that you reorder (for reasons I don't understand). Would it be possible to get rid of rewrittenParamSyms? Something like:

private[this] var myVarsForRewrittenParamSyms: List[Symbol] = _ def varsForRewrittenParamSyms(implicit ctx: Context) = { if (myVarsForRewrittenParamSyms == null) { myVarsForRewrittenParamSyms = paramSyms.map(param => ctx.newSymbol(method, TailLocalName.fresh(param.name.toTermName), Flags.Synthetic | Flags.Mutable, param.info)) } myVarsForRewrittenParamSyms }

No, rewrittenParamSyms is a subset of paramSyms. It only contains parameters whose value is changed in at least one tail-recursive call. This is quite important to avoid declaring and assigning useless variables.

allanrenucci · 2018-10-23T12:04:24Z

compiler/src/dotty/tools/dotc/transform/TailRec.scala

+    var continueLabel: Option[Symbol] = None
+    var varForRewrittenThis: Option[Symbol] = None
+    var rewrittenParamSyms: List[Symbol] = Nil
+    var varsForRewrittenParamSyms: List[Symbol] = Nil


These four variables would benefit from comments explaining what they are

allanrenucci · 2018-10-23T13:29:34Z

compiler/src/dotty/tools/dotc/transform/TailRec.scala

+            }
+
+            val tpt = TypeTree(method.info.resultType)
+            Block(assignments, Typed(Return(Literal(Constant(())).withPos(tree.pos), ref(getContinueLabel())), tpt))


seq(assignments, ...)

allanrenucci · 2018-10-23T13:44:08Z

compiler/src/dotty/tools/dotc/transform/TailRec.scala

+ *             while (true) {
+ *               tailResult[ResultType]: {
+ *                 return {
+ *                   // original rhs


If I understand correctly, it is not original rhs here. It is original rhs with tail recursive call rewritten as you describe below

That is true, but if you follow the "story" in the comment, it later says

Self-recursive calls in tail-position are then replaced by

So at this point in the story, it is still the original rhs.

I can also rewrite the story to make everything happen "at the same time", if you prefer.

I think it's very helpful to have a self contained example that describes the whole transformation

allanrenucci · 2018-10-23T14:06:43Z

compiler/src/dotty/tools/dotc/transform/TailRec.scala

+            }
+
+            val tpt = TypeTree(method.info.resultType)
+            Block(assignments, Typed(Return(Literal(Constant(())).withPos(tree.pos), ref(getContinueLabel())), tpt))


Maybe it is worth adding a method to tpd:

def Return(expr: Tree, from: Symbol)(implicit ctx: Context): Return = Return(expr, Ident(from.termRef))

I see multiple repetitions of Return(..., ref(...)) and Return(..., Ident(... .termRef)) (I believe both are equivalent, but the latter is more efficient)

allanrenucci · 2018-10-23T14:07:04Z

compiler/src/dotty/tools/dotc/transform/TailRec.scala

+            }
+
+            val tpt = TypeTree(method.info.resultType)
+            Block(assignments, Typed(Return(Literal(Constant(())).withPos(tree.pos), ref(getContinueLabel())), tpt))


Why is the Return tree wrapped in a TypedTree node?

To perfectly preserve the type of the node (otherwise it's Nothing), which in turn is very important for the TypeAssigner not to infer different types as lubs of branches/cases when one of the branches has been replaced by a return. Otherwise Ycheck failures arise.

allanrenucci · 2018-10-23T14:22:30Z

compiler/src/dotty/tools/dotc/transform/TailRec.scala


    import dotty.tools.dotc.ast.tpd._

    var rewrote: Boolean = false

+    var continueLabel: Option[Symbol] = None


It is weird to use an Option here since the value is not really optional. Here is the convention we tend to follow for idempotent lazily computed values:

private[this] myContinueLabel: Symbol = _ def continueLabel(implicit ctx: Context) = { if (myContinueLabel == null) { myContinueLabel = ... } myContinueLabel }

smarter · 2018-10-24T12:34:41Z

compiler/src/dotty/tools/dotc/transform/TailRec.scala

+ *               }
+ *             }
+ *             </pre>
+ *             <p>


While you're at it, you could remove the html tags in this comment, it should use markdown instead.

smarter · 2018-10-24T12:35:47Z

compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala

@@ -486,7 +486,7 @@ trait TypeAssigner {
    tree.withType(defn.NothingType)

  def assignType(tree: untpd.WhileDo)(implicit ctx: Context): WhileDo =
-    tree.withType(defn.UnitType)
+    tree.withType(if (tree.cond eq EmptyTree) defn.NothingType else defn.UnitType)


Why does this occur ?

Because we want a way to represent an infinite while loop, whose type is therefore Nothing. It's the whole point of the second commit in this PR.

sjrd

Updated. AFAICT I should have addressed the points raised in the review.

sjrd · 2018-10-26T14:32:30Z

compiler/src/dotty/tools/dotc/transform/TailRec.scala

+              case _ :: _ =>
+                val (tempValDefs, assigns) = (for ((lhs, rhs) <- assignThisAndParamPairs) yield {
+                  val temp = c.newSymbol(method, TailTempName.fresh(lhs.name.toTermName), Flags.Synthetic, lhs.info)
+                  (ValDef(temp, rhs), Assign(ref(lhs), ref(temp)).withPos(tree.pos))


Doesn't work, because it does not use the correct owner for the new symbol.

allanrenucci

This comment is not addressed:

We should also add the missing case for While in the tree traversal:

case tree @ While(cond, body) =>
  tpd.cpy.While(tree)(noTailTransform(cond), noTailTransform(body))

And tests that exercise it

Although this could be addressed as separate PR. Otherwise LGTM!

Let's discuss in the meeting if we want to keep the last commit

allanrenucci · 2018-10-29T09:17:03Z

compiler/src/dotty/tools/dotc/transform/TailRec.scala

+            val initialVarDefs = {
+              val initialParamVarDefs = (for ((param, local) <- (rewrittenParamSyms, varsForRewrittenParamSyms).zipped) yield {
+                ValDef(local.asTerm, ref(param))
+              }).toList


I think you don't need toList if you do:

val initialParamVarDefs = (rewrittenParamSyms, varsForRewrittenParamSyms).zipped.map( (local, param) => ValDef(local.asTerm, ref(param)))

Somehow, for .. yield is not equivalent to map here

It's easier to first explain on an example. Consider the following tail-recursive method: def fact(n: Int, acc: Int): Int = if (n == 0) acc else fact(n - 1, n * acc) It is now translated as follows by the `tailrec` transform: def fact(n: Int, acc: Int): Int = { var n$tailLocal1: Int = n var acc$tailLocal1: Int = acc while (true) { tailLabel1[Unit]: { return { if (n$tailLocal1 == 0) { acc } else { val n$tailLocal1$tmp1: Int = n$tailLocal1 - 1 val acc$tailLocal1$tmp1: Int = n$tailLocal1 * acc$tailLocal1 n$tailLocal1 = n$tailLocal1$tmp1 acc$tailLocal1 = acc$tailLocal1$tmp1 (return[tailLabel1] ()): Int } } } } throw null // unreachable code } First, we allocate local `var`s for every parameter, as well as `this` if necessary. When we find a tail-recursive call, we evaluate the arguments into temporaries, then assign them to the `var`s. It is necessary to use temporaries in order not to use the new contents of a param local when computing the new value of another param local. We avoid reassigning param locals if their rhs (i.e., the actual argument to the recursive call) is itself, which does happen quite often in practice. In particular, we thus avoid reassigning the local var for `this` if the prefix is empty. We could further optimize this by avoiding the reassignment if the prefix is non-empty but equivalent to `this`. If only one parameter ends up changing value in any particular tail-recursive call, we can avoid the temporaries and directly assign it. This is also a fairly common situation, especially after discarding useless assignments to the local for `this`. After all that, we `return` from a labeled block, which is right inside an infinite `while` loop. The net result is to loop back to the beginning, implementing the jump. The `return` node is explicitly ascribed with the previous result type, so that lubs upstream are not affected (not doing so can cause Ycheck errors). For control flows that do *not* end up in a tail-recursive call, the result value is given to an explicit `return` out of the enclosing method, which prevents the looping. There is one pretty ugly artifact: after the `while` loop, we must insert a `throw null` for the body to still typecheck as an `Int` (the result type of the `def`). This could be avoided if we dared type a `WhileDo(Literal(Constant(true)), body)` as having type `Nothing` rather than `Unit`. This is probably dangerous, though, as we have no guarantee that further transformations will leave the `true` alone, especially in the presence of compiler plugins. If the `true` gets wrapped in any way, the type of the `WhileDo` will be altered, and chaos will ensue. In the future, we could enhance the codegen to avoid emitting that dead code. This should not be too difficult: * emitting a `WhileDo` whose argument is `true` would set the generated `BType` to `Nothing`. * then, when emitting a `Block`, we would drop any statements and expr following a statement whose generated `BType` was `Nothing`. This commit does not go to such lengths, however. This change removes the last source of label-defs in the compiler. After this commit, we will be able to entirely remove label-defs.

sjrd · 2018-11-05T17:35:14Z

Updated to avoid the .toList, as suggested.

I have also tried the variant where we cast to Nothing, but it turns out to be worse than throw null, unfortunately. What I had not foreseen is that the primitive Unit returned by the while is not a real value after erasure, so casting it to Nothing actually implies first loading BoxedUnit.UNIT, then casting that to Nothing. Overall, it produces more dead code than throw null. You can see the commit here:
sjrd@0897f2e

I have therefore kept the variant that uses EmptyTree in the WhileDo to represent an infinite loop.

Such `WhileDo` node have type `Nothing` instead of `Unit`. This is used in the loops generated by `TailRec` in order not to need some spurious dead code.

sjrd · 2018-11-05T17:44:22Z

compiler/src/dotty/tools/dotc/transform/TreeChecker.scala

+    override def typedWhileDo(tree: untpd.WhileDo)(implicit ctx: Context): Tree = {
+      assert((tree.cond ne EmptyTree) || ctx.phase.refChecked, i"invalid empty condition in while at $tree")
+      super.typedWhileDo(tree)
+    }


And here is the addition of an explicit Ycheck test that WhileDo(EmptyTree, _) is not used too early, as requested by @smarter.

allanrenucci · 2018-11-06T14:57:40Z

compiler/src/dotty/tools/dotc/transform/TreeChecker.scala

@@ -452,6 +452,11 @@ class TreeChecker extends Phase with SymTransformer {
      tree1
    }

+    override def typedWhileDo(tree: untpd.WhileDo)(implicit ctx: Context): Tree = {
+      assert((tree.cond ne EmptyTree) || ctx.phase.refChecked, i"invalid empty condition in while at $tree")


Why ctx.phase.refChecked? Why not an equivalent method for TailRec?

Because it existed, and the only requirement is that an infinite loop must not be pickled. It doesn't have anything to do with tailrec per se.

Probably fixed by scala#5115

dottybot reviewed Sep 17, 2018

View reviewed changes

sjrd force-pushed the tailrec-use-labeled-blocks branch 2 times, most recently from b920577 to a64ba26 Compare September 22, 2018 10:06

sjrd mentioned this pull request Sep 22, 2018

Remove label-defs. #5141

Merged

sjrd force-pushed the tailrec-use-labeled-blocks branch 2 times, most recently from c2aa05f to 4adc949 Compare September 26, 2018 09:25

sjrd changed the title ~~DO NOT MERGE Use Labeled blocks in TailRec, instead of label-defs.~~ Use Labeled blocks in TailRec, instead of label-defs. Sep 26, 2018

allanrenucci mentioned this pull request Sep 26, 2018

Fix #5163: Properly report tailrec failures on recursive call targeting a supertype #5168

Closed

OlivierBlanvillain assigned allanrenucci Oct 2, 2018

sjrd force-pushed the tailrec-use-labeled-blocks branch from 52d8c77 to abaf97e Compare October 11, 2018 14:23

sjrd added the stat:needs review label Oct 12, 2018

allanrenucci suggested changes Oct 23, 2018

View reviewed changes

smarter reviewed Oct 24, 2018

View reviewed changes

allanrenucci assigned sjrd and unassigned allanrenucci Oct 25, 2018

allanrenucci removed the stat:needs review label Oct 25, 2018

sjrd force-pushed the tailrec-use-labeled-blocks branch from abaf97e to 437a828 Compare October 26, 2018 14:41

sjrd commented Oct 26, 2018

View reviewed changes

sjrd force-pushed the tailrec-use-labeled-blocks branch from 8f408bb to 92458fc Compare October 26, 2018 14:58

sjrd assigned allanrenucci and unassigned sjrd Oct 26, 2018

allanrenucci approved these changes Oct 29, 2018

View reviewed changes

allanrenucci assigned sjrd and unassigned allanrenucci Oct 31, 2018

sjrd force-pushed the tailrec-use-labeled-blocks branch from 92458fc to 0c24a34 Compare November 5, 2018 17:31

Use EmptyTree for the cond of infinite WhileDo loops.

3084b1a

Such `WhileDo` node have type `Nothing` instead of `Unit`. This is used in the loops generated by `TailRec` in order not to need some spurious dead code.

sjrd force-pushed the tailrec-use-labeled-blocks branch from 0c24a34 to 3084b1a Compare November 5, 2018 17:43

sjrd commented Nov 5, 2018

View reviewed changes

sjrd assigned allanrenucci and unassigned sjrd Nov 5, 2018

allanrenucci reviewed Nov 6, 2018

View reviewed changes

sjrd merged commit 2cc525a into scala:master Nov 6, 2018

sjrd deleted the tailrec-use-labeled-blocks branch November 6, 2018 15:42

nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 21, 2019

Fix scala#3271: Add regression test

3173514

Probably fixed by scala#5115

nicolasstucki mentioned this pull request Jan 21, 2019

Fix #3271: Add regression test #5757

Merged

Use Labeled blocks in TailRec, instead of label-defs. #5115

Use Labeled blocks in TailRec, instead of label-defs. #5115

Uh oh!

Conversation

sjrd commented Sep 17, 2018

Uh oh!

dottybot left a comment

Choose a reason for hiding this comment

Commit Messages

Uh oh!

allanrenucci commented Sep 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sjrd commented Sep 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sjrd commented Sep 17, 2018

Uh oh!

sjrd commented Sep 26, 2018

Uh oh!

allanrenucci commented Sep 27, 2018

Uh oh!

sjrd commented Sep 27, 2018

Uh oh!

sjrd commented Sep 27, 2018

Uh oh!

sjrd commented Oct 3, 2018

Uh oh!

sjrd commented Oct 3, 2018

Uh oh!

smarter commented Oct 3, 2018

Uh oh!

dottybot commented Oct 3, 2018

Uh oh!

smarter commented Oct 3, 2018

Uh oh!

dottybot commented Oct 3, 2018

Uh oh!

liufengyun commented Oct 3, 2018

Uh oh!

sjrd commented Oct 3, 2018

Uh oh!

liufengyun commented Oct 3, 2018 via email

Uh oh!

sjrd commented Oct 11, 2018

Uh oh!

odersky commented Oct 12, 2018

Uh oh!

sjrd commented Oct 12, 2018

Uh oh!

allanrenucci left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Use `Labeled` blocks in `TailRec`, instead of label-defs. #5115

Use `Labeled` blocks in `TailRec`, instead of label-defs. #5115

allanrenucci commented Sep 17, 2018 •

edited

Loading

sjrd commented Sep 17, 2018 •

edited

Loading

allanrenucci left a comment •

edited

Loading

allanrenucci Oct 29, 2018 •

edited

Loading