Isn't that code in tail recursive style?

Question

I'm kinda new to Scala trying it out while reading Beggining Scala by David Pollack. He defines a simple recursive function that loads all strings from the file:

def allStrings(expr: => String): List[String] = expr match {
    case null => Nil
    case w => w :: allStrings(expr)
}

It's elegant and awesome except that it had thrown a StackOverflow exception when I tried to load a huge dictionary file.

Now as far as I understand Scala supports tail recursion, so that function call couldn't possibly overflow the stack, probably compiler doesn't recognize it? So after some googling I tried @tailrec annotation to help the compiler out, but it said

error: could not optimize @tailrec annotated method: it contains a recursive call not in tail position
def allStrings(expr: => String): List[String] =

Am I understanding tail recursion wrong? How do I fix this code?

Ben James · Accepted Answer · 2011-05-14T23:38:48.263

67

Scala can only optimise this if the last call is a call to the method itself.

Well, the last call is not to allStrings, it's actually to the :: (cons) method.

A way to make this tail recursive is to add an accumulator parameter, for example:

def allStrings(expr: => String, acc: List[String] = Nil): List[String] =
  expr match {
    case null => acc
    case w => allStrings(expr, w :: acc)
  }

To prevent the accumulator leaking into the API, you can define the tail recursive method as a nested method:

def allStrings(expr: => String) = {
  def iter(expr: => String, acc: List[String]): List[String] =
    expr match {
      case null => acc
      case w => iter(expr, w :: acc)
    }
  iter(expr, Nil)
}

edited May 14 '11 at 23:38

answered May 14 '11 at 23:24

Ben James

121,135
26
193
155

2

When copying my answer, you forgot the `@tailrec` annotation. This is not just an easy way to have the compiler confirm your expectations, it's also a useful hint to subsequent maintainers. – Kevin Wright May 14 '11 at 23:45
7

Kevin, I did not copy your answer, I was actually making edits when you posted. But you make a good point about the annotation, for which I've upvoted your answer, despite your snide remark :) – Ben James May 14 '11 at 23:56
I was going to delete mine on seeing we'd answered simultaneously, but didn't after noticing that you'd used a default param instead of a nested method. You can appreciate how dubious it then looks when a subsequent edit then adds this alternative in your answer. – Kevin Wright May 15 '11 at 00:25

score 25 · Answer 2 · answered May 14 '11 at 23:31

It's not tail recursive (and can't ever be) because the final operation is not a recursive call to allStrings, it's a call to the :: method.

The safest way to resolve this is with a nested method that uses an accumulator:

def allStrings(expr: => String) = {
  @tailrec
  def inner(expr: => String, acc: List[String]): List[String] = expr match {
    case null => acc
    case w => inner(expr, w :: acc)
  }
  inner(expr, Nil)
}

In this particular case, you could also lift the accumulator to a parameter on allStrings, give it a default value of Nil, and avoid the need for an inner method. But that's not always possible, and it can't be called nicely from Java code if you're concerned about interop.

Isn't that code in tail recursive style?

2 Answers2

Linked