Community
Participate
Working Groups
We are using the tycho-compiler-jdt for running our annotation processor tests for MapStruct. Each of our tests runs with javac and the tycho-compiler-jdt. When we tried updating the dependency to 1.2.0 we had a failure in one of our tests. This is the example structure that we have (maybe it can be simplified to display the issue): public class Key { } public class KeyOfAllBeings extends Key { } public class AnimalKey extends KeyOfAllBeings { } public interface Identifiable<T extends Key> { T getKey(); } public interface Animal<ID extends KeyOfAllBeings> extends Identifiable<KeyOfAllBeings> { @Override ID getKey(); void setKey(ID item); } public abstract class BaseAnimal implements Identifiable<KeyOfAllBeings>, Animal<AnimalKey> { } public class Elephant extends BaseAnimal { private AnimalKey key; @Override public AnimalKey getKey() { return key; } @Override public void setKey(AnimalKey key) { this.key = key; } } public class Elephant extends BaseAnimal { private AnimalKey key; @Override public AnimalKey getKey() { return key; } @Override public void setKey(AnimalKey key) { this.key = key; } } Then we have the following usage: BaseAnimal animal = new Elephant(); The return type for animal.getKey() reported by the compiler is Identifiable<KeyOfAllBeings>. However, on javac it is Animal<AnimalKey>. Therefore, I am not entirely sure if this is a bug in javac or the tycho compiler.
The Tycho compiler is JDT/ecj. So it's very likely that if you see such issue with Tycho, you should see it in the IDE as well. How does this behave with a recent JDT in Eclipse IDE? Or if you compile using ecj (get "batch compiler" from http://download.eclipse.org/eclipse/downloads/drops4/R-4.9-201809060745/ ) directly?
Just tried it with Eclipse 4.9.0 and the annotation processing in there generates the same code. I forgot to mention. The exact place (where it can be debugged) is within https://github.com/mapstruct/mapstruct/pull/1587. The actual problem that is causing our test to fail is the fact that for BaseAnimal the getKey method is resolved to the one from Identifiable<KeyOfAllBeings>. However, in javac it is resolved to the one from IAnimal<AnimalKey>. For our test they have different meanings as we are testing some particular edge case in there.
(In reply to Filip Hrisafov from comment #2) > Just tried it with Eclipse 4.9.0 and the annotation processing in there > generates the same code. Same code as what? What's expected and produced by javac, or same apparently erroneous code produced by Tycho 1.2?
Created attachment 276236 [details] reproducal testcase for the problem This test case shows the actual problem. >Same code as what? What's expected and produced by javac, or same apparently erroneous code produced by Tycho 1.2? It is same as the code produced by Tycho 1.2. Investigating it more I think the problem is in the jdt compiler apt. I checked out the jdt core repository and created a testcase that is green with javac but not with the jdt apt (not sure where the actual problem is). Let me know if you would prefer me to commit this somewhere. I haven't tried the test with org.eclipse.jdt.compiler.apt version 1.2.100.v20160418-1457 (used by the tycho-compiler-jdt version 1.0.0). However, I presume it would be green as our suite is green for that version.
Update about this issue. I have now tried with version 1.4.0 and the result is the same. One other interesting thing that I noticed is that the error only occurs on Java 8. Using Java 11 or 13 it works.
I compared the .class files produced by javac and ecj and found no significant difference, for Animal.getKey() both compilers emit two methods: The "real" method: with descriptor ()Ltargets/model8/KeyOfAllBeings; with signature ()TID; and a bridge method: with descriptor ()Ltargets/model8/Key; no generic signature So I don't see a problem in the compiler, but perhaps APT's model has a bug? Or it's just the order in which methods are reported and accidentally your test finds one or the other method depending on some irrelevant influence? Simply picking the first found methods of a given selector is probably not a good idea.
> So I don't see a problem in the compiler, but perhaps APT's model has a bug? I didn't check the .class files. I was working with javax.lang.model.util.Elements#overrides and that has different results between javac and ecj. Most probably this is a problem in the APT component. > Or it's just the order in which methods are reported and accidentally your > test finds one or the other method depending on some irrelevant influence? > Simply picking the first found methods of a given selector is probably > not a good idea. We are not picking the first found method. We are relying on javax.lang.model.util.Elements#overrides to pick the most specific method. Have you seen the failure in the reproducal test case [1]? What basically happens is that we get the executable element getKey from Identifiable (identifiableGetKeyMethod) and Animal (animalGetKeyMethod) and then call javax.lang.model.util.Elements#overrides(identifiableGetKeyMethod, animalGetKeyMethod, baseAnimalType). This methods call returns true with ecj on Java 11. Which means that identifiableGetKeyMethod overrides animalGetKeyMethod in baseAnimalType. When running with Java 11 the same method returns false. [1] https://bugs.eclipse.org/bugs/attachment.cgi?id=276236
My point relates to this part of your test program: + ExecutableElement animalGetKeyMethod = null; + for (ExecutableElement member : ElementFilter.methodsIn(_elementUtils.getAllMembers(animalType))) { + if ("getKey".equals(member.getSimpleName().toString())) { + animalGetKeyMethod = member; + break; + } + } Animal.class has two methods of this selector - directly that is, not counting methods in supertypes. Which one of those will be assigned to animalGetKeyMethod is unspecified.
You are right about that part. However, I forgot to mention. If we replace _elementUtils.getAllMembers(animalType) with animalType.getEnclosedElements() This way the method that is defined by animalGetKeyMethod is really the method in the source of the Animal class. There can be only such method in this case. Sorry for not being clearer from the start.
(In reply to Filip Hrisafov from comment #9) > You are right about that part. > > However, I forgot to mention. If we replace > > _elementUtils.getAllMembers(animalType) > > with > > animalType.getEnclosedElements() > > This way the method that is defined by animalGetKeyMethod is really the > method in the source of the Animal class. There can be only such method in > this case. > > Sorry for not being clearer from the start. I'm not an expert in that particular area, but from a quick glance I see no reason why getEnclosedElements() would exclude the bridge method.
> I'm not an expert in that particular area, but from a quick glance I see no reason > why getEnclosedElements() would exclude the bridge method. While I was debugging I didn't see the bridge method being returned from getEnclosedElements(). I think that the bridge method is there only in the class file and not in the TypeElement. I am not an expert, but I think that the TypeElement should use both the real and bridge method to construct the ExecutableElement. This might mean that there is some kind of a bug in the javax.lang.model.util.Elements#overrides method on Java 8. We are working with the Abstract Syntax Tree during the annotation processing, that is why I think that we should not be seeing bridge methods.
(In reply to Filip Hrisafov from comment #11) > This might mean that there is some kind of a bug in the > javax.lang.model.util.Elements#overrides method on Java 8. That would mean we are talking about JDT's implementation of the method, which can be found in ExecutableElementImpl#overrides(...)
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
