Incorrect bridge gen for method that has value class with underlying Object type as result type

[VladimirNik]

Incorrect bridge generation for method that has value class with the underlying Object type as result type.
The compilation of this code with -Ycheck:all:

class A(val x: Object) extends AnyVal
abstract class VCACompanion[T] {
  def yyyy(x: Object): T
}
class XXX extends VCACompanion[A] {
  override def yyyy(x: Object): A = new A("fff")
}

results in:

 method yyyy is already defined as method yyyy: (x: Object)ErasedValueType(A, Object)
 (the definitions have matching type signatures)

The problem is that there is a bridge generation during Erasure for def yyyy(x: Object): A = ....
This bridge has the same signature as original def yyyy(x: Object): A = ...:

override def yyyy(x: Object): Object = "fff"
def yyyy(x: Object): Object = new A(this.yyyy(x))

[DarkDimius]

Here is the diagnosis of this bug. It has nothing to do with VCCompanion and also happened in scalac.

class X(val underlying: Object) extends AnyVal

trait Producer[T] {
  def produce: T
}

class XProducer extends Producer[X] {
  def produce = new X(null)
}

Consider the method Producer.produce. I has erased signature: (): Object.
The implementation class XProducer intends to create an implementation (): X that will be transformed by value class transformation to def produce: Object.

This will lead to fact that we would have 2 methods with same signature but different semantics. One returns an X: Object as seen from superclass, the other returns x.underlying: Object, as seen from XProducer.

This problem is not specific to java.lang.Object. Given a universal trait UT, a value class

  class VC(u: UT) extends UT

is able to break bridge generation for methods that return UT or take UT.

The solution that I propose to this problem, is to have ExtensionMethods name mangle all methods that return instances of value classes. This solves issue, as there will no longer be a full name&signature clash. Those methods would need to be forwarded by a phase after erasure.

[DarkDimius]

Note, Scalac had similar bugs:
https://issues.scala-lang.org/browse/SI-6260
https://issues.scala-lang.org/browse/SI-9166
https://issues.scala-lang.org/browse/SI-6385

The fix that was deployed in scalac I believe is both harder to implement and brings performance slowdowns, as such value classes will get boxed more frequently scala/scala#3082

Note, that in https://issues.scala-lang.org/browse/SI-9166 @retronym proposes the same change that I'm proposing here.

[odersky]

I think name mangling is a can of worms, so would like to avoid it. The way I see it there are two options:

• Realize that a bridge is needed between the two process methods (currently we fail to do that) and issue an error at erasure because the bridge clashes with an existing method. I.e. reject the example.
• Or, do what scalac does. If a value class returning method implements a generic method in a base trait, always box the result.

The easiest route is probably to do what scalac does.

[VladimirNik]

Currently scalac rejects this example (2.11 and 2.12) with:

test.scala:8: error: bridge generated for member method produce: ()X in class XProducer
which overrides method produce: ()T in trait Producer
clashes with definition of the member itself;
both have erased type ()Object
  def produce = new X(null)
      ^
one error found

[odersky]

OK, I think that is acceptable.

On Fri, Mar 18, 2016 at 5:45 PM, Vladimir Nikolaev <[email protected]

wrote:

Currently scalac rejects this example (2.11 and 2.12) with:

test.scala:8: error: bridge generated for member method produce: ()X in class XProducer
which overrides method produce: ()T in trait Producer
clashes with definition of the member itself;
both have erased type ()Object
def produce = new X(null)
^
one error found

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#1169 (comment)

Martin Odersky
EPFL

[DarkDimius]

We have agreed in private discussion that we may reconsider.
This bug breaks assumptions of linker(marking class A as value class could break compilation of some other class B)

[odersky]

Related: #1905. We might come back to the idea of mangling, after all.