Implement IdentityHashMap for the JS target #195
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ftomassetti
reviewed
Aug 26, 2024
antlr-kotlin-runtime/src/jsMain/kotlin/com/strumenta/antlrkotlin/runtime/IdentityEntriesView.kt
Show resolved
Hide resolved
| prev = imap.put(a, "two") | ||
| assertNull(prev) | ||
|
|
||
| val b = B("one") |
There was a problem hiding this comment.
I think it would also be interesting to see if we can insert two elements that equals but do not have the same identity
There was a problem hiding this comment.
The last commit addresses both of the remaining comments.
|
|
||
| @Test | ||
| fun containsValue() { | ||
| val imap = IdentityHashMap<String, A>() |
There was a problem hiding this comment.
I think it would be useful to write a test using IdentityHashMap<A, String>() and then using different instances of A that are equals but are different instances as keys
lppedd
force-pushed
the
feat/identity-map
branch
from
d8764e8 to
55bab2f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Finally managed to implement a proper identity map for K/JS, using the ES2015
Mapas backing store.WASM and Native are more complex, and less used, so they'll have to wait longer.
And I'm not even sure it's possible in WASM.
The next step (another PR) is moving
CopyOnWriteArrayList,IdentityHashMapandWeakHashMapto beinternal, to avoid consumers using them and having to deal with unexpected results.