Merge pull request #55 from zopefoundation/fix-64bit-win

Fix Python timestamp hash on 64-bit windows

CHANGES.rst

 ``persistent`` Changelog
 ========================
 
+4.2.3 (unreleased)
+------------------
+
+- Fix the hashcode of Python ``TimeStamp`` objects on 64-bit Python on
+  Windows. See https://github.com/zopefoundation/persistent/pull/55
+
 4.2.2 (2016-11-29)
 ------------------
 
 - Drop use of ``ctypes`` for determining maximum integer size, to increase
-  pure-Python compatibility.
+  pure-Python compatibility. See https://github.com/zopefoundation/persistent/pull/31
 
 - Ensure that ``__slots__`` attributes are cleared when a persistent
   object is ghostified.  (This excluses classes that override

persistent/tests/test_timestamp.py

@@ -12,7 +12,7 @@
 #
 ##############################################################################
 import unittest
-
+import sys
 
 MAX_32_BITS = 2 ** 31 - 1
 MAX_64_BITS = 2 ** 63 - 1
@@ -277,12 +277,29 @@ class PyAndCComparisonTests(unittest.TestCase):
         bit_32_hash = -1419374591
         bit_64_hash = -3850693964765720575
         orig_maxint = MUT._MAXINT
+
+        is_32_bit_hash = orig_maxint == MAX_32_BITS
+
+        orig_c_long = None
+        c_int64 = None
+        c_int32 = None
+        if hasattr(MUT, 'c_long'):
+            import ctypes
+            orig_c_long = MUT.c_long
+            c_int32 = ctypes.c_int32
+            c_int64 = ctypes.c_int64
+            # win32, even on 64-bit long, has funny sizes
+            is_32_bit_hash = c_int32 == ctypes.c_long
+
         try:
             MUT._MAXINT = MAX_32_BITS
+            MUT.c_long = c_int32
+
             py = self._makePy(*self.now_ts_args)
             self.assertEqual(hash(py), bit_32_hash)
 
             MUT._MAXINT = int(2 ** 63 - 1)
+            MUT.c_long = c_int64
             # call __hash__ directly to avoid interpreter truncation
             # in hash() on 32-bit platforms
             if not self._is_jython:
@@ -299,68 +316,73 @@ class PyAndCComparisonTests(unittest.TestCase):
                     384009219096809580920179179233996861765753210540033)
         finally:
             MUT._MAXINT = orig_maxint
+            if orig_c_long is not None:
+                MUT.c_long = orig_c_long
+            else:
+                del MUT.c_long
 
         # These are *usually* aliases, but aren't required
         # to be (and aren't under Jython 2.7).
-        if orig_maxint == MAX_32_BITS:
+        if is_32_bit_hash:
             self.assertEqual(py.__hash__(), bit_32_hash)
-        elif orig_maxint == MAX_64_BITS:
+        else:
             self.assertEqual(py.__hash__(), bit_64_hash)
 
     def test_hash_equal_constants(self):
         # The simple constants make it easier to diagnose
         # a difference in algorithms
         import persistent.timestamp as MUT
-        # We get 32-bit hash values of 32-bit platforms, or on the JVM
-        is_32_bit = MUT._MAXINT == (2**31 - 1) or self._is_jython
+        # We get 32-bit hash values on 32-bit platforms, or on the JVM
+        # OR on Windows (whether compiled in 64 or 32-bit mode)
+        is_32_bit = MUT._MAXINT == (2**31 - 1) or self._is_jython or sys.platform == 'win32'
 
         c, py = self._make_C_and_Py(b'\x00\x00\x00\x00\x00\x00\x00\x00')
-        self.assertEqual(c.__hash__(), 8)
+        self.assertEqual(hash(c), 8)
         self.assertEqual(hash(c), hash(py))
 
         c, py = self._make_C_and_Py(b'\x00\x00\x00\x00\x00\x00\x00\x01')
