From ddce7cb3a93480d0e16ce63d46644a3f51541ba2 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Thu, 21 Feb 2019 18:13:05 +0100 Subject: [PATCH 01/29] ENH add a state_setter arg to save_reduce --- Lib/pickle.py | 30 ++++++++++++++--- Modules/_pickle.c | 86 +++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 105 insertions(+), 11 deletions(-) diff --git a/Lib/pickle.py b/Lib/pickle.py index d533e660af3b23..eab5e076cd2b40 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -537,9 +537,9 @@ def save(self, obj, save_persistent_id=True): # Assert that it returned an appropriately sized tuple l = len(rv) - if not (2 <= l <= 5): + if not (2 <= l <= 6): raise PicklingError("Tuple returned by %s must have " - "two to five elements" % reduce) + "two to six elements" % reduce) # Save the reduce() output and finally memoize the object self.save_reduce(obj=obj, *rv) @@ -561,7 +561,7 @@ def save_pers(self, pid): "persistent IDs in protocol 0 must be ASCII strings") def save_reduce(self, func, args, state=None, listitems=None, - dictitems=None, obj=None): + dictitems=None, state_setter=None, obj=None): # This API is called by some subclasses if not isinstance(args, tuple): @@ -655,7 +655,24 @@ def save_reduce(self, func, args, state=None, listitems=None, self._batch_setitems(dictitems) if state is not None: - save(state) + msg = "state must be either a dict or a tuple of length 2" + if state_setter is None: + save(state) + + elif isinstance(state, dict): + # If a state_setter is specified, and state is a dict, we could + # be tempted to save a (state_setter, state) as state, but this + # would collide with load_build's (state, slotstate) special + # handling. Therefore, create a new format for state saving: + # (state_setter, state, slotstate) + save((state_setter, state, None)) + elif isinstance(state, tuple): + if len(state) != 2: + raise PicklingError(msg) + save((state_setter, *state)) + else: + raise PicklingError(msg) + write(BUILD) # Methods below this point are dispatched through the dispatch table @@ -1547,6 +1564,11 @@ def load_build(self): slotstate = None if isinstance(state, tuple) and len(state) == 2: state, slotstate = state + + elif isinstance(state, tuple) and len(state) == 3: + setstate, state, slostate = state + setstate(inst, state, slotstate) + return if state: inst_dict = inst.__dict__ intern = sys.intern diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 391ce5e923c6d3..634bf8432121ba 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3662,6 +3662,7 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) PyObject *state = NULL; PyObject *listitems = Py_None; PyObject *dictitems = Py_None; + PyObject *state_setter = Py_None; PickleState *st = _Pickle_GetGlobalState(); Py_ssize_t size; int use_newobj = 0, use_newobj_ex = 0; @@ -3672,14 +3673,15 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) const char newobj_ex_op = NEWOBJ_EX; size = PyTuple_Size(args); - if (size < 2 || size > 5) { + if (size < 2 || size > 6) { PyErr_SetString(st->PicklingError, "tuple returned by " "__reduce__ must contain 2 through 5 elements"); return -1; } - if (!PyArg_UnpackTuple(args, "save_reduce", 2, 5, - &callable, &argtup, &state, &listitems, &dictitems)) + if (!PyArg_UnpackTuple(args, "save_reduce", 2, 6, + &callable, &argtup, &state, &listitems, &dictitems, + &state_setter)) return -1; if (!PyCallable_Check(callable)) { @@ -3714,6 +3716,15 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) return -1; } + if (state_setter == Py_None) + state_setter = NULL; + else if (!PyFunction_Check(state_setter)) { + PyErr_Format(st->PicklingError, "sixth element of the tuple " + "returned by __reduce__ must be an function, not %s", + Py_TYPE(state_setter)->tp_name); + return -1; + } + if (self->proto >= 2) { PyObject *name; _Py_IDENTIFIER(__name__); @@ -3933,11 +3944,48 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) return -1; if (state) { - if (save(self, state, 0) < 0 || - _Pickler_Write(self, &build_op, 1) < 0) - return -1; - } + if (state_setter == NULL) { + if (save(self, state, 0) < 0 || + _Pickler_Write(self, &build_op, 1) < 0) + return -1; + } + else { + PyObject *statetup = NULL; + + + /* If a state_setter is specified, and state is a dict, we could be + * tempted to save a (state_setter, state) as state, but this would + * collide with load_build's (state, slotstate) special handling. + * Therefore, create a new format for state saving: (state_setter, + * state, slotstate) + */ + if PyDict_Check(state) + statetup = Py_BuildValue("(OOO)", state_setter, state, + Py_None); + else if PyTuple_Check(state) { + if (PyTuple_GET_SIZE(state) == 2){ + statetup = Py_BuildValue("(OOO)", state_setter, + PyTuple_GetItem(state, 0), + PyTuple_GetItem(state, 1)); + } + } + + if (statetup == NULL) { + PyErr_SetString(st->PicklingError, + "state must be either a dict or a tuple of" + " length 2"); + return -1; + } + + if (save(self, statetup, 0) < 0 || + _Pickler_Write(self, &build_op, 1) < 0){ + Py_DECREF(statetup); + return -1; + } + Py_DECREF(statetup); + } + } return 0; } @@ -6235,6 +6283,30 @@ load_build(UnpicklerObject *self) Py_INCREF(slotstate); Py_DECREF(tmp); } + /* proto 5 addition: state can embed a callable state setter */ + else if (PyTuple_Check(state) && PyTuple_GET_SIZE(state) == 3) { + /* borrowed reference*/ + PyObject *tmp = state; + + setstate = PyTuple_GET_ITEM(tmp, 0); + state = PyTuple_GET_ITEM(tmp, 1); + slotstate = PyTuple_GET_ITEM(tmp, 2); + + Py_INCREF(setstate); + Py_INCREF(state); + Py_INCREF(slotstate); + Py_DECREF(tmp); + /* call the setstate function */ + if (PyObject_CallFunctionObjArgs(setstate, inst, state, + slotstate, NULL) == NULL){ + return -1; + } + + Py_DECREF(setstate); + Py_DECREF(state); + Py_DECREF(slotstate); + return 0; + } else slotstate = NULL; From cf86a1991d0558e1c4024cd0898b9b244af4016e Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Wed, 27 Mar 2019 15:12:35 +0100 Subject: [PATCH 02/29] MNT News section --- .../next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst diff --git a/Misc/NEWS.d/next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst b/Misc/NEWS.d/next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst new file mode 100644 index 00000000000000..dc2d26659cca00 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst @@ -0,0 +1,2 @@ +allow the user to speficy custom functions to set the state of an object in +save_reduce via the new state_setter keyword argument. From 907785959e779f0bd1b7b976ef82d8b73ae98c84 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Thu, 4 Apr 2019 11:39:02 +0200 Subject: [PATCH 03/29] TST test custom reducer using with state_setter --- Lib/test/pickletester.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index bb8e6ce0964fc4..d0cc3b410a7373 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -2994,6 +2994,19 @@ def __reduce__(self): class BBB(object): pass + +def setstate_bbb(obj, state, slotstate): + """Custom state setter for BBB objects + + Such callable may be created by other persons than the ones who created the + BBB class. If passed as the state_setter item of a custom reducer, this + allows for custom state setting behavior of BBB objects. One can think of + it as the analogous of list_setitems or dict_setitems but for foreign + classes/functions. + """ + obj.a = 'foo' + + class AbstractDispatchTableTests(unittest.TestCase): def test_default_dispatch_table(self): @@ -3081,6 +3094,19 @@ def reduce_2(obj): self.assertEqual(default_load_dump(a), REDUCE_A) self.assertIsInstance(default_load_dump(b), BBB) + # End-to-end testing of save_reduce with the state_setter keyword + # argument. This is a dispatch_table test as state_setter is useful to + # register custom state-setting behavior for classes that were not + # created by the user. + def reduce_bbb(obj): + return BBB, (), obj.__dict__, None, None, setstate_bbb + + dispatch_table[BBB] = reduce_bbb + + b = BBB() + + self.assertEqual(custom_load_dump(b).a, 'foo') + if __name__ == "__main__": # Print some stuff that can be used to rewrite DATA{0,1,2} From b388aa01c4e56dc9dfae86f2a89e4712823e2346 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Thu, 4 Apr 2019 12:17:40 +0200 Subject: [PATCH 04/29] DOC mention state_setter in pickle docs --- Doc/library/pickle.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Doc/library/pickle.rst b/Doc/library/pickle.rst index 53eb5d39ef9414..af896f07cf705a 100644 --- a/Doc/library/pickle.rst +++ b/Doc/library/pickle.rst @@ -629,6 +629,11 @@ or both. value``. This is primarily used for dictionary subclasses, but may be used by other classes as long as they implement :meth:`__setitem__`. + * Optionally, an callable with a ``(obj, state, slotstate)`` signature. This + callable allows the user to control the state-updating part of classes + the user did not create. If not ``None``, this callable will have priority + over any :meth:`__setitem__` method. + .. method:: object.__reduce_ex__(protocol) From 1f286c163bc172789350bfa662cd8d764c72a83e Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Thu, 4 Apr 2019 12:18:34 +0200 Subject: [PATCH 05/29] CLN do not expose slotstate in state_setter In order to remain consistent with how slotstate is handle at pickling time, the state_setter callable should have a ``(obj, state)`` signature. ``state`` will either be a dictionary, or, if the class implements ``__slots__``, a tuple with two dicts, state and slotstate. --- Lib/pickle.py | 7 +++++-- Lib/test/pickletester.py | 2 +- Modules/_pickle.c | 15 ++++++++++++--- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/Lib/pickle.py b/Lib/pickle.py index eab5e076cd2b40..76267922243063 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -1567,8 +1567,11 @@ def load_build(self): elif isinstance(state, tuple) and len(state) == 3: setstate, state, slostate = state - setstate(inst, state, slotstate) - return + if slotstate is None: + setstate(inst, (state, slotstate)) + else: + setstate(inst, state) + if state: inst_dict = inst.__dict__ intern = sys.intern diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index d0cc3b410a7373..5ab27c26cce2b6 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -2995,7 +2995,7 @@ class BBB(object): pass -def setstate_bbb(obj, state, slotstate): +def setstate_bbb(obj, state): """Custom state setter for BBB objects Such callable may be created by other persons than the ones who created the diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 634bf8432121ba..a231eec0b23019 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -6238,7 +6238,7 @@ load_additems(UnpicklerObject *self) static int load_build(UnpicklerObject *self) { - PyObject *state, *inst, *slotstate; + PyObject *state, *inst, *slotstate, *statearg; PyObject *setstate; int status = 0; _Py_IDENTIFIER(__setstate__); @@ -6297,13 +6297,22 @@ load_build(UnpicklerObject *self) Py_INCREF(slotstate); Py_DECREF(tmp); /* call the setstate function */ - if (PyObject_CallFunctionObjArgs(setstate, inst, state, - slotstate, NULL) == NULL){ + if (slotstate != Py_None) { + statearg = Py_BuildValue("(OO)", state, slotstate); + } + else { + statearg = state; + Py_INCREF(state); + } + + if (PyObject_CallFunctionObjArgs(setstate, inst, statearg, + NULL) == NULL){ return -1; } Py_DECREF(setstate); Py_DECREF(state); + Py_DECREF(statearg); Py_DECREF(slotstate); return 0; } From 8abbef1f25dab370e806651633b82ad3cfb17bf7 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Thu, 4 Apr 2019 12:21:27 +0200 Subject: [PATCH 06/29] DOC update the doc according to the last commit --- Doc/library/pickle.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/library/pickle.rst b/Doc/library/pickle.rst index af896f07cf705a..de2fa821254212 100644 --- a/Doc/library/pickle.rst +++ b/Doc/library/pickle.rst @@ -629,7 +629,7 @@ or both. value``. This is primarily used for dictionary subclasses, but may be used by other classes as long as they implement :meth:`__setitem__`. - * Optionally, an callable with a ``(obj, state, slotstate)`` signature. This + * Optionally, an callable with a ``(obj, state)`` signature. This callable allows the user to control the state-updating part of classes the user did not create. If not ``None``, this callable will have priority over any :meth:`__setitem__` method. From 4b46841e1dc44f1f2a020192897ac1ec080e6feb Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Tue, 16 Apr 2019 09:39:54 +0200 Subject: [PATCH 07/29] CLN load_build cleanups --- Modules/_pickle.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/Modules/_pickle.c b/Modules/_pickle.c index a231eec0b23019..392dc1f7b5b44e 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -6238,7 +6238,7 @@ load_additems(UnpicklerObject *self) static int load_build(UnpicklerObject *self) { - PyObject *state, *inst, *slotstate, *statearg; + PyObject *state, *inst, *slotstate; PyObject *setstate; int status = 0; _Py_IDENTIFIER(__setstate__); @@ -6285,35 +6285,23 @@ load_build(UnpicklerObject *self) } /* proto 5 addition: state can embed a callable state setter */ else if (PyTuple_Check(state) && PyTuple_GET_SIZE(state) == 3) { - /* borrowed reference*/ - PyObject *tmp = state; + PyObject *state_slotstate; - setstate = PyTuple_GET_ITEM(tmp, 0); - state = PyTuple_GET_ITEM(tmp, 1); - slotstate = PyTuple_GET_ITEM(tmp, 2); + setstate = PyTuple_GET_ITEM(state, 0); + state_slotstate = PyTuple_GetSlice(state, 1, 3); Py_INCREF(setstate); - Py_INCREF(state); - Py_INCREF(slotstate); - Py_DECREF(tmp); - /* call the setstate function */ - if (slotstate != Py_None) { - statearg = Py_BuildValue("(OO)", state, slotstate); - } - else { - statearg = state; - Py_INCREF(state); - } - if (PyObject_CallFunctionObjArgs(setstate, inst, statearg, + /* call the setstate function */ + if (PyObject_CallFunctionObjArgs(setstate, inst, state_slotstate, NULL) == NULL){ + Py_DECREF(state_slotstate); + Py_DECREF(setstate); return -1; } + Py_DECREF(state_slotstate); Py_DECREF(setstate); - Py_DECREF(state); - Py_DECREF(statearg); - Py_DECREF(slotstate); return 0; } else From 6c2c9577ec123d9b02e6a34bdba93a821bf851ac Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Tue, 16 Apr 2019 13:07:58 +0200 Subject: [PATCH 08/29] CLN do not mention protocol 5 --- Modules/_pickle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 392dc1f7b5b44e..c5cabf1412f76e 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -6283,7 +6283,7 @@ load_build(UnpicklerObject *self) Py_INCREF(slotstate); Py_DECREF(tmp); } - /* proto 5 addition: state can embed a callable state setter */ + /* state can embed a callable state setter */ else if (PyTuple_Check(state) && PyTuple_GET_SIZE(state) == 3) { PyObject *state_slotstate; From d7f87141f6f81778eda0c750c887e15cbe71acee Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Tue, 16 Apr 2019 13:24:57 +0200 Subject: [PATCH 09/29] CLN style --- Modules/_pickle.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/_pickle.c b/Modules/_pickle.c index c5cabf1412f76e..9629a4b9cbe906 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3952,7 +3952,6 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) else { PyObject *statetup = NULL; - /* If a state_setter is specified, and state is a dict, we could be * tempted to save a (state_setter, state) as state, but this would * collide with load_build's (state, slotstate) special handling. @@ -3963,7 +3962,7 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) statetup = Py_BuildValue("(OOO)", state_setter, state, Py_None); else if PyTuple_Check(state) { - if (PyTuple_GET_SIZE(state) == 2){ + if (PyTuple_GET_SIZE(state) == 2) { statetup = Py_BuildValue("(OOO)", state_setter, PyTuple_GetItem(state, 0), PyTuple_GetItem(state, 1)); From d2534416175cb9c794928c126a241c877349380e Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Fri, 19 Apr 2019 11:38:32 +0200 Subject: [PATCH 10/29] CLN style and comments --- Lib/test/pickletester.py | 7 ++++--- Modules/_pickle.c | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 5ab27c26cce2b6..de44361efdd738 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -3095,9 +3095,10 @@ def reduce_2(obj): self.assertIsInstance(default_load_dump(b), BBB) # End-to-end testing of save_reduce with the state_setter keyword - # argument. This is a dispatch_table test as state_setter is useful to - # register custom state-setting behavior for classes that were not - # created by the user. + # argument. This is a dispatch_table test as the primary goal of + # state_setter is to tweak objects reduction behavior. + # In particular, state_setter is useful when the default __setstate__ + # behavior is not flexible enough. def reduce_bbb(obj): return BBB, (), obj.__dict__, None, None, setstate_bbb diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 9629a4b9cbe906..34e071075951f5 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3977,7 +3977,7 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) } if (save(self, statetup, 0) < 0 || - _Pickler_Write(self, &build_op, 1) < 0){ + _Pickler_Write(self, &build_op, 1) < 0) { Py_DECREF(statetup); return -1; } From 48e4686408b8df37f00ccb248c928f59cb822a0e Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Fri, 19 Apr 2019 11:45:43 +0200 Subject: [PATCH 11/29] DOC update doc --- Doc/library/pickle.rst | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Doc/library/pickle.rst b/Doc/library/pickle.rst index de2fa821254212..2e500cd8a82107 100644 --- a/Doc/library/pickle.rst +++ b/Doc/library/pickle.rst @@ -630,9 +630,10 @@ or both. by other classes as long as they implement :meth:`__setitem__`. * Optionally, an callable with a ``(obj, state)`` signature. This - callable allows the user to control the state-updating part of classes - the user did not create. If not ``None``, this callable will have priority - over any :meth:`__setitem__` method. + callable allows the user to programatically control the state-updating + behavior of a specific object, instead of using ``obj``'s static + :meth:`__setstate__` method. If not ``None``, this callable will have + priority over ``obj``'s :meth:`__setstate__`. .. method:: object.__reduce_ex__(protocol) From e83f940507de3b01da045aa7c3a7affff7dea6f8 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Fri, 19 Apr 2019 11:46:22 +0200 Subject: [PATCH 12/29] CLN typo --- Doc/library/pickle.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/library/pickle.rst b/Doc/library/pickle.rst index 2e500cd8a82107..7b09722f0ac68d 100644 --- a/Doc/library/pickle.rst +++ b/Doc/library/pickle.rst @@ -629,7 +629,7 @@ or both. value``. This is primarily used for dictionary subclasses, but may be used by other classes as long as they implement :meth:`__setitem__`. - * Optionally, an callable with a ``(obj, state)`` signature. This + * Optionally, a callable with a ``(obj, state)`` signature. This callable allows the user to programatically control the state-updating behavior of a specific object, instead of using ``obj``'s static :meth:`__setstate__` method. If not ``None``, this callable will have From 83d3a2b9aad0aa7022e54a4150dfcfa21791b939 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Tue, 23 Apr 2019 16:36:26 +0200 Subject: [PATCH 13/29] MNT dont pervert BUILD if state_setter is specified --- Lib/test/pickletester.py | 2 +- Modules/_pickle.c | 41 +++++++++------------------------------- 2 files changed, 10 insertions(+), 33 deletions(-) diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index de44361efdd738..d5c8801c25a847 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -3005,7 +3005,7 @@ def setstate_bbb(obj, state): classes/functions. """ obj.a = 'foo' - + return obj class AbstractDispatchTableTests(unittest.TestCase): diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 34e071075951f5..ac7ec0e3104201 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3950,40 +3950,17 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) return -1; } else { - PyObject *statetup = NULL; - - /* If a state_setter is specified, and state is a dict, we could be - * tempted to save a (state_setter, state) as state, but this would - * collide with load_build's (state, slotstate) special handling. - * Therefore, create a new format for state saving: (state_setter, - * state, slotstate) - */ - if PyDict_Check(state) - statetup = Py_BuildValue("(OOO)", state_setter, state, - Py_None); - else if PyTuple_Check(state) { - if (PyTuple_GET_SIZE(state) == 2) { - statetup = Py_BuildValue("(OOO)", state_setter, - PyTuple_GetItem(state, 0), - PyTuple_GetItem(state, 1)); - } - } - - if (statetup == NULL) { - PyErr_SetString(st->PicklingError, - "state must be either a dict or a tuple of" - " length 2"); - return -1; - } - - if (save(self, statetup, 0) < 0 || - _Pickler_Write(self, &build_op, 1) < 0) { - Py_DECREF(statetup); + const char tupletwo_op = TUPLE2; + const char pop_op = POP; + if ( + _Pickler_Write(self, &pop_op, 1) < 0 || + save(self, state_setter, 0) < 0 || + save(self, obj, 0) < 0 || + save(self, state, 0) < 0 || + _Pickler_Write(self, &tupletwo_op, 1) < 0 || + _Pickler_Write(self, &reduce_op, 1) < 0) return -1; - } - Py_DECREF(statetup); } - } return 0; } From 68b3a5db19804a23ffac8e9786b3fe71e3d15c47 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Tue, 23 Apr 2019 16:55:41 +0200 Subject: [PATCH 14/29] CLN style --- Modules/_pickle.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/_pickle.c b/Modules/_pickle.c index ac7ec0e3104201..91ddbe7ad76d44 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3952,8 +3952,7 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) else { const char tupletwo_op = TUPLE2; const char pop_op = POP; - if ( - _Pickler_Write(self, &pop_op, 1) < 0 || + if (_Pickler_Write(self, &pop_op, 1) < 0 || save(self, state_setter, 0) < 0 || save(self, obj, 0) < 0 || save(self, state, 0) < 0 || From f311ef6798a023c5b1700ead308525ab60023024 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Tue, 23 Apr 2019 17:08:38 +0200 Subject: [PATCH 15/29] ENH pop state_setter's output of the pickler stack --- Lib/test/pickletester.py | 1 - Modules/_pickle.c | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index d5c8801c25a847..4801d0cf234f20 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -3005,7 +3005,6 @@ def setstate_bbb(obj, state): classes/functions. """ obj.a = 'foo' - return obj class AbstractDispatchTableTests(unittest.TestCase): diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 91ddbe7ad76d44..2b125c5635a525 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3952,12 +3952,12 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) else { const char tupletwo_op = TUPLE2; const char pop_op = POP; - if (_Pickler_Write(self, &pop_op, 1) < 0 || - save(self, state_setter, 0) < 0 || + if (save(self, state_setter, 0) < 0 || save(self, obj, 0) < 0 || save(self, state, 0) < 0 || _Pickler_Write(self, &tupletwo_op, 1) < 0 || - _Pickler_Write(self, &reduce_op, 1) < 0) + _Pickler_Write(self, &reduce_op, 1) < 0 || + _Pickler_Write(self, &pop_op, 1) < 0) return -1; } } From 744525eff91f70bbe75a11e4d24292fb247722e3 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Tue, 23 Apr 2019 18:20:41 +0200 Subject: [PATCH 16/29] DOC write code explanation comments --- Modules/_pickle.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 2b125c5635a525..d736a15e9d4e01 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3950,11 +3950,20 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) return -1; } else { + + /* If a state_setter is specified, call it instead of load_build to + * update obj's with its previous state. + * The first 4 save/write instructions push state_setter and its + * tuple of expected arguments (obj, state) onto the stack. The + * REDUCE opcode triggers the state_setter(obj, state) function + * call. Finally, because state-updating routines only do in-place + * modification, the whole operation has to be stack-transparent. + * Thus, we finally pop the call's output from the stack.*/ + const char tupletwo_op = TUPLE2; const char pop_op = POP; if (save(self, state_setter, 0) < 0 || - save(self, obj, 0) < 0 || - save(self, state, 0) < 0 || + save(self, obj, 0) < 0 || save(self, state, 0) < 0 || _Pickler_Write(self, &tupletwo_op, 1) < 0 || _Pickler_Write(self, &reduce_op, 1) < 0 || _Pickler_Write(self, &pop_op, 1) < 0) From a2687d346273287adac1bedc1330fffe96bd2d4b Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Wed, 24 Apr 2019 15:58:04 +0200 Subject: [PATCH 17/29] CLN spurious line deletion --- Lib/test/pickletester.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 4801d0cf234f20..de44361efdd738 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -3006,6 +3006,7 @@ def setstate_bbb(obj, state): """ obj.a = 'foo' + class AbstractDispatchTableTests(unittest.TestCase): def test_default_dispatch_table(self): From 1dadb11c4afbcc7437e17275902d2d82bf9c1a3e Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Wed, 24 Apr 2019 16:12:14 +0200 Subject: [PATCH 18/29] CLN remove stale state_setter handling in load_build --- Modules/_pickle.c | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/Modules/_pickle.c b/Modules/_pickle.c index d736a15e9d4e01..5aee71ca2e099d 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -6267,27 +6267,6 @@ load_build(UnpicklerObject *self) Py_INCREF(slotstate); Py_DECREF(tmp); } - /* state can embed a callable state setter */ - else if (PyTuple_Check(state) && PyTuple_GET_SIZE(state) == 3) { - PyObject *state_slotstate; - - setstate = PyTuple_GET_ITEM(state, 0); - state_slotstate = PyTuple_GetSlice(state, 1, 3); - - Py_INCREF(setstate); - - /* call the setstate function */ - if (PyObject_CallFunctionObjArgs(setstate, inst, state_slotstate, - NULL) == NULL){ - Py_DECREF(state_slotstate); - Py_DECREF(setstate); - return -1; - } - - Py_DECREF(state_slotstate); - Py_DECREF(setstate); - return 0; - } else slotstate = NULL; From ab3cef223ab008beb6a65704013169e1418766af Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Wed, 24 Apr 2019 16:13:15 +0200 Subject: [PATCH 19/29] ENH implement the same mechanism in pickle.py --- Lib/pickle.py | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/Lib/pickle.py b/Lib/pickle.py index 76267922243063..a5c289b7257e8c 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -655,25 +655,22 @@ def save_reduce(self, func, args, state=None, listitems=None, self._batch_setitems(dictitems) if state is not None: - msg = "state must be either a dict or a tuple of length 2" if state_setter is None: save(state) - - elif isinstance(state, dict): - # If a state_setter is specified, and state is a dict, we could - # be tempted to save a (state_setter, state) as state, but this - # would collide with load_build's (state, slotstate) special - # handling. Therefore, create a new format for state saving: - # (state_setter, state, slotstate) - save((state_setter, state, None)) - elif isinstance(state, tuple): - if len(state) != 2: - raise PicklingError(msg) - save((state_setter, *state)) + write(BUILD) else: - raise PicklingError(msg) - - write(BUILD) + # If a state_setter is specified, call it instead of load_build + # to update obj's with its previous state. The first 4 + # save/write instructions push state_setter and its tuple of + # expected arguments (obj, state) onto the stack. The REDUCE + # opcode triggers the state_setter(obj, state) function call. + # Finally, because state-updating routines only do in-place + # modification, the whole operation has to be + # stack-transparent. Thus, we finally pop the call's output + # from the stack. + save(state_setter), save(obj), save(state), write(TUPLE2) + write(REDUCE) + write(POP) # Methods below this point are dispatched through the dispatch table @@ -1564,14 +1561,6 @@ def load_build(self): slotstate = None if isinstance(state, tuple) and len(state) == 2: state, slotstate = state - - elif isinstance(state, tuple) and len(state) == 3: - setstate, state, slostate = state - if slotstate is None: - setstate(inst, (state, slotstate)) - else: - setstate(inst, state) - if state: inst_dict = inst.__dict__ intern = sys.intern From 3fd958b3998569b9dc416079e9bb4b822b82b7ad Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Wed, 24 Apr 2019 16:59:13 +0200 Subject: [PATCH 20/29] CLN little refactoring of pickle.py (comment + style) --- Lib/pickle.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/Lib/pickle.py b/Lib/pickle.py index a5c289b7257e8c..10a3955a864473 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -660,16 +660,19 @@ def save_reduce(self, func, args, state=None, listitems=None, write(BUILD) else: # If a state_setter is specified, call it instead of load_build - # to update obj's with its previous state. The first 4 - # save/write instructions push state_setter and its tuple of - # expected arguments (obj, state) onto the stack. The REDUCE - # opcode triggers the state_setter(obj, state) function call. - # Finally, because state-updating routines only do in-place - # modification, the whole operation has to be - # stack-transparent. Thus, we finally pop the call's output - # from the stack. - save(state_setter), save(obj), save(state), write(TUPLE2) + # to update obj's with its previous state. + # First, push state_setter and its tuple of expected arguments + # (obj, state) onto the stack. + save(state_setter) + save(obj) + save(state) + write(TUPLE2) + # Trigger a state_setter(obj, state) function call. write(REDUCE) + # The purpose of state_setter is to carry-out an + # inplace modification of obj. We do not care about what the + # method might return, so its output is eventually removed from + # the stack. write(POP) # Methods below this point are dispatched through the dispatch table From 785c0673154157ca12c6212c0fc6f584855e34df Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Fri, 26 Apr 2019 10:31:21 +0200 Subject: [PATCH 21/29] MNT versionadded directive --- Doc/library/pickle.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Doc/library/pickle.rst b/Doc/library/pickle.rst index 7b09722f0ac68d..af8340ac224198 100644 --- a/Doc/library/pickle.rst +++ b/Doc/library/pickle.rst @@ -635,6 +635,9 @@ or both. :meth:`__setstate__` method. If not ``None``, this callable will have priority over ``obj``'s :meth:`__setstate__`. + .. versionadded:: 3.8 + The ``state_setter`` option was added. + .. method:: object.__reduce_ex__(protocol) From bb2882a3363aaa9237b7ad14a7f0dcc315ce1988 Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Sat, 27 Apr 2019 03:10:31 +1000 Subject: [PATCH 22/29] Fix typo --- .../next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst b/Misc/NEWS.d/next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst index dc2d26659cca00..a761cb01022479 100644 --- a/Misc/NEWS.d/next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst +++ b/Misc/NEWS.d/next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst @@ -1,2 +1,2 @@ -allow the user to speficy custom functions to set the state of an object in +Allow the user to specify custom functions to set the state of an object in save_reduce via the new state_setter keyword argument. From c0ea035081fc40f71483db5e8c4ebfab104a1344 Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Sat, 27 Apr 2019 03:12:32 +1000 Subject: [PATCH 23/29] Fix copy-and-paste issue in error message --- Modules/_pickle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 5aee71ca2e099d..bd8f0d3ca8646f 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3720,7 +3720,7 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) state_setter = NULL; else if (!PyFunction_Check(state_setter)) { PyErr_Format(st->PicklingError, "sixth element of the tuple " - "returned by __reduce__ must be an function, not %s", + "returned by __reduce__ must be a function, not %s", Py_TYPE(state_setter)->tp_name); return -1; } From 4450b7b998315b1328619a3fda27135dccd59c8e Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Sat, 27 Apr 2019 03:13:37 +1000 Subject: [PATCH 24/29] Fix reduce result length checking error message --- Modules/_pickle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_pickle.c b/Modules/_pickle.c index bd8f0d3ca8646f..17a8808fe1f149 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3675,7 +3675,7 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) size = PyTuple_Size(args); if (size < 2 || size > 6) { PyErr_SetString(st->PicklingError, "tuple returned by " - "__reduce__ must contain 2 through 5 elements"); + "__reduce__ must contain 2 through 6 elements"); return -1; } From c7811033fb5960b79aaf0344d8dec645dae38458 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Wed, 8 May 2019 14:14:55 +0200 Subject: [PATCH 25/29] DOC rephrasings --- Doc/library/pickle.rst | 4 ++-- .../next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Doc/library/pickle.rst b/Doc/library/pickle.rst index af8340ac224198..3d89536d7d1182 100644 --- a/Doc/library/pickle.rst +++ b/Doc/library/pickle.rst @@ -598,7 +598,7 @@ or both. module; the pickle module searches the module namespace to determine the object's module. This behaviour is typically useful for singletons. - When a tuple is returned, it must be between two and five items long. + When a tuple is returned, it must be between two and six items long. Optional items can either be omitted, or ``None`` can be provided as their value. The semantics of each item are in order: @@ -636,7 +636,7 @@ or both. priority over ``obj``'s :meth:`__setstate__`. .. versionadded:: 3.8 - The ``state_setter`` option was added. + The optional sixth tuple item, ``(obj, state)``, was added. .. method:: object.__reduce_ex__(protocol) diff --git a/Misc/NEWS.d/next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst b/Misc/NEWS.d/next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst index a761cb01022479..7f3a0675cdab94 100644 --- a/Misc/NEWS.d/next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst +++ b/Misc/NEWS.d/next/Library/2019-03-27-15-09-00.bpo-35900.fh56UU.rst @@ -1,2 +1,3 @@ -Allow the user to specify custom functions to set the state of an object in -save_reduce via the new state_setter keyword argument. +Allow reduction methods to return a 6-item tuple where the 6th item specifies a +custom state-setting method that's called instead of the regular +``__setstate__`` method. From 5404bcd3d4e45172b6bfb010d6ee5b8f2c9cfba3 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Wed, 8 May 2019 15:14:42 +0200 Subject: [PATCH 26/29] DOC comments --- Lib/pickle.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/pickle.py b/Lib/pickle.py index 10a3955a864473..47f0d280efc945 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -664,7 +664,7 @@ def save_reduce(self, func, args, state=None, listitems=None, # First, push state_setter and its tuple of expected arguments # (obj, state) onto the stack. save(state_setter) - save(obj) + save(obj) # simple BINGET opcode as obj is already memoized. save(state) write(TUPLE2) # Trigger a state_setter(obj, state) function call. From 88d19c07557650165bc71bbc8b6ae9ddf9575038 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Wed, 8 May 2019 15:39:26 +0200 Subject: [PATCH 27/29] TST test priority of state_setter over __setstate__ --- Lib/test/pickletester.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index de44361efdd738..b0368e67e9a501 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -2992,7 +2992,13 @@ def __reduce__(self): return str, (REDUCE_A,) class BBB(object): - pass + def __init__(self): + # Add an instance attribute to enable state-saving routines at pickling + # time. + self.a = "some attribute" + + def __setstate__(self, state): + self.a = "BBB.__setstate__" def setstate_bbb(obj, state): @@ -3004,7 +3010,7 @@ def setstate_bbb(obj, state): it as the analogous of list_setitems or dict_setitems but for foreign classes/functions. """ - obj.a = 'foo' + obj.a = "custom state_setter" class AbstractDispatchTableTests(unittest.TestCase): @@ -3099,6 +3105,11 @@ def reduce_2(obj): # state_setter is to tweak objects reduction behavior. # In particular, state_setter is useful when the default __setstate__ # behavior is not flexible enough. + + # No custom reducer for b has been registered for now, so + # BBB.__setstate__ should be used at unpickling time + self.assertEqual(default_load_dump(b).a, "BBB.__setstate__") + def reduce_bbb(obj): return BBB, (), obj.__dict__, None, None, setstate_bbb @@ -3106,7 +3117,9 @@ def reduce_bbb(obj): b = BBB() - self.assertEqual(custom_load_dump(b).a, 'foo') + # The custom reducer reduce_bbb includes a state setter, that should + # have priority over BBB.__setstate__ + self.assertEqual(custom_load_dump(b).a, "custom state_setter") if __name__ == "__main__": From 183f2b01335ec5fc9d175df5eb78788de3ebddc5 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Wed, 8 May 2019 15:41:53 +0200 Subject: [PATCH 28/29] TST, CLN remove unnecessary instance recreation --- Lib/test/pickletester.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index b0368e67e9a501..19e8823a731035 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -3115,8 +3115,6 @@ def reduce_bbb(obj): dispatch_table[BBB] = reduce_bbb - b = BBB() - # The custom reducer reduce_bbb includes a state setter, that should # have priority over BBB.__setstate__ self.assertEqual(custom_load_dump(b).a, "custom state_setter") From 3f2fdb2eb1bdbb7661007a49eabe738003889e81 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Wed, 8 May 2019 16:49:43 +0200 Subject: [PATCH 29/29] FIX allow state_setter to be an arbitrary callable --- Modules/_pickle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 17a8808fe1f149..897bbe1f24e46a 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3718,7 +3718,7 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) if (state_setter == Py_None) state_setter = NULL; - else if (!PyFunction_Check(state_setter)) { + else if (!PyCallable_Check(state_setter)) { PyErr_Format(st->PicklingError, "sixth element of the tuple " "returned by __reduce__ must be a function, not %s", Py_TYPE(state_setter)->tp_name);