Skip to content

Don't emit forwarder in mirror class for bridge methods#6531

Merged
lrytz merged 1 commit intoscala:2.13.xfrom
lrytz:b10812
May 13, 2018
Merged

Don't emit forwarder in mirror class for bridge methods#6531
lrytz merged 1 commit intoscala:2.13.xfrom
lrytz:b10812

Conversation

@lrytz
Copy link
Copy Markdown
Member

@lrytz lrytz commented Apr 16, 2018

When overriding a (concrete) method in an object with a varying
signature, make sure only one static forwarder method is emitted in the
mirror class, not an additional one with the signature of the bridge.

class A { def f: Object = null }
object B extends A { override def f: String = "b" }

typeOf[B.type].membersBasedOnFlags(BRIDGE, METHOD) after erasure works
correctly in that it doesn't return the bridge in B, but it returns
both A.f and B.f. This commit makes it run in an earlier phase.

Fixes scala/bug#10812

@scala-jenkins scala-jenkins added this to the 2.12.7 milestone Apr 16, 2018
@lrytz lrytz force-pushed the b10812 branch 2 times, most recently from cdf1663 to 19717bf Compare April 17, 2018 15:44
@lrytz lrytz requested a review from retronym April 17, 2018 15:44
@lrytz lrytz changed the base branch from 2.12.x to 2.13.x April 17, 2018 16:03
When overriding a (concrete) method in an object with a varying
signature, make sure only one static forwarder method is emitted in the
mirror class, not an additional one with the signature of the bridge.

    class A { def f: Object = null }
    object B extends A { override def f: String = "b" }

`typeOf[B.type].membersBasedOnFlags(BRIDGE, METHOD)` after erasure works
correctly in that it doesn't return the bridge in B, but it returns
both `A.f` and `B.f`. This commit makes it run in an earlier phase.
@lrytz
Copy link
Copy Markdown
Member Author

lrytz commented Apr 17, 2018

changed base to 2.13.x

@lrytz lrytz modified the milestones: 2.12.7, 2.13.0-M4 Apr 17, 2018
@lrytz
Copy link
Copy Markdown
Member Author

lrytz commented May 10, 2018

ping for review, @retronym

@lrytz lrytz merged commit f9583bb into scala:2.13.x May 13, 2018
lrytz added a commit to lrytz/scala that referenced this pull request Aug 8, 2018
eed3si9n added a commit to eed3si9n/scala that referenced this pull request Aug 11, 2018
Fixes scala/bug#11061
Ref scala/bug#10812

On 2.13.x branch scala#6531 removed the forwarder for bridge methods. I would like to do same in 2.12.x since Java 11-ea started to find them ambiguous as seen in akka/akka-core#25449 / scala/bug#11061.

To keep binary compatibility, I am still emitting the forwarder for bridge methods, but with `ACC_BRIDGE` flag.
eed3si9n added a commit to eed3si9n/scala that referenced this pull request Aug 20, 2018
Fixes scala/bug#11061
Ref scala/bug#10812

On 2.13.x branch scala#6531 removed the mirror class forwarders for bridge methods. I would like to do same in 2.12.x since Java 11-ea started to find them ambiguous as seen in akka/akka-core#25449 / scala/bug#11061.

To keep binary compatibility, I am still emitting the forwarders for bridge methods, but with `ACC_BRIDGE` flag.
eed3si9n added a commit to eed3si9n/scala that referenced this pull request Aug 20, 2018
Fixes scala/bug#11061
Ref scala/bug#10812

On 2.13.x branch scala#6531 removed the mirror class forwarders for bridge methods. I would like to do same in 2.12.x since Java 11-ea started to find them ambiguous as seen in akka/akka-core#25449 / scala/bug#11061.

To keep binary compatibility, I am still emitting the forwarders for bridge methods, but with `ACC_BRIDGE` flag.
@SethTisue
Copy link
Copy Markdown
Member

see also scala/bug#11061

lrytz added a commit to lrytz/scala that referenced this pull request Nov 27, 2018
In 2.12.6 and before, the Scala compiler emits static forwarders for
bridge methods in top-level modules. These forwarders are emitted by
mistake, the filter to exclude bridges did not work as expected. These
bridge forwarders make the Java compiler on JDK 11 report ambiguity
errors when using static forwarders (scala/bug#11061).

PR scala#7035 fixed this for 2.12.7 by adding the `ACC_BRIDGE` flag to static
forwarders for bridges. We decided to keep these bridges for binary
compatibility.

However, the new flag causes the eclipse Java compiler (and apparently
also IntelliJ) to report ambiguity errors when using static forwarders
(scala/bug#11271).

In 2.13.x the Scala compiler no longer emits static forwarders for
bridges (PR scala#6531). This PR brings the same behavior to 2.12.8.

This change breaks binary compatibility. However, in the examples we
tested, the Java compiler emits references to the non-bridge methods,
so compiled code continues to work if a library is replaced by a new
version that doesn't have forwarders for bridges:

```
$> cat T.scala
class A[T] {
  def get: T = ???
}
object T extends A[String] {
  override def get: String = "hi"
}

$> ~/scala/scala-2.12.7/bin/scalac T.scala
```

Generates two forwarders in `T.class`

```
// access flags 0x49
public static bridge get()Ljava/lang/Object;

// access flags 0x9
public static get()Ljava/lang/String;
```

```
$> javac -version
javac 1.8.0_181

$> cat Test.java
public class Test {
  public static void main(String[] args) {
    System.out.println(T.get());
  }
}

$> javac Test.java
```

Generates in Test.class
```
  INVOKESTATIC T.get ()Ljava/lang/String;
```
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.

4 participants