Community
Participate
Working Groups
Build Identifier: Scope lack equals and hashCode methods and sometimes need to be tested for equality or used in HashMaps. Reproducible: Always
Created attachment 200720 [details] Patch to add equals and hashCode methods
Created attachment 200906 [details] Test to check equals and hashCode work Relies on https://bugs.eclipse.org/bugs/attachment.cgi?id=200904
the patch has been merged and nominally tested in our local source base. the test case has been added instead to org.eclipse.cdt.debug.edc.tests.ScopeTest rather than as a separate regression test. both are waiting for final review and merge to eclipse edc git repository by committer Ken Ryall.
after testing these changes for a bit, we discovered a regression having to do with the fact that the lowAddress and highAddress are mutable, and can cause the hashCode values to be computed differently before and after fixupRanges, and leave the user with HashMap and HashSet containers whose contents contain duplicate entries and eventually pointers to symbols that are stale. our analysis showed that the most important scope involving hashCode values is compileUnitScope, and so we have created abstract hashCode()/equals() in CompileUnitScope, forcing specific implementers of ICompileUnitScope to provide a hashCode(), which allows ModuleLineEntryProvider to have its hashSet of ICompileUnitScope to behave properly. please comment if there is a specific need you have for hashCode() for Scope itself.
I have a HashMap of IScopes to other things to which scopes are added after being fully parsed so that their high and low addresses will not change. Hence a need for hashCode in Scope.
(In reply to comment #5) > I have a HashMap of IScopes to other things to which scopes are added after > being fully parsed so that their high and low addresses will not change. Hence > a need for hashCode in Scope. for such a case, it still needs to be a rule that hashCode()/equals() work only with immutable members, and lowAddress/highAddress do not qualify, even if you intend only to use them after they won't be changed. my best recommendation would be to override Scope (and whatever other extensions you need to place in the HashMaps you are using) with extensions that would use constructors that would copy the Scope in which you have finalized the low and high addresses. make these classes implementors of an interface named something like IFinalizedScope that is an extension of IScope, and in which you can implement hashCode()/equals() in a way that you know will rely on values that would be immutable. whatever is done, it has to guarantee that nothing will confuse one scope for another just because some immutable value has been changed despite attempts/efforts to conventionalize the method that the objects are used/stored.
Created attachment 202508 [details] Patch implementing hashCode and equals and only using immutable fields for hashCode Only hashCode is required to only use immutable members and it does not have to return a unique value - just a pretty unique value. Hence by making parent and name immutable it is possible to write a hashCode in Scope which works fine and the value of which does not vary.
(In reply to comment #7) > Created attachment 202508 [details] > Patch implementing hashCode and equals and only using immutable fields for > hashCode > > Only hashCode is required to only use immutable members and it does not have to > return a unique value - just a pretty unique value. Hence by making parent and > name immutable it is possible to write a hashCode in Scope which works fine and > the value of which does not vary. one of the problems we had originally been facing that led us to completely separating the compileUnitScope and below from the scope from above was directly related to the lineEntries information changing and cashing a visited hashSet from behaving properly. so adding lineEntries back into the equals function raises a huge red flag. i will agree that, in principal & theory, what gets compared in equals() doesn't have to be what gets used for calculation in hashCode() ... but from the very real pragmatic standpoint of testing, the regressions we hit when trying to use variations of hashCode()/equals() before finally settling on what's in eclipse edc git repository now make me wary to wade back in with this commit.
(this is part of a batch change) The Eclipse CDT EDC (https://wiki.eclipse.org/CDT/cdt-debug-edc) is now obsolete and has not had any active development since 2011. Therefore the still open bugs are being marked as wontfix. The git repo for the project still exists for posterity at https://git.eclipse.org/c/cdt/org.eclipse.cdt.edc.git/
