Skip to content
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

Duplicate method name&signature in class file (lambda on AnyVal subclass argument) #6260

Closed
scabug opened this issue Aug 20, 2012 · 23 comments
Closed
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented Aug 20, 2012

Lambdas with AnyVal extension arguments, where the single val is erased to Object, seem to produce invalid classes, which is only indicated when trying to load those classes. This is a simplified example.

> console-quick
[info] Starting scala interpreter...
[info]
Welcome to Scala version 2.10.0-M6 (Java HotSpot(TM) 64-Bit Server VM, Java 1.6.0_33).
Type in expressions to have them evaluated.
Type :help for more information.

scala> class Box[X](val x: X) extends AnyVal {
     |   def map[Y](f: X => Y): Box[Y] =
     |     ((bx: Box[X]) => new Box(f(bx.x)))(this)
     | }
defined class Box

scala> new Box(42) map (_ + 1)
java.lang.ClassFormatError: Duplicate method name&signature in class file Box$$anonfun$extension$map$1
        at java.lang.ClassLoader.defineClass1(Native Method)
        at java.lang.ClassLoader.defineClassCond(Unknown Source)
        at java.lang.ClassLoader.defineClass(Unknown Source)
        at java.lang.ClassLoader.defineClass(Unknown Source)
        at scala.tools.nsc.interpreter.AbstractFileClassLoader.findClass(AbstractFileClassLoader.scala:71)
        at java.lang.ClassLoader.loadClass(Unknown Source)
        at java.lang.ClassLoader.loadClass(Unknown Source)
        at Box$.extension$map(<console>:9)
        at .<init>(<console>:8)
        at .<clinit>(<console>)
        at .<init>(<console>:7)
        at .<clinit>(<console>)
        at $print(<console>)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
        at java.lang.reflect.Method.invoke(Unknown Source)
        at scala.tools.nsc.interpreter.IMain$ReadEvalPrint.call(IMain.scala:736)
        at scala.tools.nsc.interpreter.IMain$Request.loadAndRun(IMain.scala:991)
        at scala.tools.nsc.interpreter.IMain.loadAndRunReq$1(IMain.scala:579)
        at scala.tools.nsc.interpreter.IMain.interpret(IMain.scala:610)
        at scala.tools.nsc.interpreter.IMain.interpret(IMain.scala:574)
        at scala.tools.nsc.interpreter.ILoop.reallyInterpret$1(ILoop.scala:742)
        at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:787)
        at scala.tools.nsc.interpreter.ILoop.command(ILoop.scala:699)
        at scala.tools.nsc.interpreter.ILoop.processLine$1(ILoop.scala:563)
        at scala.tools.nsc.interpreter.ILoop.innerLoop$1(ILoop.scala:570)
        at scala.tools.nsc.interpreter.ILoop.loop(ILoop.scala:573)
        at scala.tools.nsc.interpreter.ILoop$$anonfun$process$1.apply$mcZ$sp(ILoop.scala:864)
        at scala.tools.nsc.interpreter.ILoop$$anonfun$process$1.apply(ILoop.scala:819)
        at scala.tools.nsc.interpreter.ILoop$$anonfun$process$1.apply(ILoop.scala:819)
        at scala.tools.nsc.util.ScalaClassLoader$.savingContextLoader(ScalaClassLoader.scala:135)
        at scala.tools.nsc.interpreter.ILoop.process(ILoop.scala:819)
        at scala.tools.nsc.interpreter.ILoop.main(ILoop.scala:886)
        at xsbt.ConsoleInterface.run(ConsoleInterface.scala:57)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
        at java.lang.reflect.Method.invoke(Unknown Source)
        at sbt.compiler.AnalyzingCompiler.call(AnalyzingCompiler.scala:73)
        at sbt.compiler.AnalyzingCompiler.console(AnalyzingCompiler.scala:64)
        at sbt.Console.console0$1(Console.scala:23)
        at sbt.Console$$anonfun$apply$2$$anonfun$apply$1.apply$mcV$sp(Console.scala:24)
        at sbt.TrapExit$.executeMain$1(TrapExit.scala:33)
        at sbt.TrapExit$$anon$1.run(TrapExit.scala:42)
// Box$$anonfun$extension$map$1 from javap
public final class Box$$anonfun$extension$map$1 extends scala.runtime.AbstractFunction1 implements scala.Serializable{
public static final long serialVersionUID;

public final java.lang.Object apply(java.lang.Object);
  Code:
   0:	aload_0
   1:	getfield	#23; //Field f$1:Lscala/Function1;
   4:	aload_1
   5:	invokeinterface	#27,  2; //InterfaceMethod scala/Function1.apply:(Ljava/lang/Object;)Ljava/lang/Object;
   10:	areturn

public final java.lang.Object apply(java.lang.Object);
  Code:
   0:	new	#33; //class Box
   3:	dup
   4:	aload_0
   5:	aload_1
   6:	checkcast	#33; //class Box
   9:	invokevirtual	#37; //Method Box.x:()Ljava/lang/Object;
   12:	invokevirtual	#38; //Method apply:(Ljava/lang/Object;)Ljava/lang/Object;
   15:	invokespecial	#42; //Method Box."<init>":(Ljava/lang/Object;)V
   18:	areturn

public Box$$anonfun$extension$map$1(scala.Function1);
  Code:
   0:	aload_0
   1:	aload_1
   2:	putfield	#23; //Field f$1:Lscala/Function1;
   5:	aload_0
   6:	invokespecial	#47; //Method scala/runtime/AbstractFunction1."<init>":()V
   9:	return

}
@scabug
Copy link
Author

