Community
Participate
Working Groups
Build Identifier: MyEclipse Enterprise Workbench 8.0-20091120 When running code built by the eclipse compiler, a particular method call finds the wrong method, resulting in bad output from our code. When the code is compiled outside of eclipse, it works fine. It's a bit complicated to explain but I've managed to put together a minimal (at least I think it's minimal) test case that replicates the error. Basically, a call to a method that is overloaded across two classes in the hierarchy, results in matching the least specific method out of two, instead of the most specific method. The particular mix of generics is quite complex, I suppose. The bug means I can't run my code from eclipse and get the correct results without running a specific build so that the internal compiler doesn't produce the class files. Though this is a workaround, it's a mjor pain to have to keep doing that for every run. Reproducible: Always Steps to Reproduce: 1.Set up a project with the attached source and let eclipse build it. 2.Run the class C. 3.The output should be: In B.set(CharSequence) but is: In A.set(Object) 4: Now build the program outside of eclipse (perhaps using the simple ant build file that's included) and run class C. The output is as expected.
Created attachment 158659 [details] Test project to reproduce the error
Confirmed the difference in behavior between eclipse on the one hand and javac 5,6,7 on the other. Here is the same test case simplified a bit : just copy & paste into package explorer: -----------------------8<------------------------------- class A<ModelType extends D, ValueType> implements I<ModelType, ValueType> { @Override public void doSet(ModelType valueGetter) { this.set((ValueType) valueGetter.getObject()); } @Override public void set(Object object) { System.out.println("In A.set(Object)"); } } class B extends A<E, CharSequence> { public void set(CharSequence string) { System.out.println("In B.set(CharSequence)"); } } public class C extends B { static public void main(String[] args) { C c = new C(); c.run(); } public void run() { E e = new E<String>(String.class); this.doSet(e); } } class D { public Object getObject() { return null; } } class E<Type extends CharSequence> extends D { private Class<Type> typeClass; public E(Class<Type> typeClass) { this.typeClass = typeClass; } @Override public Type getObject() { try { return (Type) typeClass.newInstance(); } catch (Exception e) { throw new RuntimeException(e); } } } interface I<ModelType, ValueType> { public void doSet(ModelType model); public void set(ValueType value); }
Here is some interesting behavior: Experiment#1 ------------ (1) Set project compiler compliance to 1.6 (2) Change the class B to be: (i.e add @Override annotation to set() method) class B extends A<E, CharSequence> { @Override public void set(CharSequence string) { System.out.println("In B.set(CharSequence)"); } } The project fails to build now. If you hover over the margin quickfix marker you get two messages that completely contradict each other: Multiple markers at this line - implements I<E,java.lang.CharSequence>.set - The method set(CharSequence) of type B must override or implement a supertype method It would appear that the compiler fails to identify set(CharSequence) as B's implementation for I<E,java.lang.CharSequence>.set while other parts of the JDT toolchain do recognize. Experiment#2: ------------ Change class B to be: class B extends A<E, CharSequence> { @Override public void set(Object object) { System.out.println("In B.set(CharSequence)"); } } Now the @Override annotation is not complained against. And, when we run the program output is as expected (i.e in B.set(CharSequence)) So from a black box p.o.v it looks like there are two problems here: (1) An overriding method is not recognized as such. (2) And so needed bridge methods are not generated.
Yes, without the @Override, the method is recognised as an override, but with the annotation, it isn't. Another odd situation arises if the implements clause is removed from the definition of class A. As far as I can see, the removal should have no impact on the running of the test case (type I is never referenced anywhere else and the generic type parameters still apply in class A). However, both inside and outside eclipse, the set(Object) method is called, rather than the set(CharSequence) method. It may be that the eclipse compiler is doing the right thing (in the original test case) and the Java compiler is doing the wrong thing. Unfortunately, the generics is too complicated for me to figure out at the moment, so I'm hoping one of you guys can shed some light on it.
(In reply to comment #4) > Yes, without the @Override, the method is recognised as an override, but with > the annotation, it isn't. Actually, with or without the annotations, the eclipse *compiler* fails to recognize the method as an override (otherwise, the method call would have worked as expected). The override margin decoration code recognizes the method as an override with or without annotations. Which explains why we get this conflicting messages when you hover over the margin marker. Multiple markers at this line - implements I<E,java.lang.CharSequence>.set - The method set(CharSequence) of type B must override or implement a supertype method So, the left brain and the right brain are working in a self consistent manner, they are at odds only with each other. > Another odd situation arises if the implements clause is removed from the > definition of class A. [...] > However, both inside and > outside eclipse, the set(Object) method is called, rather than the > set(CharSequence) method. I believe in this case eclipse & javac are both correct. Since now, B's set is not overriding anything and the receiver type knows of only A's set, the dispatch should result in A' set being called.
Right. I thought the compiler somehow got a say in the override decorations. As regards the behaviour if the implements clause is removed, I see what you mean. So this seems to be reasonable behaviour for Java; with erasure, the valueGetter appears to return an Object (from the point of view of A.doSet) and so it finds A.set(Object) as the closest match. However, isn't this exactly the same situation with the implements clause reinstated, since the erasure of I.set still takes Object as the argument?
(In reply to comment #6) > Right. I thought the compiler somehow got a say in the override decorations. The Override decorations are implemented using org.eclipse.jdt.core.dom.ITypeBinding.getSuperclass() org.eclipse.jdt.core.dom.ITypeBinding.getInterfaces() org.eclipse.jdt.core.dom.ITypeBinding.getDeclaredMethods() and not from the compiler's inferences per se. > so it finds A.set(Object) as the closest match. However, isn't this exactly the > same situation with the implements clause reinstated, since the erasure of > I.set still takes Object as the argument? Since B implements I<E, CharSequence> if B were to define a method (as in your test case) with the signature void set(CharSequence sequence) that would be B's implementation of the interface method. Since A's implementation of the interface method and B's implementation of the same interface method differ in erasure, the compiler will generate suitable bridges.
This defect goes back a long way. The fix for bug# 52916, (erroneously) eliminated inspection of super interfaces when computing inherited methods the moment one concrete class is seen as we walk the super types.
Also broken is : class A<ModelType extends D, ValueType> extends I<ModelType, ValueType> { @Override public void doSet(ModelType valueGetter) { this.set((ValueType) valueGetter.getObject()); } @Override public void set(Object object) { System.out.println("In A.set(Object)"); } } class B extends A<E, CharSequence> { // @Override public void set(CharSequence string) { System.out.println("In B.set(CharSequence)"); } } public class C extends B { static public void main(String[] args) { C c = new C(); c.run(); } public void run() { E e = new E<String>(String.class); this.doSet(e); } } class D { public Object getObject() { return null; } } class E<Type extends CharSequence> extends D { private Class<Type> typeClass; public E(Class<Type> typeClass) { this.typeClass = typeClass; } @Override public Type getObject() { try { return (Type) typeClass.newInstance(); } catch (Exception e) { throw new RuntimeException(e); } } } abstract class I<ModelType, ValueType> { public abstract void doSet(ModelType model); public abstract void set(ValueType value); } --------------------------------- Probably (didn't verify) due to the fix for bug#227185
Created attachment 159149 [details] Patch under consideration.
Created attachment 159151 [details] Proposed patch Same patch with some cleanup in comments. Added new tests via: org.eclipse.jdt.core.tests.compiler.regression.AmbiguousMethodTest.test081() org.eclipse.jdt.core.tests.compiler.regression.AmbiguousMethodTest.test082() org.eclipse.jdt.core.tests.compiler.regression.AmbiguousMethodTest.test083()
(In reply to comment #11) > Created an attachment (id=159151) [details] > Proposed patch While this patch fixes the problem and passes all our tests it does cause other legal code to fail to compile. In the example below, the compiler now issues an incorrect error message: "The type B must implement the inherited abstract method I<E,CharSequence>.set(CharSequence)" abstract class A<ModelType extends D, ValueType> implements I<ModelType, ValueType> { @Override public void doSet(ModelType valueGetter) { this.set((ValueType) valueGetter.getObject()); } @Override public void set(Object object) { System.out.println("In A.set(Object)"); } } class B extends A<E, CharSequence> { } public class C extends B { static public void main(String[] args) { C c = new C(); c.run(); } public void run() { E e = new E<String>(String.class); this.doSet(e); } } class D { public Object getObject() { return null; } } class E<Type extends CharSequence> extends D { private Class<Type> typeClass; public E(Class<Type> typeClass) { this.typeClass = typeClass; } @Override public Type getObject() { try { return (Type) typeClass.newInstance(); } catch (Exception e) { throw new RuntimeException(e); } } } interface I<ModelType, ValueType> { public void doSet(ModelType model); public void set(ValueType value); } > > Same patch with some cleanup in comments. > > Added new tests via: > org.eclipse.jdt.core.tests.compiler.regression.AmbiguousMethodTest.test081() > org.eclipse.jdt.core.tests.compiler.regression.AmbiguousMethodTest.test082() > org.eclipse.jdt.core.tests.compiler.regression.AmbiguousMethodTest.test083()
Created attachment 159272 [details] Revised patch Olivier, please review. Added 3 more tests for a total of six. org.eclipse.jdt.core.tests.compiler.regression.AmbiguousMethodTest.test081() org.eclipse.jdt.core.tests.compiler.regression.AmbiguousMethodTest.test082() org.eclipse.jdt.core.tests.compiler.regression.AmbiguousMethodTest.test083() org.eclipse.jdt.core.tests.compiler.regression.AmbiguousMethodTest.test084() org.eclipse.jdt.core.tests.compiler.regression.AmbiguousMethodTest.test085() org.eclipse.jdt.core.tests.compiler.regression.AmbiguousMethodTest.test086()
Satyam, please provide an additional review - Thanks!
Released in HEAD for 3.6M6
+1 for this patch
Verified for 3.6M6 using build I20100305-1011.
Verified.
