-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Duplicate symbol on bridge generation #1905
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
Comments
BTW: Here is scalac's behavior, which is also wrong (see SI-10150) iarray.scala:10: error: bridge generated for member method fromIterableWithSameElemType: ()Arr[A] in class ArrOps |
In this example there indeed should be two methods with same signature with our current compilation scheme:
Both those methods have same names and their entire JVM signature is the very same. This can be fixed by making value-classes method be name-mangled, ie method that returns |
The problem with scala#1905 was that we checked "new" overriding relationships are phase erasure + 1. This is wrong when we have ErasedValueTypes because these do not compare correctly ith their underlying type. So we get spurious mismatches which force in turn spurious bridge generation. The fix is to compute newly overridden symbols at phase elimErasedValueTypes + 1, i.e. one phase later.
Note that you cant skip generation of the bridge, as callsites that observe the instance of the class as |
@DarkDimius I don't understand. According to Scala's typing rules, these methods are in an overriding relationship (both scalac and dotc agree on this). Furthermore, they have the same erasure, so overriding also holds on the JVM level. So, no bridge should be generated. |
No, the erasure of Arr[A] is Object, not A. |
What I've ment here was observable semantics, not erasure:
Those two methods have different semantics, but the same erazed form, which means that in runtime they will be dispatch to same location that should follow both semantics, and this isn't possible as semantics contradict. |
No, please have a look at the signature of the value class again. This is simply wrong. |
Fixed a typo, but the message stays the same |
But Repr is scala.Array[T]! I still don't understand. Are you claiming that the two |
Consider this, simpler example:
how many methods should there be defined and implemented in
Note that two last methods clash on erasure. |
Repr isn't scala.Array[T], Repr is |
OK, can you come up with a counter example that breaks semantics? Just take #1906 and construct a crashing scenario. I think that would help me understand what you mean. |
class Arr[T](val underlying: scala.Array[T]) extends AnyVal {}
abstract class SeqMonoTransforms[+A, +Repr] {
protected[this] def fromIterableWithSameElemType(): Repr
def getFIWSET: Repr = fromIterableWithSameElemType
}
class ArrOps[A](val xs: Arr[A]) extends SeqMonoTransforms[A, Arr[A]] {
def fromIterableWithSameElemType(): Arr[A] = xs
}
object Test {
def main(args: Array[String]) = {
val t = new ArrOps(new Arr(Array(1, 2, 3)))
val t2 = t.getFIWSET
}
}
|
@DarkDimius Ah, that helps! What do you get when compiling with master? |
Thanks for the counter-example. I think I get it now. The problem is that erasure assumes that if a value x is of type Object and it needs to be cast to a value class such as Arr[T], then x must have the boxed representation of Arr[T] at runtime. But without adding the bridge we return directly the unboxed representation and that leads to the classcast error. But we still need to detect the problem (it seems that right now dotc does not). Also, would it be possible to have a more refined test at runtime? I see we currently generate this:
Could we instead generate this?
And, would that solve the problem? |
Unforturnatelly you cant(what if one class is a sublass of another)? There's a small change to the compilation scheme of value classes that would permit the code you want to compie: we need to name-mangle the name of |
You mean, the value class is a subclass of its underlying class? Do we have usecases for this? Could we simply forbid it? |
@odersky, similarly, you cant forbid it statically(without a closed world), it can be not be observed locally. Consider my example(with Super and Child). The
|
I retract the idea. I believe value classes over
not compile. But parametric value classes such as
are common, and they pose the same problem. Can you explain in more detail the name mangling idea? |
The problem arises from the fact that two methods with different semantics have same signature. Signature includes name an argument&result type. We can make them be unique if we change the name. The proposed scheme would be:
This means that a
Similarly, in my example id(a: A): A will be compiled into three methods:
This can either be implemented in Erasure or by a phase preceding erasure(I'd prefer this). he phase would also change callsites that know that they call a method that takes an unboxed value class to dispatch to freshly-created method. |
What happens if we have several unboxed arguments and an unboxed return type? Would we have to generate several methods? If not, why not simply append a $vr to the method name, if it has at least one unboxed argument or an unboxed return type? |
I prefer @DarkDimius's proposal (to treat optimized calls to methods with value classes and rewrite calls sites, ala specialization) to the current approach. I think we discussed before the release of Scala 2.10, but it was seen as to hard to implement in scalac. One thing to keep in mind is how to make these rewrites for value classes, specialization, and implicit function types composable. Ideally they could be unified somehow. |
Yes, I am also coming around to think that name mangling is a good solution, if we can keep it simple & composable.
Which aspect of implicit function types did you have in mind here? |
I think I can answer my own question: Because several subclasses might have overriding methods with different value class parameters or results. We need a more fine-grained naming scheme to prevent accidental clashes between them. |
As value classes are final, every argument may become value class only once(and will then stay a value class). This ensures absence of clashes as well as some sort of optimality. |
I recall part of the proposal that rewrites |
@retronym Ah, res, |
As a first measure, we still need to detect the duplicate bridge issue (scalac does it but dotty does not). @DarkDimius, can you look into this? |
The missing error message is fixed by aa7ee91. The possible name mangling to avoid the error is left for future work. |
Fix #1905: Detect case where bridge would clash with the member
We get a duplicate generated symbol for
fromIterableWithSameElemType
in classArrOps
when compiling iarray.scala:If we compile this with -Ycheck:erasure, we get:
exception occurred while compiling iarray.scala
Exception in thread "main" class dotty.tools.dotc.reporting.diagnostic.messages$Error at iarray.scala:9: method fromIterableWithSameElemType is already defined as method fromIterableWithSameElemType: ()ErasedValueType(Arr, Object)
(the definitions have matching type signatures)
at dotty.tools.dotc.reporting.Reporting$class.error(Reporter.scala:85)
at dotty.tools.dotc.core.Contexts$Context.error(Contexts.scala:57)
at dotty.tools.dotc.typer.Checking$$anonfun$checkDecl$1$1.doubleDefError$1(Checking.scala:548)
at dotty.tools.dotc.typer.Checking$$anonfun$checkDecl$1$1.apply(Checking.scala:551)
at dotty.tools.dotc.typer.Checking$$anonfun$checkDecl$1$1.apply(Checking.scala:540)
at scala.collection.immutable.List.foreach(List.scala:381)
The text was updated successfully, but these errors were encountered: