lcm-proj / lcm

Lightweight Communications and Marshalling
GNU Lesser General Public License v2.1
980 stars 388 forks source link

What does `_hash = None` accomplish in emit_python.c? #335

Closed mdonahoe closed 3 years ago

mdonahoe commented 3 years ago

https://github.com/lcm-proj/lcm/blob/8f7189bd1db9dfeb66cd4ed0bfd34edba819f46e/lcmgen/emit_python.c#L599

This is different from the typical __hash__ = None used to make a Python object unhashable.

hoxell commented 3 years ago

See the fingerprint section here for the details

mdonahoe commented 3 years ago

That was my first guess too, but the fingerprint logic is handled by _get_packed_fingerprint(), which then uses _packed_fingerprint to cache the result.

But I don't see where _hash is ever referenced in the python implementation, except for _hash = None

    _hash = None
    def _get_hash_recursive(parents):
        if bar_t in parents: return 0
        tmphash = (0xd21b643a0f4a4e57) & 0xffffffffffffffff
        tmphash  = (((tmphash<<1)&0xffffffffffffffff) + (tmphash>>63)) & 0xffffffffffffffff
        return tmphash
    _get_hash_recursive = staticmethod(_get_hash_recursive)
    _packed_fingerprint = None

    def _get_packed_fingerprint():
        if bar_t._packed_fingerprint is None:
            bar_t._packed_fingerprint = struct.pack(">Q", bar_t._get_hash_recursive([]))
        return bar_t._packed_fingerprint
    _get_packed_fingerprint = staticmethod(_get_packed_fingerprint)
hoxell commented 3 years ago

You're absolutely right (as far as I can tell)! I browsed the src now and can't find that _hash is used anywhere, but it's possible that I've missed something. I also performed a quick and naive test of manually deleting the line from the generated file and nothing blew up, so that seems promising.

Create a PR where you remove the line and see if someone remembers if there was a reason to introduce the line 13 years ago 😉

mdonahoe commented 3 years ago

Done https://github.com/lcm-proj/lcm/pull/336

mdonahoe commented 3 years ago

merged