Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 6, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions compiler/src/dotty/tools/dotc/transform/TailRec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import reporting.diagnostic.messages.TailrecNotApplicable
* {{{
* var localForParam1: T1 = param1
* ...
* while (true) {
* while (<empty>) {
* tailResult[ResultType]: {
* return {
* // original rhs with tail recursive calls transformed (see below)
Expand Down Expand Up @@ -60,7 +60,7 @@ import reporting.diagnostic.messages.TailrecNotApplicable
* def fact(n: Int, acc: Int): Int = {
* var acc$tailLocal1: Int = acc
* var n$tailLocal1: Int = n
* while (true) {
* while (<empty>) {
* tailLabel1[Unit]: {
* return {
* if (n$tailLocal1 == 0)
Expand All @@ -75,7 +75,6 @@ import reporting.diagnostic.messages.TailrecNotApplicable
* }
* }
* }
* throw null // unreachable
* }
* }}}
*
Expand Down Expand Up @@ -163,14 +162,12 @@ class TailRec extends MiniPhase {
}

Block(
initialVarDefs :::
WhileDo(Literal(Constant(true)), {
initialVarDefs,
WhileDo(EmptyTree, {
Labeled(transformer.continueLabel.asTerm, {
Return(rhsFullyTransformed, origMeth)
})
}) ::
Nil,
Throw(Literal(Constant(null))) // unreachable code
})
)
} else {
if (mandatory) ctx.error(
Expand Down
5 changes: 5 additions & 0 deletions compiler/src/dotty/tools/dotc/transform/TreeChecker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

super.typedWhileDo(tree)
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


override def ensureNoLocalRefs(tree: Tree, pt: Type, localSyms: => List[Symbol])(implicit ctx: Context): Tree =
tree

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this occur ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


def assignType(tree: untpd.Try, expr: Tree, cases: List[CaseDef])(implicit ctx: Context): Try =
if (cases.isEmpty) tree.withType(expr.tpe)
Expand Down
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1127,7 +1127,9 @@ class Typer extends Namer
}

def typedWhileDo(tree: untpd.WhileDo)(implicit ctx: Context): Tree = track("typedWhileDo") {
val cond1 = typed(tree.cond, defn.BooleanType)
val cond1 =
if (tree.cond eq EmptyTree) EmptyTree
else typed(tree.cond, defn.BooleanType)
val body1 = typed(tree.body, defn.UnitType)
assignType(cpy.WhileDo(tree)(cond1, body1))
}
Expand Down