-        self.assertEqual(c.__hash__(), 9)
+        self.assertEqual(hash(c), 9)
         self.assertEqual(hash(c), hash(py))
 
         c, py = self._make_C_and_Py(b'\x00\x00\x00\x00\x00\x00\x01\x00')
-        self.assertEqual(c.__hash__(), 1000011)
+        self.assertEqual(hash(c), 1000011)
         self.assertEqual(hash(c), hash(py))
 
         # overflow kicks in here on 32-bit platforms
         c, py = self._make_C_and_Py(b'\x00\x00\x00\x00\x00\x01\x00\x00')
         if is_32_bit:
-            self.assertEqual(c.__hash__(), -721379967)
+            self.assertEqual(hash(c), -721379967)
         else:
-            self.assertEqual(c.__hash__(), 1000006000001)
+            self.assertEqual(hash(c), 1000006000001)
         self.assertEqual(hash(c), hash(py))
 
         c, py = self._make_C_and_Py(b'\x00\x00\x00\x00\x01\x00\x00\x00')
         if is_32_bit:
-            self.assertEqual(c.__hash__(), 583896275)
+            self.assertEqual(hash(c), 583896275)
         else:
-            self.assertEqual(c.__hash__(), 1000009000027000019)
+            self.assertEqual(hash(c), 1000009000027000019)
         self.assertEqual(hash(c), hash(py))
 
         # Overflow kicks in at this point on 64-bit platforms
         c, py = self._make_C_and_Py(b'\x00\x00\x00\x01\x00\x00\x00\x00')
         if is_32_bit:
-            self.assertEqual(c.__hash__(), 1525764953)
+            self.assertEqual(hash(c), 1525764953)
         else:
-            self.assertEqual(c.__hash__(), -4442925868394654887)
+            self.assertEqual(hash(c), -4442925868394654887)
         self.assertEqual(hash(c), hash(py))
 
         c, py = self._make_C_and_Py(b'\x00\x00\x01\x00\x00\x00\x00\x00')
         if is_32_bit:
-            self.assertEqual(c.__hash__(), -429739973)
+            self.assertEqual(hash(c), -429739973)
         else:
-            self.assertEqual(c.__hash__(), -3993531167153147845)
+            self.assertEqual(hash(c), -3993531167153147845)
         self.assertEqual(hash(c), hash(py))
 
         c, py = self._make_C_and_Py(b'\x01\x00\x00\x00\x00\x00\x00\x00')
         if is_32_bit:
-            self.assertEqual(c.__hash__(), 263152323)
+            self.assertEqual(hash(c), 263152323)
         else:
-            self.assertEqual(c.__hash__(), -3099646879006235965)
+            self.assertEqual(hash(c), -3099646879006235965)
         self.assertEqual(hash(c), hash(py))
 
     def test_ordering(self):

persistent/timestamp.py

@@ -29,12 +29,22 @@ def _makeOctets(s):
 
 _ZERO = _makeOctets('\x00' * 8)
 
-
-def _wraparound(x):
+try:
     # Make sure to overflow and wraparound just
     # like the C code does.
-    return int(((x + (_MAXINT + 1)) & ((_MAXINT << 1) + 1)) - (_MAXINT + 1))
-
+    from ctypes import c_long
+except ImportError: # pragma: no cover
+    # XXX: This is broken on 64-bit windows, where
+    # sizeof(long) != sizeof(Py_ssize_t)
+    # sizeof(long) == 4, sizeof(Py_ssize_t) == 8
+    # It can be fixed by setting _MAXINT = 2 ** 31 - 1 on all
+    # win32 platforms, but then that breaks PyPy3 64 bit for an unknown
+    # reason.
+    def _wraparound(x):
+        return int(((x + (_MAXINT + 1)) & ((_MAXINT << 1) + 1)) - (_MAXINT + 1))
+else:
+    def _wraparound(x):
+        return c_long(x).value
 
 class _UTC(datetime.tzinfo):
     def tzname(self):