explosion / preshed

💥 Cython hash tables that assume keys are pre-hashed
MIT License
82 stars 19 forks source link

PreshMap can not contain 0 value #17

Closed svlandeg closed 5 years ago

svlandeg commented 5 years ago

Is it a feature or a bug that a PreshMapcan not contain 0 as value?

Consider this code:

self._alias_index = PreshMap()
self._alias_index[342] = 1
if 342 in self._alias_index:
    print("yay")
else:
    print("not so yay")

which prints yay

self._alias_index = PreshMap()
self._alias_index[342] = 0
if 342 in self._alias_index:
    print("yay")
else:
    print("not so yay")

which prints not so yay

honnibal commented 5 years ago

I guess it's an unideal quirk. The first implementations actually didn't allow 0 keys either, but there's now a mechanism for that: you can specify the value that denotes "empty", so you can set that to something else if you need 0 in the table. We could do something like this to allow 0 as a value.

There does need to be some 64-bit value that denotes empty key, and a 64-bit value that denotes deleted key. We also need a 64-bit value to denote empty.

Unfortunately I think it'll cause some trouble to fix this, as I think a lot of code assumes that 0 is missing from Preshed, as it's pretty much always been like this. Does it inconvenience your code much?

svlandeg commented 5 years ago

It just took me some time to realise what was going on, as it wasn't clear to me that 0 couldn't be a value. But now I just add a dummy element in the array first, so that the PreshMap never has to point to index 0.