Issue #16231: Allow false values other than None to be used as persistent IDs.

3 files changed, 28 insertions, 19 deletions

diff --git a/Lib/pickle.py b/Lib/pickle.py
index 508e858..299de16 100644
--- a/Lib/pickle.py
+++ b/Lib/pickle.py
@@ -269,7 +269,7 @@ class Pickler:
     def save(self, obj):
         # Check for persistent id (defined by a subclass)
         pid = self.persistent_id(obj)
-        if pid:
+        if pid is not None:
             self.save_pers(pid)
             return
 
diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py
index 34cafcb..1599893 100644
--- a/Lib/test/pickletester.py
+++ b/Lib/test/pickletester.py
@@ -1152,30 +1152,34 @@ class AbstractPersistentPicklerTests(unittest.TestCase):
         if isinstance(object, int) and object % 2 == 0:
             self.id_count += 1
             return str(object)
+        elif object == "test_false_value":
+            self.false_count += 1
+            return ""
         else:
             return None
 
     def persistent_load(self, oid):
-        self.load_count += 1
-        object = int(oid)
-        assert object % 2 == 0
-        return object
+        if not oid:
+            self.load_false_count += 1
+            return "test_false_value"
+        else:
+            self.load_count += 1
+            object = int(oid)
+            assert object % 2 == 0
+            return object
 
     def test_persistence(self):
-        self.id_count = 0
-        self.load_count = 0
-        L = range(10)
-        self.assertEqual(self.loads(self.dumps(L)), L)
-        self.assertEqual(self.id_count, 5)
-        self.assertEqual(self.load_count, 5)
-
-    def test_bin_persistence(self):
-        self.id_count = 0
-        self.load_count = 0
-        L = range(10)
-        self.assertEqual(self.loads(self.dumps(L, 1)), L)
-        self.assertEqual(self.id_count, 5)
-        self.assertEqual(self.load_count, 5)
+        L = range(10) + ["test_false_value"]
+        for proto in protocols:
+            self.id_count = 0
+            self.false_count = 0
+            self.load_false_count = 0
+            self.load_count = 0
+            self.assertEqual(self.loads(self.dumps(L, proto)), L)
+            self.assertEqual(self.id_count, 5)
+            self.assertEqual(self.false_count, 1)
+            self.assertEqual(self.load_count, 5)
+            self.assertEqual(self.load_false_count, 1)
 
 class AbstractPicklerUnpicklerObjectTests(unittest.TestCase):
 
diff --git a/Misc/NEWS b/Misc/NEWS
index e894f3e..0581dd2 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -15,6 +15,11 @@ Core and Builtins
 Library
 -------
 
+- Issue #16231: Fixed pickle.Pickler to only fallback to its default pickling
+  behaviour when Pickler.persistent_id returns None, but not for any other
+  false values.  This allows false values other values other than None to be
+  used as persistent IDs.  This behaviour is consistent with cPickle.
+
 - Issue #11508: Fixed uuid.getnode() and uuid.uuid1() on environment with
   virtual interface.  Original patch by Kent Frazier.