← Back to branch summary

~bzr-pqm/bzr/bzr.dev

« back to all changes in this revision

Viewing changes to bzrlib/_simple_set_pyx.pyx

  • Committer: John Arbash Meinel
  • Date: 2009-10-13 16:44:43 UTC
  • mto: This revision was merged to the branch mainline in revision 4741.
  • Revision ID: john@arbash-meinel.com-20091013164443-b92lnyiir2ucyguj
Stop using hash() because of bugs wrt pyrex 0.9.8.5

Rather than going directly to the Py_TYPE() object, I also use PyObject_Hash()
everywhere now. This simplifies the code a little bit, as I can declare it
returns -1 as an exception, rather than having to manually check the return
value.

What is really strange is that pyrex 0.9.7.2 gets it right, strange
regression to have. cython 0.11.3 also gets it right, but I don't know
that all versions of cython handle it correctly, either.


The main problem is that we are mixing, and then comparing
'other_hash = this_hash'. If we always used the 32-bit form, we would
be okay for our purposes, or always use the 64-bit form. I'm focusing
on the latter.

Show diffs side-by-side

added added

removed removed

Lines of Context:
bzrlib/_simple_set_pyx.pyx
37
37
 
38
38
    PyTypeObject *Py_TYPE(PyObject *)
39
39
    int PyObject_IsTrue(PyObject *)
 
40
    # Note: *Don't* use hash(), Pyrex 0.9.8.5 thinks it returns an 'int', and
 
41
    #       thus silently truncates to 32-bits on 64-bit machines.
 
42
    long PyObject_Hash(PyObject *) except -1
40
43
        
41
44
    void *PyMem_Malloc(size_t nbytes)
42
45
    void PyMem_Free(void *)
61
64
 
62
65
    if this == other:
63
66
        return 1
64
 
    other_hash = Py_TYPE(other).tp_hash(other)
65
 
    if other_hash == -1:
66
 
        # Even though other successfully hashed in the past, it seems to have
67
 
        # changed its mind, and failed this time, so propogate the failure.
68
 
        return -1
 
67
    other_hash = PyObject_Hash(other)
69
68
    if other_hash != this_hash:
70
69
        return 0
71
70
 
203
202
        mask = self._mask
204
203
        table = self._table
205
204
 
206
 
        the_hash = Py_TYPE(key).tp_hash(key)
207
 
        if the_hash == -1:
208
 
            return -1
 
205
        the_hash = PyObject_Hash(key)
209
206
        i = the_hash
210
207
        for n_lookup from 0 <= n_lookup <= <size_t>mask: # Don't loop forever
211
208
            slot = &table[i & mask]
458
455
    cdef long key_hash
459
456
    cdef PyObject **table, **slot, *cur, **free_slot, *py_key
460
457
 
461
 
    # hash is a signed long(), we are using an offset at unsigned size_t
462
 
    key_hash = hash(key)
 
458
    py_key = <PyObject *>key
 
459
    # Note: avoid using hash(obj) because of a bug w/ pyrex 0.9.8.5 and 64-bit
 
460
    #       (it treats hash() as returning an 'int' rather than a 'long')
 
461
    key_hash = PyObject_Hash(py_key)
463
462
    i = <size_t>key_hash
464
463
    mask = self._mask
465
464
    table = self._table
466
465
    free_slot = NULL
467
 
    py_key = <PyObject *>key
468
466
    for n_lookup from 0 <= n_lookup <= <size_t>mask: # Don't loop forever
469
467
        slot = &table[i & mask]
470
468
        cur = slot[0]