orlp / polymur-hash

The PolymurHash universal hash function.
zlib License
318 stars 7 forks source link

Redundant len == 0 check #2

Closed e4m2 closed 1 year ago

e4m2 commented 1 year ago

This branch is never reached since the same condition already exists in the public API.

BTW I found this while implementing PolymurHash in Zig (here if you're interested). Thanks for your work on this!

orlp commented 1 year ago

I am aware. But (blame my Rust background) I'd like each function to be sound on its own, and not invoke undefined behavior when called with zero-length input. And the compiler gets rid of the extra check after inlining.

orlp commented 1 year ago

FYI taking a quick look at your repo, my name is Orson, not Orsen.

e4m2 commented 1 year ago

Ah, sorry, I spelled it right in the source code as well. Fixed now.

orlp commented 1 year ago

@e4m2 I have already released version 2 of PolymurHash that changed the behavior for zero-length strings because it was brought to my attention that having a static return of 0 is problematic for a variety of probabilistic algorithms like MinHash and HyperLogLog: https://github.com/orlp/polymur-hash/issues/3