Skip to content

Fix code gen for Outer.super[Q].foo #5944

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

Merged
merged 1 commit into from
Jun 28, 2017
Merged

Fix code gen for Outer.super[Q].foo #5944

merged 1 commit into from
Jun 28, 2017

Conversation

lrytz
Copy link
Member

@lrytz lrytz commented Jun 14, 2017

Consider

class B extends T { class C { B.super[T].f }}

After flatten, that call is B$C.this.$outer().super[T].f().

In 2.11, mixin translates this to A$class.f(B$C.this.$outer()).
In 2.12, the tree is passed unchanged to the backend.

In genApply we assumed that in Apply(Select(Super(qual, ... ))),
qual is a This tree, so we just emitted ALOAD_0, which caused
the $outer() call to get lost. Now we invoke genLoad(qual).

Fixes scala/bug#10290.

@scala-jenkins scala-jenkins added this to the 2.12.3 milestone Jun 14, 2017
@lrytz lrytz requested a review from retronym June 14, 2017 13:03
@lrytz
Copy link
Member Author

lrytz commented Jun 14, 2017

  • TODO: jardiff that -- empty

@sjrd
Copy link
Member

sjrd commented Jun 14, 2017

Aaand yup, Scala.js has exactly the same bug, for exactly the same reason: scala-js/scala-js#3013 😄

@lrytz
Copy link
Member Author

lrytz commented Jun 14, 2017

@sjrd @densh you guys are so quick, I just wanted to notify you :)

@dwijnand
Copy link
Member

"code gen" is a trigger word :D

@@ -317,9 +317,12 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
}
else {
mnode.visitVarInsn(asm.Opcodes.ALOAD, 0)
generatedType =
if (tree.symbol == ArrayClass) ObjectRef
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This special case is old, it goes back to 0ad5e129f3. Note that on a current 2.12.x, that code path is never triggered when compiling library+compiler+reflect. With this PR we get here again, but Object doesn't work when the expected type is [Object.

Consider

    class B extends T { class C { B.super[T].f }}

After flatten, that call is ` B$C.this.$outer().super[T].f()`.

In 2.11, mixin translates this to `A$class.f(B$C.this.$outer())`.
In 2.12, the tree is passed unchanged to the backend.

In `genApply` we assumed that in `Apply(Select(Super(qual, ... )))`,
`qual` is a `This` tree, so we just emitted `ALOAD_0`, which caused
the `$outer()` call to get lost. Now we invoke `genLoad(qual)`.

Fixes scala/bug#10290.
@lrytz
Copy link
Member Author

lrytz commented Jun 15, 2017

jardiff is empty (library+reflect+compiler)

@retronym retronym merged commit 7f13681 into scala:2.12.x Jun 28, 2017
@lrytz lrytz deleted the t10290 branch July 20, 2017 14:28
liufengyun added a commit to dotty-staging/dotty that referenced this pull request Jul 10, 2020
The problem can be seen from the following example:

trait A { def foo() = ??? }
trait B { def foo() = ??? }

object C extends A with B {
	super[A].foo()
	super[B].foo()
}

In the code above, we cannot translate the following calls
from <init> to <clinit>:

  super[A].foo()
  super[B].foo()

  super[A].$iinit$()
  super[B].$init$()

More details can be found here: scala#5928

A principled way would be to generage super accessors as it
is done in posttyper.

However, the backend has a magic to support
prefix to super trees in [1], which is exploited
in the Scala 2 fix [2].

[1] scala/scala#5944
[2] scala/scala#7270
liufengyun added a commit to dotty-staging/dotty that referenced this pull request Jul 15, 2020
The problem can be seen from the following example:

trait A { def foo() = ??? }
trait B { def foo() = ??? }

object C extends A with B {
	super[A].foo()
	super[B].foo()
}

In the code above, we cannot translate the following calls
from <init> to <clinit>:

  super[A].foo()
  super[B].foo()

  super[A].$iinit$()
  super[B].$init$()

More details can be found here: scala#5928

A principled way would be to generage super accessors as it
is done in posttyper.

However, the backend has a magic to support
prefix to super trees in [1], which is exploited
in the Scala 2 fix [2].

[1] scala/scala#5944
[2] scala/scala#7270
liufengyun added a commit to dotty-staging/dotty that referenced this pull request Jul 15, 2020
liufengyun added a commit to dotty-staging/dotty that referenced this pull request Jul 21, 2020
The problem can be seen from the following example:

trait A { def foo() = ??? }
trait B { def foo() = ??? }

object C extends A with B {
	super[A].foo()
	super[B].foo()
}

In the code above, we cannot translate the following calls
from <init> to <clinit>:

  super[A].foo()
  super[B].foo()

  super[A].$iinit$()
  super[B].$init$()

More details can be found here: scala#5928

A principled way would be to generage super accessors as it
is done in posttyper.

However, the backend has a magic to support
prefix to super trees in [1], which is exploited
in the Scala 2 fix [2].

[1] scala/scala#5944
[2] scala/scala#7270
liufengyun added a commit to dotty-staging/dotty that referenced this pull request Jul 21, 2020
liufengyun added a commit to dotty-staging/dotty that referenced this pull request Jul 22, 2020
The problem can be seen from the following example:

trait A { def foo() = ??? }
trait B { def foo() = ??? }

object C extends A with B {
	super[A].foo()
	super[B].foo()
}

In the code above, we cannot translate the following calls
from <init> to <clinit>:

  super[A].foo()
  super[B].foo()

  super[A].$iinit$()
  super[B].$init$()

More details can be found here: scala#5928

A principled way would be to generage super accessors as it
is done in posttyper.

However, the backend has a magic to support
prefix to super trees in [1], which is exploited
in the Scala 2 fix [2].

[1] scala/scala#5944
[2] scala/scala#7270
liufengyun added a commit to dotty-staging/dotty that referenced this pull request Jul 22, 2020
liufengyun added a commit to dotty-staging/dotty that referenced this pull request Aug 3, 2020
The problem can be seen from the following example:

trait A { def foo() = ??? }
trait B { def foo() = ??? }

object C extends A with B {
	super[A].foo()
	super[B].foo()
}

In the code above, we cannot translate the following calls
from <init> to <clinit>:

  super[A].foo()
  super[B].foo()

  super[A].$iinit$()
  super[B].$init$()

More details can be found here: scala#5928

A principled way would be to generage super accessors as it
is done in posttyper.

However, the backend has a magic to support
prefix to super trees in [1], which is exploited
in the Scala 2 fix [2].

[1] scala/scala#5944
[2] scala/scala#7270
liufengyun added a commit to dotty-staging/dotty that referenced this pull request Aug 3, 2020
liufengyun added a commit to dotty-staging/dotty that referenced this pull request Aug 6, 2020
The problem can be seen from the following example:

trait A { def foo() = ??? }
trait B { def foo() = ??? }

object C extends A with B {
	super[A].foo()
	super[B].foo()
}

In the code above, we cannot translate the following calls
from <init> to <clinit>:

  super[A].foo()
  super[B].foo()

  super[A].$iinit$()
  super[B].$init$()

More details can be found here: scala#5928

A principled way would be to generage super accessors as it
is done in posttyper.

However, the backend has a magic to support
prefix to super trees in [1], which is exploited
in the Scala 2 fix [2].

[1] scala/scala#5944
[2] scala/scala#7270
liufengyun added a commit to dotty-staging/dotty that referenced this pull request Aug 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants