Scala Programming Language
  1. Scala Programming Language
  2. SI-5788

Specialized trait with final recursive method explodes during lambdalift

    Details

      Description

      This relatively small test case blows up during "lambdalift":

      trait Foo[@specialized(Int) A] {
        final def bar(a:A):A = bar(a)
      }
      

      There are a lot of variants that do compile properly. By removing specialized, or final, you can get code that compiles. Of course, this code will just overflow the stack, but the bug seems to affect any recursive final method, not just pointless ones.

      I ran into this issue when trying to update https://github.com/non/spire to work with 2.10.0.

      I've attached a test source file and the error log.

      1. error_20120814.log
        16 kB
        Adam Klein
      2. error.log
        21 kB
        Erik Osheim
      3. test.scala
        0.1 kB
        Erik Osheim

        Activity

        Hide
        Vlad Ureche added a comment -

        Thanks Adam! Then I'll revert the hack and put in the full patch in 2.10.x.

        I'm a bit confused as to why it passed the testsuite without this tesecase working. I'll have a look into this too, after I finish debugging the recent inliner crashes.

        Show
        Vlad Ureche added a comment - Thanks Adam! Then I'll revert the hack and put in the full patch in 2.10.x. I'm a bit confused as to why it passed the testsuite without this tesecase working. I'll have a look into this too, after I finish debugging the recent inliner crashes.
        Hide
        Vlad Ureche added a comment -

        Status update: I prepared the pull request but there is a problem in the backend: the closure elimination phase incorrectly eliminates some code and this leads to a failed test:

        $ cat test/files/run/tailcalls-base2.scala 
        // This test checks correct handling of the this pointer in the tailcalls transformation
        trait Base {
          def message: String
          class T(i: Int) { println(message) ; def msg = message }
        
          @annotation.tailrec final /* make sure it's transformed by tailcalls */
          def tailCall(a:Int, other: Base): T =
            if (a == 0) 
              new T(0)
            else 
              other.tailCall(a-1, this) // tail call-transformed
        }
        
        class ReturnsInt extends Base { def message = this.getClass.toString }
        class ReturnsStr extends Base { def message = this.getClass.toString }
        
        object Test extends App {
          val rInt = new ReturnsInt
          val rStr = new ReturnsStr
        
          for (x <- 1 to 10)
            println(rInt.tailCall(x, rStr).msg)
        }
        
        $ build/quick/bin/scalac test/files/run/tailcalls-base2.scala -d classes
        $ build/quick/bin/scala -cp classes Test
        class ReturnsStr
        class ReturnsStr
        class ReturnsInt
        class ReturnsInt
        class ReturnsStr
        class ReturnsStr
        class ReturnsInt
        class ReturnsInt
        class ReturnsStr
        class ReturnsStr
        class ReturnsInt
        class ReturnsInt
        class ReturnsStr
        class ReturnsStr
        class ReturnsInt
        class ReturnsInt
        class ReturnsStr
        class ReturnsStr
        class ReturnsInt
        class ReturnsInt
        
        $ build/quick/bin/scalac test/files/run/tailcalls-base2.scala -d classes -optimize
        $ build/quick/bin/scala -cp classes Test
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        class ReturnsStr
        
        Show
        Vlad Ureche added a comment - Status update: I prepared the pull request but there is a problem in the backend: the closure elimination phase incorrectly eliminates some code and this leads to a failed test: $ cat test/files/run/tailcalls-base2.scala // This test checks correct handling of the this pointer in the tailcalls transformation trait Base { def message: String class T(i: Int) { println(message) ; def msg = message } @annotation.tailrec final /* make sure it's transformed by tailcalls */ def tailCall(a:Int, other: Base): T = if (a == 0) new T(0) else other.tailCall(a-1, this) // tail call-transformed } class ReturnsInt extends Base { def message = this.getClass.toString } class ReturnsStr extends Base { def message = this.getClass.toString } object Test extends App { val rInt = new ReturnsInt val rStr = new ReturnsStr for (x <- 1 to 10) println(rInt.tailCall(x, rStr).msg) } $ build/quick/bin/scalac test/files/run/tailcalls-base2.scala -d classes $ build/quick/bin/scala -cp classes Test class ReturnsStr class ReturnsStr class ReturnsInt class ReturnsInt class ReturnsStr class ReturnsStr class ReturnsInt class ReturnsInt class ReturnsStr class ReturnsStr class ReturnsInt class ReturnsInt class ReturnsStr class ReturnsStr class ReturnsInt class ReturnsInt class ReturnsStr class ReturnsStr class ReturnsInt class ReturnsInt $ build/quick/bin/scalac test/files/run/tailcalls-base2.scala -d classes -optimize $ build/quick/bin/scala -cp classes Test class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr class ReturnsStr
        Hide
        Vlad Ureche added a comment -

        It's enough to run with -Yclosure-elim, no need for -optimize. I'll be looking at this tonight.

        Show
        Vlad Ureche added a comment - It's enough to run with -Yclosure-elim, no need for -optimize. I'll be looking at this tonight.
        Hide
        Vlad Ureche added a comment -

        I made another attempt to fix the problem without changing tailcalls: https://github.com/scala/scala/pull/1161/files
        The full patch is still in the works, my fix for the problem above generated incorrect bytecode, so there's no way it's going into 2.10. It will also be better if we keep the current design, as the full patch generates more JVM instructions and I fear previously-JITted methods will now be interpreted because they're now larger.

        Show
        Vlad Ureche added a comment - I made another attempt to fix the problem without changing tailcalls: https://github.com/scala/scala/pull/1161/files The full patch is still in the works, my fix for the problem above generated incorrect bytecode, so there's no way it's going into 2.10. It will also be better if we keep the current design, as the full patch generates more JVM instructions and I fear previously-JITted methods will now be interpreted because they're now larger.
        Hide
        Vlad Ureche added a comment -

        The patch is now in trunk. I'll close the bug but feel free to reopen in there are any problems.

        Show
        Vlad Ureche added a comment - The patch is now in trunk. I'll close the bug but feel free to reopen in there are any problems.

          People

          • Assignee:
            Vlad Ureche
            Reporter:
            Erik Osheim
          • Votes:
            1 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development