-
Notifications
You must be signed in to change notification settings - Fork 21
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
Static methods in objects extending an abstract class, or classes with covariant-overriding mxins, get inconsistent java generics signature #3452
Comments
Imported From: https://issues.scala-lang.org/browse/SI-3452?orig=1 |
@paulp said: |
@jrudolph said: trait Search[M] {
def search(input: M): C[Int] = null
}
object StringSearch extends Search[String]
trait C[T] and public class JavaApp {
public static void main(String[] args) {
StringSearch.search("test");
}
} In a strange way the error depends on the return value being an applied type (strange because the error appears in the parameter position). The problem is actually catched using the src/main/scala/Test.scala:5: warning: compiler bug: created generic signature for method search in StringSearch that does not conform to its erasure
signature: (Ljava/lang/String;)LC<Ljava/lang/Object;>;
original type: (input: String)C[Int]
normalized type: (input: String)C[Object]
erasure type: (input: Object)C
if this is reproducible, please report bug at http://lampsvn.epfl.ch/trac/scala
object StringSearch extends Search[String]
^
one warning found |
DaveScala (davescala) said: package types
trait IStringPair[T] {
def a : String
def b : String
def build(a : String, b : String) : T
def cat(that : IStringPair[T]) = build(this.a + that.a, this.b + that.b)
override def toString = a + b
}
class StringPair(val a : String, val b : String) extends IStringPair[StringPair] {
def build(a : String, b : String) = new StringPair(a, b)
def len = a.length + b.length
}
|
@paulp said: |
@paulp said: |
DaveScala (davescala) said: |
@jrudolph said: It may be sensible to enable this flag/code path by default if this can't be fixed soon or is still likely to be broken in some cases. That would mean that a) you can use these methods at all from Java even if the compiler still has a bug when generating them b) you are loosing generic type info at the Java side because the generic type is missing. Compare that to the current situation where although Java can compile against the generic type, the call fails at runtime. |
@paulp said: BTW I don't see the warning regardless of -Ycheck, I don't know why. I know it's generating the bad signatures. |
@jrudolph said: For the new case Dave gave this is obviously not working because the important part is that the method descriptor actually implements the Java interface correctly. So, the generation of the generic signature has to be fixed in this case to match the descriptor. Maybe in this case we should just copy the overwritten method's signature over to the method implementing the method. It's strange that you're not seeing the warning, it's there in the code, isn't it?. Dave seemed to be working with Scala 2.10.0-M2, right? In my unorderly ways I'm often on some version, where in this case it was f987afe which is master from two weeks ago. |
@paulp said:
That means it's -Ycheck:jvm, and -Ycheck:genjvm refers to a non-existent phase. The options which take a phase argument should probably fail on mystery strings, but right now they silently ignore. Still, you know if it checked by examining the output: it will always say "[Now checking: jvm]" if it understood you, and it will be silent if it did not. |
@paulp said: https://github.com/paulp/awesome Look how beautiful: This one is especially compelling given that that's all the necessary logic: |
@Blaisorblade said: More in detail: In a moment I'll create a pull request which will reference this comment.
|
@Blaisorblade said: |
@Blaisorblade said: However, the number of times the warning triggers (for instance when compiling the standard library) is scary: Moreover, it is somewhat interesting that tests pass, since on my local machine I get errors (mostly OOM) while running the test suite, both with and without my changes (I'm comparing the results). |
@paulp said: It's easy for the tests to pass and all the warnings to be valid, because scalac ignores the generic signatures completely. To see the breakage caused by this bug, you generally have to involve java so that something will be miscompiled based on the inaccurate signature. Signature verification wasn't on by default for performance reasons, so you will probably want to measure the asm impact. The underlying issue is fairly hairy (and there is more than one.) I have it fairly well outlined and fixed for some approximation of fixed, but I have recently lost my taste for this brand of completely thankless work. If you want I can clean up that branch and hand it over if you'd like to go for the finish line. |
@paulp said (edited on May 20, 2012 2:51:34 AM UTC): Another reason I stopped there is that I thought my time would be better spent rewriting the backend not to generate wrong signatures, rather than spending any more of my life chasing bugs which could be made impossible with decent engineering. But that's another thing I've lost enthusiasm for. |
@Blaisorblade said: I'm taking a look at this. After reading the commit message, I'm not totally sure what's missing there, so I'll have to give a closer look - I'd greatly appreciate any specific hints. Meanwhile, I'll try testing the results. I've merged your commit and I'm running the testsuite again to verify that no warning is produced, this time with enough memory for ant. Do you know if Jenkins will recompile my pull request if I push again? |
@retronym said: https://groups.google.com/d/topic/scala-internals/VCGsivMKtoY/discussion |
@paulp said:
Take a look at the diff from the two failing tests (assuming it still acts the way I've described; since I've rebased it I can't guarantee it.) It will point you in the direction of the problem. The change in mixin overshoots the target. |
@Blaisorblade said: strap.comp:
[scalacfork] Compiling 5 files to /Users/pgiarrusso/Documents/Research/Sorgenti/scala/build/strap/classes/compiler
[scalacfork] /Users/pgiarrusso/Documents/Research/Sorgenti/scala/src/compiler/scala/tools/nsc/transform/Erasure.scala:14: warning: compiler bug: created generic signature for method mkTypeCompleter in scala.tools.nsc.transform.Erasure that does not conform to its erasure
[scalacfork] signature: (Lscala/reflect/api/Trees$Tree;Lscala/Function1<Lscala/reflect/internal/Symbols$Symbol;Lscala/runtime/BoxedUnit;>;)Lscala/tools/nsc/typechecker/Namers$TypeCompleter;
[scalacfork] original type: (t: scala.reflect.api.Trees$Tree, c: Function1)Namers.this.Namers$TypeCompleter with Namers.this.Namers$LockingTypeCompleter
[scalacfork] normalized type: (t: scala.reflect.api.Trees$Tree, c: Function1)Namers.this.Namers$TypeCompleter with Namers.this.Namers$LockingTypeCompleter
[scalacfork] erasure type: (t: scala.reflect.api.Trees$Tree, c: Function1)scala.tools.nsc.typechecker.Namers$LockingTypeCompleter
[scalacfork] if this is reproducible, please report bug at https://issues.scala-lang.org/
[scalacfork] abstract class Erasure extends AddInterfaces The problem is in the return type, |
@Blaisorblade said: /**** This implementation to merge parents was checked in in commented-out
form and has languished unaltered for five years. I think we should
use it or lose it. Moreover, as far as I see (mostly) from comments, there's no attempt to simplify the intersection of a class with its parents in method |
@paulp said:
No, I don't think this quite captures the problem. Given this situation: abstract class A In scala B <: A, but that relationship is only known to scala and can't be expressed in the method descriptor. If the descriptor says "B", scala is attempting to say "some unknown class which extends A and implements B" but all java is going to hear is "B", which does not include A. So it is not clear that B is the most specific type, at least not usefully so. abstract class Foo
trait Bar1
trait Bar2 extends Foo
trait Bar3 { self: Foo => }
class Test {
def f1 = new Foo { }
def f2 = new Foo with Bar1 { }
def f3 = new Foo with Bar2 { }
def f4 = new Foo with Bar3 { }
// descriptors:
// public Foo f1();
// public Foo f2();
// public Bar2 f3();
// public Foo f4();
} |
@Blaisorblade said (edited on May 26, 2012 11:11:13 PM UTC): I'm not sure I get you fully, but I do see that things are not so simple as I thought. But does your example support your point? In our case, the relevant descriptor is the one for However, the erasure algorithm does what I considered obvious, and I would keep that unchanged for binary compatibility, since there might be existing Java code calling into Scala using erased descriptors (I wrote some myself, even if it was an horrible experience since the Scala interface was much more fancy). Do you agree about changing that algorithm? ExampleThe above example has no generic signatures, but adding type parameters produces a reduced version of the problem in //Other definitions like above, I only changed Test.
class Test {
def f1 = new Foo { }
def f2 = new Foo with Bar1 { }
def f3 = new Foo with Bar2 { }
def f4 = new Foo with Bar3 { }
def f5 = new Bar2 { }
def f6[T](t: T) = new Foo with Bar2 { }
def f7[T](t: T) = new Bar2 { }
// descriptors:
// public Foo f1();
// public Foo f2();
// public Bar2 f3();
// public Foo f4();
// public Bar2 f5();
// public Bar2 f6(Object);
// public Bar2 f7(Object);
// Both f6 and f7 have generic signature:
// <T:Ljava/lang/Object;>(TT;)LFoo;
// which means <T extends Object> Foo methodName(T);
} Of course, the generic signature is only emitted by a mainline Scala compiler, with the warning disabled. The generic signature warning (that is, SI-3452/DescriptorExample.scala:13: warning: compiler bug: created generic signature for method f6 in Test that does not conform to its erasure
signature: <T:Ljava/lang/Object;>(TT;)LFoo;
original type: [T >: ? <: ?](t: Object)Foo with Bar2
normalized type: [T >: ? <: ?](t: Object)Foo with Bar2
erasure type: (t: Object)Bar2
if this is reproducible, please report bug at https://issues.scala-lang.org/
def f6[T](t: T) = new Foo with Bar2 { }
^
SI-3452/DescriptorExample.scala:14: warning: compiler bug: created generic signature for method f7 in Test that does not conform to its erasure
signature: <T:Ljava/lang/Object;>(TT;)LFoo;
original type: [T >: ? <: ?](t: Object)Foo with Bar2
normalized type: [T >: ? <: ?](t: Object)Foo with Bar2
erasure type: (t: Object)Bar2
if this is reproducible, please report bug at https://issues.scala-lang.org/
def f7[T](t: T) = new Bar2 { }
^
two warnings found Notice, in particular, that the algorithm producing generic signatures transforms not only Back to encoding Other remaining bugsI categorized warnings on the library (which are simpler). Here's a list of what is left there:
[scalacfork] signature: (TB;)Lscala/runtime/Nothing$;
[scalacfork] original type: (x$1: Object)Nothing
[scalacfork] normalized type: (x$1: Object)scala.runtime.Nothing$
[scalacfork] erasure type: (x$1: Object)Nothing The documentation of Nothing$ claims that Nothing should erase to Nothing$; this one sounds easiest to fix, so I'll take a look at it first - I might actually be able to fix this.
[scalacfork] signature: <A:Ljava/lang/Object;>(TA;)Lscala/Predef$ArrowAssoc<TA;>;
[scalacfork] original type: [A >: ? <: ?](x: Object)Predef$ArrowAssoc[A]
[scalacfork] normalized type: [A >: ? <: ?](x: Object)Predef$ArrowAssoc[A]
[scalacfork] erasure type: (x: Object)Object
[scalacfork] if this is reproducible, please report bug at https://issues.scala-lang.org/
[scalacfork] @inline implicit def any2ArrowAssoc[A](x: A): ArrowAssoc[A] = new ArrowAssoc(x) (same problem on any2Ensuring)
[scalacfork] /Users/pgiarrusso/Documents/Research/Sorgenti/scala/src/library/scala/collection/parallel/mutable/ParHashMap.scala:192: warning: compiler bug: created generic signature for object table$1 in scala.collection.parallel.mutable.ParHashMapCombiner that does not conform to its erasure
[scalacfork] signature: ()Lscala/collection/parallel/mutable/ParHashMapCombiner$table$2$;
[scalacfork] original type: ()...
[scalacfork] normalized type: ()...
[scalacfork] erasure type: (table$module$1: scala.runtime.VolatileObjectRef)...
[scalacfork] if this is reproducible, please report bug at https://issues.scala-lang.org/
[scalacfork] object table extends HashTable[K, DefaultEntry[K, V]] {
[scalacfork] /Users/pgiarrusso/Documents/Research/Sorgenti/scala/src/library/scala/runtime/AbstractPartialFunction.scala:64: warning: compiler bug: created generic signature for method applyOrElse$mcZD$sp in scala.runtime.AbstractTotalFunction that does not conform to its erasure
[scalacfork] signature: <A1:Ljava/lang/Object;B1:Ljava/lang/Object;>(TA1;Lscala/Function1<TA1;TB1;>;)TB1;
[scalacfork] original type: [A1 >: ? <: ?, B1 >: ? <: ?](x: Double, default: Function1)B1
[scalacfork] normalized type: [A1 >: ? <: ?, B1 >: ? <: ?](x: Object, default: Function1)B1
[scalacfork] erasure type: (x: Double, default: Function1)Object
[scalacfork] if this is reproducible, please report bug at https://issues.scala-lang.org/
[scalacfork] @annotation.unspecialized override final def applyOrElse[A1 <: T1, B1 >: R](x: A1, default: A1 => B1): B1 = apply(x) |
@paulp said: This is exceeding the complexity of conversation I want to have in the useless jira comment interface. Could I talk you into starting a thread on scala-language? Also that way we might get some assistance. |
@Blaisorblade said: |
@Blaisorblade said: |
@adriaanm said: |
@Blaisorblade said: |
@Blaisorblade said: |
@retronym said: The key dilemna is what to do about mixin signatures. We should really add bridges. But that has proved really tough to implement. So as a stopgap measure, I'm weakening the as-seen-from the subclass generic signature if it doesn't erase to the same signature as the trait forwarder. By doing this conditionally, I can at least maintain backwards compatibility with code that didn't previously result in a LinkageError. |
@retronym said (edited on Feb 16, 2014 1:58:01 PM UTC): I'm hopeful we'll find a more refined approach in the future. I've outlined the ideas for that: #8293 If I have missed any residual signature issues that weren't related to forwarders, please open a new ticket and link it to this one. |
See this [http://scala-programming-language.1934581.n4.nabble.com/valid-code-throws-NoSuchMethodError-at-runtime-td2222685.html#a2222685 thread]
Compiling this with scalac (tested 2.8.0.RC2):
and compiling this with javac:
produces no compile-time errors but a runtime error later on:
After some analysis, I found out that is probably due to an inconsistent generic signature being generated for the static method delegate in
BulkSearchInstance.class
.Scalac generates these signatures for
BulkSearchInstance.searchFor
:When javac sees that, it produces something strange (i.e. not matching the descriptor found in
BulkSearchInstance.class
):which looks like the erasure of the generic signature. However, JVM's linker looks for the exact descriptor when looking for methods. I think the solution should be
Workaround: use a trait instead of an abstract class
The text was updated successfully, but these errors were encountered: