Skip to content
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

tailrec complains about suspended call #20012

Closed
som-snytt opened this issue Mar 25, 2024 · 5 comments
Closed

tailrec complains about suspended call #20012

som-snytt opened this issue Mar 25, 2024 · 5 comments

Comments

@som-snytt
Copy link
Contributor

Compiler version

3.4.2

Minimized code

final class C {
  def g(f: Int => Int) = 42

  @annotation.tailrec
  def f(i: Int): Int =
    if (i > 0) f(i-1)
    else g(f)
}

similarly or originally with by-name arguments:

@annotation.tailrec
def lazyFilter[E](s: Stream[E], p: E => Boolean): Stream[E] = s match {
  case h #:: t => if (p(h)) h #:: lazyFilter(t, p) else lazyFilter(t, p)
}

or

import annotation.tailrec

object Test {
  @tailrec
  def lazyFilter[E](s: LazyList[E], p: E => Boolean): LazyList[E] = s match {
    case h #:: t => if (p(h)) h #:: lazyFilter(t, p) else lazyFilter(t, p)
  }
}

Apologies for the extra import. If only it were not required.

Output

8 |    else g(f)
  |           ^
  |           Cannot rewrite recursive call: it is not in tail position

Expectation

Since the expression is just passing the lambda, it should not complain.

The "strict" behavior might be nice to have under a lint. "Leaking" a call in this way might be dangerous.

Alternative expectation is that you only use @tailrec when in peril of stack overflow, so you want maximum warnage; if you don't like that, leave it off.

Reported as scala/bug#4649

@som-snytt som-snytt added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 25, 2024
@sjrd
Copy link
Member

sjrd commented Mar 25, 2024

That seems to behave as designed to me. Both g and #:: could directly call their function argument, resulting in a non tail recursive behavior.

@som-snytt
Copy link
Contributor Author

There's always a recurrent danger of recursion. The suggestion is that it should warn for references I can do something about. But on the other ticket, I said it's a matter of expectations.

@LucySMartin
Copy link

While I do see a value in some sort of softer tail recursion check, I think a change here would be quite harmful to how a lot of things use this.
A quite common pattern would be to pass a lambda into another method such as:

def recurse(i: Int):Int = {
  processing(i)
  if (test(i))
    with_performance_monitor(_ => recurse(i-1))
  else 
    i
}

clearly in this case invoking the performance monitor breaks tail recursion, which we do need something to protect against.

I think if we wanted to support something like this, it would need to be new functionality rather than an alteration to @tailrec

@som-snytt
Copy link
Contributor Author

Maybe it needs @notailrec for offending expressions.

For the scala 2 ticket, I added -Wstrict-tailrec, but @tailrec should just mean what it says and not rely on an additional (invisible) knob.

The concern is that "I need to write something reasonable, but then I can't use @tailrec to ensure that my other reasonable usages are in fact reasonable."

Another example is assert(condition, s"I would have computed ${f(n)}").

Note that tailrec requires an existing tail call, so the example of "I wrapped all my tail calls" would still error.

I'll leave the ticket open for the suppressing annotation.

@Gedochao Gedochao added itype:invalid area:annotations stat:wontfix and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 3, 2024
@Gedochao
Copy link

Gedochao commented Apr 3, 2024

Seems like the consensus is this works as intended.
@som-snytt A suppressing annotation feature request can be raised separately.
I'm closing this.

@Gedochao Gedochao closed this as not planned Won't fix, can't repro, duplicate, stale Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants