diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 021adcc408d..444a1036085 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -1153,16 +1153,62 @@ class AbstractPickleTests(unittest.TestCase): self.assertGreaterEqual(num_additems, 2) def test_simple_newobj(self): - x = object.__new__(SimpleNewObj) # avoid __init__ + x = SimpleNewObj.__new__(SimpleNewObj, 0xface) # avoid __init__ x.abc = 666 for proto in protocols: - s = self.dumps(x, proto) - self.assertEqual(opcode_in_pickle(pickle.NEWOBJ, s), - 2 <= proto < 4) - self.assertEqual(opcode_in_pickle(pickle.NEWOBJ_EX, s), - proto >= 4) - y = self.loads(s) # will raise TypeError if __init__ called - self.assert_is_copy(x, y) + with self.subTest(proto=proto): + s = self.dumps(x, proto) + if proto < 1: + self.assertIn(b'\nL64206', s) # LONG + else: + self.assertIn(b'M\xce\xfa', s) # BININT2 + self.assertEqual(opcode_in_pickle(pickle.NEWOBJ, s), + 2 <= proto) + self.assertFalse(opcode_in_pickle(pickle.NEWOBJ_EX, s)) + y = self.loads(s) # will raise TypeError if __init__ called + self.assert_is_copy(x, y) + + def test_complex_newobj(self): + x = ComplexNewObj.__new__(ComplexNewObj, 0xface) # avoid __init__ + x.abc = 666 + for proto in protocols: + with self.subTest(proto=proto): + s = self.dumps(x, proto) + if proto < 1: + self.assertIn(b'\nL64206', s) # LONG + elif proto < 2: + self.assertIn(b'M\xce\xfa', s) # BININT2 + elif proto < 4: + self.assertIn(b'X\x04\x00\x00\x00FACE', s) # BINUNICODE + else: + self.assertIn(b'\x8c\x04FACE', s) # SHORT_BINUNICODE + self.assertEqual(opcode_in_pickle(pickle.NEWOBJ, s), + 2 <= proto) + self.assertFalse(opcode_in_pickle(pickle.NEWOBJ_EX, s)) + y = self.loads(s) # will raise TypeError if __init__ called + self.assert_is_copy(x, y) + + def test_complex_newobj_ex(self): + x = ComplexNewObjEx.__new__(ComplexNewObjEx, 0xface) # avoid __init__ + x.abc = 666 + for proto in protocols: + with self.subTest(proto=proto): + if 2 <= proto < 4: + self.assertRaises(ValueError, self.dumps, x, proto) + continue + s = self.dumps(x, proto) + if proto < 1: + self.assertIn(b'\nL64206', s) # LONG + elif proto < 2: + self.assertIn(b'M\xce\xfa', s) # BININT2 + else: + assert proto >= 4 + self.assertIn(b'\x8c\x04FACE', s) # SHORT_BINUNICODE + self.assertFalse(opcode_in_pickle(pickle.NEWOBJ, s)) + self.assertEqual(opcode_in_pickle(pickle.NEWOBJ_EX, s), + 4 <= proto) + y = self.loads(s) # will raise TypeError if __init__ called + self.assert_is_copy(x, y) def test_newobj_list_slots(self): x = SlotList([1, 2, 3]) @@ -1891,12 +1937,20 @@ myclasses = [MyInt, MyFloat, class SlotList(MyList): __slots__ = ["foo"] -class SimpleNewObj(object): - def __init__(self, a, b, c): +class SimpleNewObj(int): + def __init__(self, *args, **kwargs): # raise an error, to make sure this isn't called raise TypeError("SimpleNewObj.__init__() didn't expect to get called") def __eq__(self, other): - return self.__dict__ == other.__dict__ + return int(self) == int(other) and self.__dict__ == other.__dict__ + +class ComplexNewObj(SimpleNewObj): + def __getnewargs__(self): + return ('%X' % self, 16) + +class ComplexNewObjEx(SimpleNewObj): + def __getnewargs_ex__(self): + return ('%X' % self,), {'base': 16} class BadGetattr: def __getattr__(self, key): diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index 39782a41f79..0c88fd282e3 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -4592,26 +4592,15 @@ class PicklingTests(unittest.TestCase): def _check_reduce(self, proto, obj, args=(), kwargs={}, state=None, listitems=None, dictitems=None): - if proto >= 4: + if proto >= 2: reduce_value = obj.__reduce_ex__(proto) - self.assertEqual(reduce_value[:3], - (copyreg.__newobj_ex__, - (type(obj), args, kwargs), - state)) - if listitems is not None: - self.assertListEqual(list(reduce_value[3]), listitems) + if kwargs: + self.assertEqual(reduce_value[0], copyreg.__newobj_ex__) + self.assertEqual(reduce_value[1], (type(obj), args, kwargs)) else: - self.assertIsNone(reduce_value[3]) - if dictitems is not None: - self.assertDictEqual(dict(reduce_value[4]), dictitems) - else: - self.assertIsNone(reduce_value[4]) - elif proto >= 2: - reduce_value = obj.__reduce_ex__(proto) - self.assertEqual(reduce_value[:3], - (copyreg.__newobj__, - (type(obj),) + args, - state)) + self.assertEqual(reduce_value[0], copyreg.__newobj__) + self.assertEqual(reduce_value[1], (type(obj),) + args) + self.assertEqual(reduce_value[2], state) if listitems is not None: self.assertListEqual(list(reduce_value[3]), listitems) else: diff --git a/Misc/NEWS b/Misc/NEWS index fcff454a66d..0e5dd2b53d8 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -196,6 +196,9 @@ Core and Builtins Library ------- +- Issue #22783: Pickling now uses the NEWOBJ opcode instead of the NEWOBJ_EX + opcode if possible. + - Issue #15513: Added a __sizeof__ implementation for pickle classes. - Issue #19858: pickletools.optimize() now aware of the MEMOIZE opcode, can diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 6416233fc6e..7a234f1dfff 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3506,20 +3506,19 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) } PyErr_Clear(); } - else if (self->proto >= 4) { - _Py_IDENTIFIER(__newobj_ex__); - use_newobj_ex = PyUnicode_Check(name) && - PyUnicode_Compare( - name, _PyUnicode_FromId(&PyId___newobj_ex__)) == 0; - Py_DECREF(name); - } - else { - _Py_IDENTIFIER(__newobj__); - use_newobj = PyUnicode_Check(name) && - PyUnicode_Compare( - name, _PyUnicode_FromId(&PyId___newobj__)) == 0; - Py_DECREF(name); + else if (PyUnicode_Check(name)) { + if (self->proto >= 4) { + _Py_IDENTIFIER(__newobj_ex__); + use_newobj_ex = PyUnicode_Compare( + name, _PyUnicode_FromId(&PyId___newobj_ex__)) == 0; + } + if (!use_newobj_ex) { + _Py_IDENTIFIER(__newobj__); + use_newobj = PyUnicode_Compare( + name, _PyUnicode_FromId(&PyId___newobj__)) == 0; + } } + Py_XDECREF(name); } if (use_newobj_ex) { diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 941dedb0fac..dd8a940b1c9 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3886,48 +3886,87 @@ _PyObject_GetItemsIter(PyObject *obj, PyObject **listitems, } static PyObject * -reduce_4(PyObject *obj) +reduce_newobj(PyObject *obj, int proto) { PyObject *args = NULL, *kwargs = NULL; PyObject *copyreg; PyObject *newobj, *newargs, *state, *listitems, *dictitems; PyObject *result; - _Py_IDENTIFIER(__newobj_ex__); - if (_PyObject_GetNewArguments(obj, &args, &kwargs) < 0) { + if (_PyObject_GetNewArguments(obj, &args, &kwargs) < 0) return NULL; - } + if (args == NULL) { args = PyTuple_New(0); - if (args == NULL) + if (args == NULL) { + Py_XDECREF(kwargs); return NULL; + } } - if (kwargs == NULL) { - kwargs = PyDict_New(); - if (kwargs == NULL) - return NULL; - } - copyreg = import_copyreg(); if (copyreg == NULL) { Py_DECREF(args); - Py_DECREF(kwargs); + Py_XDECREF(kwargs); return NULL; } - newobj = _PyObject_GetAttrId(copyreg, &PyId___newobj_ex__); - Py_DECREF(copyreg); - if (newobj == NULL) { + if (kwargs == NULL || PyDict_Size(kwargs) == 0) { + _Py_IDENTIFIER(__newobj__); + PyObject *cls; + Py_ssize_t i, n; + + Py_XDECREF(kwargs); + newobj = _PyObject_GetAttrId(copyreg, &PyId___newobj__); + Py_DECREF(copyreg); + if (newobj == NULL) { + Py_DECREF(args); + return NULL; + } + n = PyTuple_GET_SIZE(args); + newargs = PyTuple_New(n+1); + if (newargs == NULL) { + Py_DECREF(args); + Py_DECREF(newobj); + return NULL; + } + cls = (PyObject *) Py_TYPE(obj); + Py_INCREF(cls); + PyTuple_SET_ITEM(newargs, 0, cls); + for (i = 0; i < n; i++) { + PyObject *v = PyTuple_GET_ITEM(args, i); + Py_INCREF(v); + PyTuple_SET_ITEM(newargs, i+1, v); + } + Py_DECREF(args); + } + else if (proto >= 4) { + _Py_IDENTIFIER(__newobj_ex__); + + newobj = _PyObject_GetAttrId(copyreg, &PyId___newobj_ex__); + Py_DECREF(copyreg); + if (newobj == NULL) { + Py_DECREF(args); + Py_DECREF(kwargs); + return NULL; + } + newargs = PyTuple_Pack(3, Py_TYPE(obj), args, kwargs); Py_DECREF(args); Py_DECREF(kwargs); + if (newargs == NULL) { + Py_DECREF(newobj); + return NULL; + } + } + else { + PyErr_SetString(PyExc_ValueError, + "must use protocol 4 or greater to copy this " + "object; since __getnewargs_ex__ returned " + "keyword arguments."); + Py_DECREF(args); + Py_DECREF(kwargs); + Py_DECREF(copyreg); return NULL; } - newargs = PyTuple_Pack(3, Py_TYPE(obj), args, kwargs); - Py_DECREF(args); - Py_DECREF(kwargs); - if (newargs == NULL) { - Py_DECREF(newobj); - return NULL; - } + state = _PyObject_GetState(obj); if (state == NULL) { Py_DECREF(newobj); @@ -3950,79 +3989,6 @@ reduce_4(PyObject *obj) return result; } -static PyObject * -reduce_2(PyObject *obj) -{ - PyObject *cls; - PyObject *args = NULL, *args2 = NULL, *kwargs = NULL; - PyObject *state = NULL, *listitems = NULL, *dictitems = NULL; - PyObject *copyreg = NULL, *newobj = NULL, *res = NULL; - Py_ssize_t i, n; - _Py_IDENTIFIER(__newobj__); - - if (_PyObject_GetNewArguments(obj, &args, &kwargs) < 0) { - return NULL; - } - if (args == NULL) { - assert(kwargs == NULL); - args = PyTuple_New(0); - if (args == NULL) { - return NULL; - } - } - else if (kwargs != NULL) { - if (PyDict_Size(kwargs) > 0) { - PyErr_SetString(PyExc_ValueError, - "must use protocol 4 or greater to copy this " - "object; since __getnewargs_ex__ returned " - "keyword arguments."); - Py_DECREF(args); - Py_DECREF(kwargs); - return NULL; - } - Py_CLEAR(kwargs); - } - - state = _PyObject_GetState(obj); - if (state == NULL) - goto end; - - if (_PyObject_GetItemsIter(obj, &listitems, &dictitems) < 0) - goto end; - - copyreg = import_copyreg(); - if (copyreg == NULL) - goto end; - newobj = _PyObject_GetAttrId(copyreg, &PyId___newobj__); - if (newobj == NULL) - goto end; - - n = PyTuple_GET_SIZE(args); - args2 = PyTuple_New(n+1); - if (args2 == NULL) - goto end; - cls = (PyObject *) Py_TYPE(obj); - Py_INCREF(cls); - PyTuple_SET_ITEM(args2, 0, cls); - for (i = 0; i < n; i++) { - PyObject *v = PyTuple_GET_ITEM(args, i); - Py_INCREF(v); - PyTuple_SET_ITEM(args2, i+1, v); - } - - res = PyTuple_Pack(5, newobj, args2, state, listitems, dictitems); - - end: - Py_XDECREF(args); - Py_XDECREF(args2); - Py_XDECREF(state); - Py_XDECREF(listitems); - Py_XDECREF(dictitems); - Py_XDECREF(copyreg); - Py_XDECREF(newobj); - return res; -} - /* * There were two problems when object.__reduce__ and object.__reduce_ex__ * were implemented in the same function: @@ -4043,10 +4009,8 @@ _common_reduce(PyObject *self, int proto) { PyObject *copyreg, *res; - if (proto >= 4) - return reduce_4(self); - else if (proto >= 2) - return reduce_2(self); + if (proto >= 2) + return reduce_newobj(self, proto); copyreg = import_copyreg(); if (!copyreg)