scabug commented Aug 20, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6260?orig=1
Reporter: Stephen Compall (s11001001)
Affected Versions: 2.10.1-RC3, 2.11.0-M1

@scabug
Copy link
Author

scabug commented Sep 4, 2012

@paulp said:
This appears to be quite sticky and I can see it's not realistic for me to solve it soon.

final class Boxy(val x: Object) extends AnyVal { }
object Test {
  def crash(f: () => Boxy) = ()
  def main(args: Array[String]): Unit = crash(() => new Boxy(""))
}

The value classes want to generate bridge methods; so does Function0. They collide with the same signature in the closure.

@scabug
Copy link
Author

scabug commented Sep 11, 2012

@odersky said:
The duplicate method problem is fixed by scala/scala#1285.

@scabug
Copy link
Author

scabug commented Sep 16, 2012

@odersky said:
scala/scala#1313

@scabug
Copy link
Author

scabug commented Mar 13, 2013

@paulp said:

class X(val value: Object) extends AnyVal { def or(alt: => X): X = this }
class Y { def f = new X("") or new X("") }

// ./a.scala:2: error: bridge generated for member method apply: ()X in anonymous class $anonfun
// which overrides method apply: ()R in trait Function0
// clashes with definition of the member itself;
// both have erased type ()Object
// class Y { def f = new X("") or new X("") }
//                                ^
// one error found
% scalac3 -version
Scala compiler version 2.11.0-20130312-191920-c964643604 -- Copyright 2002-2013, LAMP/EPFL

@scabug
Copy link
Author

scabug commented Apr 10, 2013

@retronym said:
Test case from #7354

class Prob(val toN: Any) extends AnyVal
object Prob {
  (_: Prob).toN
}

@scabug
Copy link
Author

scabug commented Apr 25, 2013

@paulp said:
Here's another one, notable for having no closures, no by-name arguments, nothing beyond an abstract method being implemented. It's not possible for Option to be a value class without a solution to this.

final class Option[+A](val value: A) extends AnyVal

abstract class Foo[A]                { def f(): Option[A] }
         class Bar[A] extends Foo[A] { def f(): Option[A] = ??? }
./a.scala:7: error: bridge generated for member method f: ()Option[A] in class Bar
which overrides method f: ()Option[A] in class Foo
clashes with definition of the member itself;
both have erased type ()Object
         class Bar[A] extends Foo[A] { def f(): Option[A] = ??? }
                                           ^
one error found

@scabug
Copy link
Author

scabug commented Apr 25, 2013

@paulp said:
See https://github.com/paulp/scala/tree/issue/6260 for a change which makes a lot of these work, but lets through too much. Basically we are generating bridges where AFAICS no bridge is necessary. But the "double boxing" scenario must be recognized.

@scabug
Copy link
Author

scabug commented Jun 30, 2013

Andrzej Giniewicz (giniu) said:
I've found example very similar to Option example above, but I think even simpler. This was without realizing there was such ticket. I'm adding this as an example as suggested on mailing list.

class Wrapper(val value: Int) extends AnyVal
abstract class Test { def check(the: Wrapper): Boolean }
new Test { def check(the: Wrapper) = true }
error: bridge generated for member method check: (the: Wrapper)Boolean in anonymous class $anon
which overrides method check: (the: Wrapper)Boolean in class Test
clashes with definition of the member itself;
both have erased type (the: Int)Boolean
              new Test { def check(the: Wrapper) = true }
                             ^

@scabug
Copy link
Author

scabug commented Jun 30, 2013

@paulp said:
That example reinforces that we are generating unnecessary bridges. There is no possible basis for conflict.

@scabug
Copy link
Author

scabug commented Oct 21, 2013

@gkossakowski said:
I believe this is unfixable in general case. Let's consider the following code:

trait Function1[T,U]
class Foo(val x: AnyRef) extends AnyVal
object FooUnwrap extends Function1[Foo, AnyRef] {
  def apply(f: Foo): AnyRef = f.x
}

Now, let's consider a code that uses those classes and objects:

val fooStr = new Foo("str") // erases to val fooStr: String = "str"
val f: Function1[Foo, AnyRef] = FooUnwrap // erases to val f: Function1[AnyRef, AnyRef] = FooUnwrap
f.apply(fooStr) // erases to f.apply(new Foo(fooStr)) must dispatch to apply(AnyRef)AnyRef method
FooUnwrap.apply(fooStr) // erases to FooUnwrap.apply(fooStr) must dispatch to apply(AnyRef)AnyRef method

