Tailcalls generates references to labels from local functions.

[odersky]

... and LambdaLift does not know what to do about them. In fact, I think we should not let TailCalls generate such labels.

To reproduce, check out branch transform/lambdalift and run the compiler with
-Ycheck:lambda on tests/pending/pos/tailcalls/t1672.scala.

[DarkDimius]

@odersky I had a look on the failing test.

object Test1672 {
  @annotation.tailrec
  def bar(x: Int)(y: Int) : Nothing = {
    try {
      throw new RuntimeException
    } catch {
      case _: Throwable => bar(x)(y)
    }
  }
}

from https://issues.scala-lang.org/browse/SI-1672

For this particular test I would argue that tailrec does the right job: the call is in tail position and should be rewritten.

But instead I would say that the $anonfun inside should not get lifted, as this $anonfunis the exception handler and should be inline in the bytecode.
What do you think?

But for case of local function that isn't an exception handler I agree that it's a bug in tailrec.

[odersky]

On Wed, Oct 22, 2014 at 1:35 PM, Dmitry Petrashko notifications@github.com
wrote:

@odersky https://github.com/odersky I had a look on the failing test.

object Test1672 {
@annotation.tailrec
def bar(x: Int)(y: Int) : Nothing = {
try {
throw new RuntimeException
} catch {
case _: Throwable => bar(x)(y)
}
}
}

from https://issues.scala-lang.org/browse/SI-1672

For this particular test I would argue that tailrec does the right job:
the call is in tail position and should be rewritten.

But instead I would say that the $anonfun inside should not get lifted,
as this $anonfunis the exception handler and should be inline in the
bytecode.
What do you think?

Lambalift sees anonymous functions and that is it. It currently does not know that this
is an exception handler that can be lifted. We should decide whether it is feasible to do so, or whether
it's preferable not to generate an anonymous function for an exception handler in the first place.

In fact, the problem is more complicated. Note that some try expressions
need to be put into a separate function (because they get executed on
non-empty expression stacks).

Here's an example:

def foo(x: Int) =
try {
foo(x + 1)
} catch {
case ex => 0
}

This code can be written without a local function. But the following
variation cannot:

def foo(x: Int) =
1 + try {
foo(x + 1)
} catch {
case ex => 0
}

The second example needs to be expanded to something like:

def foo(x: Int) = {
def foo$try = try {
foo(x + 1)
} catch {
case ex => 0
}
1+ foo$try(x)
}

... and then foo is no longer in tail position.

It looks to me that all this should be clarified before lambda lift, and also before tailcalls. I.e. we need a phase that puts these try's in local functions, so that tailcalls does the right thing.

[odersky]

previous comment was sent by error in an unfinished state. I have corrected it now.

[odersky]

After thinking about it, I agree this has nothing to do with TailCalls. What we should do instead is generate a Try node that does not introduce artificial anonymous functions. Essentially, the way it is doen in scalac. @DarkDimius do you want to give this a go?

[DarkDimius]

@odersky in your example

def foo(x: Int) = 
 1 + try { foo(x + 1) }
catch { case ex => 0 } 

call to foo is not in tail position even before desugaring for 2 reasons: first it's in Try block with non-empty handler, and second it is inside an argument of Apply node.

So for both your examples call to foo is not in tail position.

@odersky can you please explain why is try block desugared in the first place? Was it creating some problems in scalac?

[odersky]

That's true. As I wrote in my last comment, it has nothing to do with tailcalls.

Try representation: It's easier for the parser and typer to treat the catch block as an anonymous function, precisely because one can use an anonymous function. I.e.

try foo(...)
catch handler 

where handler is an ident of type Exception => T is perfectly legal. Normalizing to catch blocks takes work. But it looks like we have to do it anyway because at the latest lambdaLift does not like to see closures as catch handlers.

[DarkDimius]

I see now. Thanks for explanation.

@DarkDimius do you want to give this a go?

Ok, I'll have a look how to modify parser & typer.
Do I understand correctly that after this modification we won't need Closure AST node anymore?

[odersky]

No, closures are the results of Function literals.

On Wed, Oct 22, 2014 at 10:16 PM, Dmitry Petrashko <notifications@github.com

wrote:

I see now. Thanks for explanation.

@DarkDimius https://github.com/DarkDimius do you want to give this a go?

Ok, I'll have a look how to modify parser & typer.
Do I understand correctly that after this modification we won't need
Closure AST node anymore?

—
Reply to this email directly or view it on GitHub
#196 (comment).

Martin Odersky
EPFL