Make PTBKey not extend ReferenceBinding #3452
Conversation
Its just an IdentityWrapper - do not rely on ReferenceBinding implementation. relates to eclipse-jdt#3452
|
@srikanth-sankaran may i ask for review? I would like to improve ReferenceBinding in small steps. |
I'll try - but can't promise, I am trying to wrap up some projects I am working on before this week since post that I am away till 2nd Jan. |
Its just an IdentityWrapper - do not rely on ReferenceBinding implementation. relates to #3452
|
@srikanth-sankaran ping, please review. |
|
Sure, I will look into these this week. Happy new year! |
Sorry, it looks like my full return to work will be delayed until a few days more. I will be at work 3 out of 5 days next week and I am hopeful the review can be completed well ahead of M2. |
There was a problem hiding this comment.
See comments, need some clarifications on what exactly we are trying to accomplish here.
I am yet to come to grips with what the new additional interface buys us.
Also the title of the PR PTBKey is not a ReferenceBinding - need clarification. What exactly do you mean by that.
I don't see anything seriously wrong/faulty - but simply don't get yet where we are going with this and why
...jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeBindingWrapper.java
Outdated
Show resolved
Hide resolved
...clipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeBinding.java
Show resolved
Hide resolved
...eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeSystem.java
Show resolved
Hide resolved
| @@ -53,18 +53,18 @@ public TypeBinding clone(TypeBinding outerType) { | |||
| return copy; | |||
| } | |||
|
|
|||
| void addWrapper(TypeBinding wrapper, LookupEnvironment environment) { | |||
| void addWrapper(TypeBindingWrapper wrapper, LookupEnvironment environment) { | |||
There was a problem hiding this comment.
This is the only usage of the proposed interface. Here the instance is called "wrapper" and the method name add"Wrapper" also suggest to have "Wapper" in the interface name. Thats why i chose "TypeBindingWrapper". but i would be fine with any other Name.
jukzi
changed the title
jukzi
force-pushed
the
TypeBindingWrapper
branch
from
54187ac to
068f092
Compare
|
Not sure if you are planning to explain the PR title change - the new one still doesn't clear the mystery for me :) Do you have a scenario where the PTBKey is actually not a reference binding ?? Without the additional name TypeBindingWrapper being used, things were working so far - so what does extracting the swapUnresolved behavior into its own type buy us ? |
I am trying to clean up the ReferenceBinding equals/hashCode contract as it hard to understand and errorprone to accidentialy use when using a ReferenceBinding as key (for example #3561 ). As this PTBKey does not need to implement ReferenceBinding one could just ignore the PTBKey in such scenarios. At the end it will be fine if the PTBKey will have a custom equals/hashcode, but any actual ReferenceBinding probably won't need to override equals/hashcode => no more worries in the future. |
PTBKey needs a custom hashCode/equals contract, while ReferenceBinding should not need any. * PTBKey not extends ReferenceBinding anymore * Introduces interface Resolvable. * Removes little code from PTBKey that is irrelevant for the wrapper relates to eclipse-jdt#3412
jukzi
force-pushed
the
TypeBindingWrapper
branch
from
068f092 to
cf810f5
Compare
jukzi
changed the title
PTBKey is not "implementing" |
It was a ReferenceBinding but did not need to be ReferenceBinding. It is only a wrapper used as key for ParameterizedTypeBinding. It is not used as ReferenceBinding itself anywhere. Especially PTBKey.hash(TypeBinding) will never call PTBKey.hashCode() which was not clear from the type hierarchy. |
relates to #3412