I've put erasure of each line in a comment. You can see that we have two calls to an apply where in one case we should unwrap the argument and in another one we shouldn't but the target (erased) signature is the same. Am I missing something?

@scabug
Copy link
Author

scabug commented Oct 25, 2013

@retronym said (edited on Oct 25, 2013 1:13:41 PM UTC):
In https://github.com/retronym/scala/tree/ticket/3452-rebased, I have a fix for the side issue that Paul reports:

final class Option[+A](val value: A) extends AnyVal
 
abstract class Foo[A]                { def f(): Option[A] }
         class Bar[A] extends Foo[A] { def f(): Option[A] = ??? }

These signatures were considered different wrt bridging because:

!(ErasedValueClass(Option[A(in Foo)]) =:= ErasedValueClass(Option[A(in Bar)]])

In my branch, I fix this in a slightly hacky way with a type map from ErasedValueClass[M[A]] to ErasedValueClass[M[scalaErasure(A)]. Importantly, this still recognises the cases when the bridge is needed:

final class Option[+A](val value: A) extends AnyVal

abstract class Foo1[A]                         { def f(): A }
         class Bar1[A] extends Foo1[Option[A]] { def f(): Option[A] = ??? }

I have a hunch we can move that into the normal erasure type map, rather than leaving it to posterasure.

@scabug
Copy link
Author

scabug commented Oct 25, 2013

@retronym said (edited on Oct 25, 2013 1:12:51 PM UTC):
Paul, do you still have your issue/6260 branch around? I'd be interested to compare approaches.

Incidentally, I was motivated to fix this problem because I'm extending bridging to Mixin to try to fix #3452, and that got tripped up when mixing DeprectedPredef.any2ArrowAssoc into Predef.

def any2ArrowAssoc[A](x: A): ArrowAssoc[A]

@scabug
Copy link
Author

scabug commented Oct 26, 2013

@paulp said:
I don't see a branch by that name anywhere, sorry.

Since you're poking at value class erasures, you might want to consider this one just for fun. It was brought up at PNW Scala that it would be a lot better if both these methods were specialized. But specialization is about as hip to the value class scene as is erasure.

class Meter(val m: Double) extends AnyVal

object Test {
  def fn1(f: Meter => Double)(x: Meter): Double = f(x)
  //  0: aload_1
  //  1: new           #16                 // class Meter
  //  4: dup
  //  5: dload_2
  //  6: invokespecial #19                 // Method Meter."<init>":(D)V
  //  9: invokeinterface #25,  2           // InterfaceMethod scala/Function1.apply:(Ljava/lang/Object;)Ljava/lang/Object;
  // 14: invokestatic  #31                 // Method scala/runtime/BoxesRunTime.unboxToDouble:(Ljava/lang/Object;)D
  // 17: dreturn

  def fn2(f: Double => Double)(x: Double): Double = f(x)
  // 0: aload_1
  // 1: dload_2
  // 2: invokeinterface #41,  3           // InterfaceMethod scala/Function1.apply$mcDD$sp:(D)D
  // 7: dreturn
}

@scabug
Copy link
Author

scabug commented Oct 26, 2013

@paulp said:
Oh that's funny, I'm already in that ticket. It came at me from a completely different direction and I didn't notice where I was.

@scabug
Copy link
Author

scabug commented Oct 26, 2013

@paulp said:
Oh, no I'm not. Too many windows, or too many hallucinogens, or something. "That ticket" is #5611.

@scabug
Copy link
Author

scabug commented Oct 27, 2013

@retronym said:
SI-5611?! That gives me reason, once again, to simultaneously admire your ambition and question your sanity :)

@scabug
Copy link
Author

scabug commented Oct 28, 2013

@retronym said:
Here's my patch to avoid spurious bridges: scala/scala#3082. It only fixes the side issues brought up here, the core issue is more fundamental as grek summarized.

@scabug
Copy link
Author

scabug commented Jan 28, 2014

@retronym said:
We might be able to learn something from Java 8 lambdas to help out here.

https://github.com/retronym/java-8-function1/blob/master/README.md

What if we only created the generic signature for anoynmous functions, rather than a more specific signature in the subclass that will always be accessed via a bridge?

@scabug
Copy link
Author

scabug commented Jan 29, 2014

@retronym said:
I've implemented a scheme to allow this for anonymous classes:

https://github.com/retronym/scala/tree/ticket/6260

@scabug
Copy link
Author

scabug commented Jan 29, 2014

@retronym said:
scala/scala#3428

@scabug
Copy link
Author

scabug commented Feb 10, 2014

@gkossakowski said:
The PR has been merged which fixes the problem when it comes to anonymous classes. It won't help the situation like:

object MyLambda extends (ValueClass => ValueClass)

However, in majority of cases people care about lambdas so I'd mark this as fixed. Jason, WDYT?

@scabug scabug closed this as completed Feb 11, 2014
@scabug
Copy link
Author

scabug commented Feb 11, 2014

@retronym said:
Agreed, I've marked as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants