scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

@tailrec fails silently when used inside short-circuited boolean expression #8657

Closed scabug closed 10 years ago

scabug commented 10 years ago

At least that's the most specific explanation that I've found yet.

From https://gist.github.com/jrudolph/d5213d32d425fce0d88a:

  def run() =
    {
      @tailrec def rec(count: Long): Boolean =
        if (count == 0) true else rec(count - 1)

      rec(1000000L)
    } && true

Here's the original report: https://groups.google.com/d/msg/parboiled-user/1gZ8ffmTNe8/yth0_8xU07gJ

Parboiled2 relies heavily on expressions like this.

scabug commented 10 years ago

Imported From: https://issues.scala-lang.org/browse/SI-8657?orig=1 Reporter: @jrudolph Assignee: @jrudolph Affected Versions: 2.10.4, 2.11.1

scabug commented 10 years ago

@jrudolph said: That's not the only miss. From glancing over the code there are still other expression positions which are not even searched for inner methods with potential tailcalls. E.g. condition of an If expression. I think one reason is that the code tries to find methods and tailcalls inside of methods at the same time.

scabug commented 10 years ago

@paulp said: @tailrec was one of my very first commits to the compiler, and it has barely been touched since that time - put it all together and it spells... yeah.

 commit 67c3c68da5
 Author: Paul Phillips <paulp@improving.org>
 Date:   5 years ago

     The birth of the @switch and @tailrec annotations.
scabug commented 10 years ago

@jrudolph said: Cool :)

Actually, this wasn't so much about the annotation but more about the optimization itself. It turns out one part of the bug (If condition) was already introduced in the original introduction of tailcall optimization in 2005:

commit b4ba0b8045dc81357d4124b4913db0531665e54e
Author: Iulian Dragos <jaguarul@gmail.com>
Date:   Tue Jun 28 08:52:00 2005 +0000

    Added tail call elimination phase to nsc.

and the other part (left operand of && / ||) was added in 2007:

commit a694dd57cc8e0dadd3a1b7ec30a18dc4739c16f0
Author: Iulian Dragos <jaguarul@gmail.com>
Date:   Mon Mar 19 17:46:30 2007 +0000

    Catch tail calls made from tail boolean or.

So much for the history...

The fix was merged as https://github.com/scala/scala/pull/3832