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
Specialized trait with final recursive method explodes during lambdalift #5788
Comments
Imported From: https://issues.scala-lang.org/browse/SI-5788?orig=1
|
@non said: |
@VladUreche said: |
@non said: |
@VladUreche said: |
@som-snytt said: I'll be curious to see the rest of the fix if there's more; you don't just get the enclosing context? Sorry I didn't get it done. :) |
@VladUreche said: The problem in our case was that new symbols were passed as parameters to the LabelDef. Later, the erasure phase would update the symbol owners and types, but would skip LabelDef parameters, because they should have been updated at their definition site (which, normally, is before the LabelDef). Finally, this owner mismatch was crashing Lambdalift, because the LabelDef parameters had the pre-erasure owner chains (Foo instead of Foo$class). It's the first time I use -Ybrowse:, thanks for pointing it out. I used -uniqid, -Xprint: and -Xshow-syms for debugging. And the plain old assert(id != <...>) in Symbols.scala :p I committed the patch to [https://github.com/VladUreche/scala/tree/issue/5788]. (it passed the testsuite on my machine) @erik: |
@non said: |
@VladUreche said: |
@non said: Using your branch, I can compile Spire with specialization and it runs the benchmark at the speed I expect. Looks good to me! |
@VladUreche said: |
@VladUreche said (edited on May 16, 2012 12:48:05 PM UTC): |
@som-snytt said: |
@VladUreche said: |
@som-snytt said: |
@non said: Is the plan still to try to change the tailcalls behavior? Is this something that can be fixed for 2.10.0? |
@VladUreche said: It won't compile the following case, complaining it's expecting trait Test {
trait B[T]
private final def grow[T](): B[T] = grow[T]()
} |
@non said: I was on vacation last week but may try to work on this since it's so important to me. |
@VladUreche said: |
@non said: |
@VladUreche said: If all goes well, I'll revert the quick fix and add the complete one in another pull request once 2.10.x is merged into master. |
Adam Klein (aklein-at-novus.com) said: |
@VladUreche said: |
@VladUreche said: |
Adam Klein (aklein-at-novus.com) said (edited on Aug 14, 2012 4:32:22 PM UTC): (yes, I am running the test case from the original parent) |
@VladUreche said: 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. |
@VladUreche said: $ 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 |
@VladUreche said: |
@VladUreche said: |
@VladUreche said: |
This relatively small test case blows up during "lambdalift":
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.
The text was updated successfully, but these errors were encountered: