bpo-34722: Consistent serialization of sets in bytecode #9472
Conversation
| Py_DECREF(value); | ||
| for (i = 0; i < n; i++) { | ||
| value = PyList_GetItem(l, i); | ||
| value = PyMarshal_WriteObjectToString(value, Py_MARSHAL_VERSION); |
There was a problem hiding this comment.
This will lost the identity of objects. For example, in {(o, 1), (o, 2)} you will get different objects after marshalling/unmarshalling.
There was a problem hiding this comment.
I'm not sure I can get the same objects back at present?
def test_object_identity(self):
o = 'test'
obj = {(o, 1), (o, 2)}
data = marshal.dumps(obj)
v = marshal.loads(data)
ids_before = {id(x) for x in obj}
ids_after = {id(x) for x in marshal.loads(data)}
self.assertEqual(ids_before, ids_after)
AssertionError: Items in the first set but not the second:
139864671036808
139864645838728
Maybe I'm misunderstanding what you mean, or that's not a good test case?
There was a problem hiding this comment.
Additional tests were added in #13736. Rebase your PR and make the tests be success.
| w_object(value, p); | ||
| Py_DECREF(value); | ||
| for (i = 0; i < n; i++) { | ||
| value = PyList_GetItem(l, i); |
There was a problem hiding this comment.
I think the PyList_GetItem() and PyList_SetItem() calls added in this patch should be replaced with the PyList_GET_ITEM() and PyList_SET_ITEM() macros.
Also, PyMarshal_WriteObjectToString() should be checked for failure.
|
Close this because #27926 was merged. |
This ensures that sets / frozensets marshal in a consistent order by sorting the serialised items before writing them. That will obviously make serialisation of such types somewhat slower.
Added a test for the case in question; it needs to use
compileallas a subprocess in order to test with different hash seeds.https://bugs.python.org/issue34722