Skip to content

targetName with AnyVal affects multiple methods, which can cause invalid bytecode #16464

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

Closed
v6ak opened this issue Dec 4, 2022 · 0 comments · Fixed by #16487
Closed

targetName with AnyVal affects multiple methods, which can cause invalid bytecode #16464

v6ak opened this issue Dec 4, 2022 · 0 comments · Fixed by #16487

Comments

@v6ak
Copy link

v6ak commented Dec 4, 2022

Compiler version

3.2.1

Minimized code

import scala.annotation.targetName

implicit final class SomeOps(e: Int) extends AnyVal:
  @targetName("a")
  def -(other: Seq[Int]) = ???
  @targetName("b")
  def -(other: Seq[Long]) = ???

@main
def main(): Unit = 1 - Seq.empty[Int]

Output

Exception in thread "sbt-bg-threads-19" java.lang.ClassFormatError: Duplicate method name "b" with signature "(ILscala.collection.immutable.Seq;)Lscala.runtime.Nothing$;" in class file main$package$SomeOps$
	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
…

Brief analysis

The implementing class seems to use name "b" for both methods:

~/AnyVal-targetName-bug% javap -cp target/scala-3.2.1/classes 'main$package$SomeOps$' | grep Seq
  public final scala.runtime.Nothing$ b(int, scala.collection.immutable.Seq<java.lang.Object>);
  public final scala.runtime.Nothing$ b(int, scala.collection.immutable.Seq<java.lang.Object>);

The bytecode of the wrapper class seems to be valid, but even the a method calls the main$package$SomeOps$.b:(ILscala/collection/immutable/Seq;)Lscala/runtime:

% javap -c -cp target/scala-3.2.1/classes 'main$package$SomeOps' | grep Seq --after=100
  public scala.runtime.Nothing$ a(scala.collection.immutable.Seq<java.lang.Object>);
    Code:
       0: getstatic     #19                 // Field main$package$SomeOps$.MODULE$:Lmain$package$SomeOps$;
       3: aload_0
       4: invokevirtual #39                 // Method main$package$SomeOps$$e:()I
       7: aload_1
       8: invokevirtual #51                 // Method main$package$SomeOps$.b:(ILscala/collection/immutable/Seq;)Lscala/runtime/Nothing$;
      11: areturn

  public scala.runtime.Nothing$ b(scala.collection.immutable.Seq<java.lang.Object>);
    Code:
       0: getstatic     #19                 // Field main$package$SomeOps$.MODULE$:Lmain$package$SomeOps$;
       3: aload_0
       4: invokevirtual #39                 // Method main$package$SomeOps$$e:()I
       7: aload_1
       8: invokevirtual #51                 // Method main$package$SomeOps$.b:(ILscala/collection/immutable/Seq;)Lscala/runtime/Nothing$;
      11: areturn
}

Expectation

I expect:

  1. the generated bytecode of main$package$SomeOps$ to be valid and
  2. the app to produce scala.NotImplementedError: an implementation is missing and
  3. the generated bytecode of main$package$SomeOps.a(scala.collection.immutable.Seq) to call a, not b. (i.e., main$package$SomeOps$.a:(ILscala/collection/immutable/Seq;)Lscala/runtime, not main$package$SomeOps$.b:(ILscala/collection/immutable/Seq;)Lscala/runtime)
@v6ak v6ak added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Dec 4, 2022
@Kordyjan Kordyjan added compat:java and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Dec 5, 2022
odersky added a commit to dotty-staging/dotty that referenced this issue Dec 9, 2022
…ue classes

Before target name we only matched on signatures. This was OK, since multiple extension
methods of the same class must be different, otherwise we will get a "have the same erasure"
error later at erasurePhase. But with @TargetNAME that's now a legal situation that
needs to be resolve correctly. We do this by propagating the target name to the extension
method and verifiying that the target names of the original and extension methods match.

Fixes scala#16464
odersky added a commit to dotty-staging/dotty that referenced this issue Dec 9, 2022
…ue classes

Before target name we only matched on signatures. This was OK, since multiple extension
methods of the same class must be different, otherwise we will get a "have the same erasure"
error later at erasurePhase. But with @TargetNAME that's now a legal situation that
needs to be resolve correctly. We do this by propagating the target name to the extension
method and verifiying that the target names of the original and extension methods match.

Fixes scala#16464
smarter added a commit that referenced this issue Dec 12, 2022
Take @TargetNAME into account when resolving extension methods of value
classes

Before target name we only matched on signatures. This was OK, since
multiple extension methods of the same class must be different,
otherwise we will get a "have the same erasure" error later at
erasurePhase. But with @TargetNAME that's now a legal situation that
needs to be resolved correctly. We do this by propagating the target
name to the extension method and verifying that the target names of the
original and extension methods match.

Fixes #16464
little-inferno pushed a commit to little-inferno/dotty that referenced this issue Jan 25, 2023
…ue classes

Before target name we only matched on signatures. This was OK, since multiple extension
methods of the same class must be different, otherwise we will get a "have the same erasure"
error later at erasurePhase. But with @TargetNAME that's now a legal situation that
needs to be resolve correctly. We do this by propagating the target name to the extension
method and verifiying that the target names of the original and extension methods match.

Fixes scala#16464
@Kordyjan Kordyjan added this to the 3.3.0 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants