New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes NPE in hashcode with empty keys (Issue 52) #53
Conversation
src/potemkin/collections.clj
Outdated
(unchecked-add acc (bit-xor (.hashCode k) (.hashCode v)))) | ||
(fn [acc [k v]] | ||
(unchecked-add acc (bit-xor | ||
(if k (.hashCode k) 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, maybe (if (some? k) (.hashCode k) 0)
, since (not= 0 (.hashCode false))
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see what you mean. but its more relevant for a value rather than the key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and that's not gonna be a nitpick but we just won't hash false values with my code :)
195fadc
to
4bca73a
Compare
I'd prefer to use |
ah. didn't know about that one. Worked like a charm. Updated @ztellman |
Also, can add a test if you would like but it's not particularly complicated. Up to you |
No test required, thanks for the report and fix. |
…ANT-633] This just ports the fix from clj-commons/potemkin#53 to our vendored potemkin.
Several alternatives were tried: if you put the whole unchecked-add if
an
(if (and k v) (unchecked-add ...) 0)
then you get a hashvalue of0 when there is only one key and a missing value as the key isn't
hashed. If you
(unchecked-add acc (bit-xor (.hashCode (or k 0)) ..
then you end up with reflection warnings.