Community
Participate
Working Groups
Fixing bug 527462 wasn't enough to make the example https://github.com/AndreasMager/eclipse-dagger-test for the "dagger 2" issue https://github.com/google/dagger/issues/912 work with eclipse. The remaining problem is that various elements are compared with hashCode and equals, especially I noted org.eclipse.jdt.internal.compiler.apt.model.VariableElementImpl being compared for parameters of binary classes. In this case, comparing the bindings with == is not enough, as the bindings may be different AptBinaryLocalVariableBinding instances that are created on-demand for each invocation of org.eclipse.jdt.internal.compiler.apt.model.ExecutableElementImpl.getParameters(). The patch adds matching hashcode and equals methods for this case.
New Gerrit change created: https://git.eclipse.org/r/111931
@Jay, I don't have a regression test for this and currently think it is not worth the effort to invest too much time in this. Should I still commit this? Also, during debugging, I noted that hashCode might be invoked for UnresolvedAnnotationBinding before it is resolved, so I added a resolve() (in AnnotationBinding). WDYT?
@Jay, I think it would be a good idea if you have a look at the code and then we commit this even without tests.
I don't have any problem in principle with implementing hashCode and equals for these. However, I'm concerned about "as the bindings may be different AptBinaryLocalVariableBinding instances that are created on-demand for each invocation of ...ExecutableElementImpl.getParameters()". Should we instead be ensuring that we return the same instance, if one exists? Should clients of this method be able to assume == identity? What does javac do?
(In reply to Walter Harley from comment #4) > I don't have any problem in principle with implementing hashCode and equals > for these. However, I'm concerned about "as the bindings may be different > AptBinaryLocalVariableBinding instances that are created on-demand for each > invocation of ...ExecutableElementImpl.getParameters()". Should we instead > be ensuring that we return the same instance, if one exists? Should clients > of this method be able to assume == identity? > > What does javac do? "Elements should be compared using the equals(Object) method. There is no guarantee that any particular element will always be represented by the same object." https://docs.oracle.com/javase/8/docs/api/javax/lang/model/element/Element.html
The patch generally looks good to me. Just couple of points: 1. The call to resolve() in hashCode() makes me a bit uneasy for two reasons: a) It does lot of things in a hashCode() call, which is usually in the middle of intense collections operation. b) If the unresolved annotation binding was created for a reason, will resolving it here have any side effects. I am only thinking aloud and I don't have any solid theory behind these questions.
For an UnresolvedAnnotationBinding, equals (defined in AnnotationBinding) calls resolve() anyway because it calls getAnnotationType which is overridden in UnresolvedAnnotationBinding and calls resolve(): public ReferenceBinding getAnnotationType() { resolve(); return this.type; } I don't think it makes sense to invoke hashCode without possibly calling equals , so there can't be an issue. But rather than invoke resolve(), it might clearer to simply use this.getAnnotationType() instead of this.type in hashCode, so I'll update the patch.
Gerrit change https://git.eclipse.org/r/111931 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=8e6858e858b81430059914da5f4189ae96c32222
(In reply to Eclipse Genie from comment #8) > Gerrit change https://git.eclipse.org/r/111931 was merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/ > ?id=8e6858e858b81430059914da5f4189ae96c32222 Released for 4.8M5
Verified for Eclipse Photon 4.8 M5 with Build id: I20180124-2000
