From 9397506be6f17e729c8e92900e75e25cd0a2f991 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 27 Feb 2019 18:58:26 -0800 Subject: [PATCH 01/36] Create dict_add and dict__iadd dict methods. `d1 + d2` is semantically identical to `d3 = d1.copy(); d3.update(d2); d3`, and `d1 += d2` is semantically identical to `d1.update(d2)`. Isn't that sweet? --- Objects/dictobject.c | 63 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 83cadda84c6dc4..55d48e071e6841 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -3083,6 +3083,44 @@ dict_sizeof(PyDictObject *mp, PyObject *Py_UNUSED(ignored)) return PyLong_FromSsize_t(_PyDict_SizeOf(mp)); } +static PyObject * +dict_add(PyDictObject *self, PyObject *other) +{ + PyObject *new; + + if (!PyDict_Check(self) || !PyDict_Check(other)) { + Py_RETURN_NOTIMPLEMENTED; + } + + new = PyDict_Copy((PyObject *)self); + + if (new == NULL) { + return NULL; + } + + if (PyDict_Update(new, other)) { + Py_DECREF(new); + Py_RETURN_NOTIMPLEMENTED; + } + + return new; +} + +static PyObject * +dict_iadd(PyDictObject *self, PyObject *other) +{ + if (!PyDict_Check(self) || !PyDict_Check(other)) { + Py_RETURN_NOTIMPLEMENTED; + } + + if (PyDict_Update((PyObject *)self, other)) { + Py_RETURN_NOTIMPLEMENTED; + } + + Py_INCREF((PyObject *)self); + return (PyObject *)self; +} + PyDoc_STRVAR(getitem__doc__, "x.__getitem__(y) <==> x[y]"); PyDoc_STRVAR(sizeof__doc__, @@ -3237,6 +3275,29 @@ dict_iter(PyDictObject *dict) return dictiter_new(dict, &PyDictIterKey_Type); } +static PyNumberMethods dict_as_number = { + (binaryfunc)dict_add, /*nb_add*/ + 0, /*nb_subtract*/ + 0, /*nb_multiply*/ + 0, /*nb_remainder*/ + 0, /*nb_divmod*/ + 0, /*nb_power*/ + 0, /*nb_negative*/ + 0, /*nb_positive*/ + 0, /*nb_absolute*/ + 0, /*nb_bool*/ + 0, /*nb_invert*/ + 0, /*nb_lshift*/ + 0, /*nb_rshift*/ + 0, /*nb_and*/ + 0, /*nb_xor*/ + 0, /*nb_or*/ + 0, /*nb_int*/ + 0, /*nb_reserved*/ + 0, /*nb_float*/ + (binaryfunc)dict_iadd, /*nb_inplace_add*/ +}; + PyDoc_STRVAR(dictionary_doc, "dict() -> new empty dictionary\n" "dict(mapping) -> new dictionary initialized from a mapping object's\n" @@ -3259,7 +3320,7 @@ PyTypeObject PyDict_Type = { 0, /* tp_setattr */ 0, /* tp_reserved */ (reprfunc)dict_repr, /* tp_repr */ - 0, /* tp_as_number */ + &dict_as_number, /* tp_as_number */ &dict_as_sequence, /* tp_as_sequence */ &dict_as_mapping, /* tp_as_mapping */ PyObject_HashNotImplemented, /* tp_hash */ From c69108ac77aba3d0757c4696cac984ad6f4039b2 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 27 Feb 2019 19:23:35 -0800 Subject: [PATCH 02/36] Fix failing "sum" test. It feels so weird to sum dicts like this... cool. --- Lib/test/test_builtin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index 5674ea89b1c408..2128643568d52b 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -1296,6 +1296,7 @@ def test_sum(self): self.assertEqual(sum(Squares(10)), 285) self.assertEqual(sum(iter(Squares(10))), 285) self.assertEqual(sum([[1], [2], [3]], []), [1, 2, 3]) + self.assertEqual(sum([{3:4}, {5:6}], {1:2}), {1:2, 3:4, 5:6}) self.assertEqual(sum(range(10), 1000), 1045) self.assertEqual(sum(range(10), start=1000), 1045) @@ -1309,7 +1310,6 @@ def test_sum(self): self.assertRaises(TypeError, sum, values, bytearray(b'')) self.assertRaises(TypeError, sum, [[1], [2], [3]]) self.assertRaises(TypeError, sum, [{2:3}]) - self.assertRaises(TypeError, sum, [{2:3}]*2, {2:3}) class BadSeq: def __getitem__(self, index): From dd42f4f7070089b7eba04702725a6c8bbe0e1f0b Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 27 Feb 2019 19:25:08 -0800 Subject: [PATCH 03/36] Update collections.UserDict. This gets the collections tests passing again. --- Lib/collections/__init__.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index 632a509d316cb3..c99e21494c12bc 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -1034,6 +1034,17 @@ def __contains__(self, key): # Now, add the methods in dicts but not in MutableMapping def __repr__(self): return repr(self.data) + + def __add__(self, other): + return UserDict(self.data + other) + + def __radd__(self, other): + return UserDict(other + self.data) + + def __iadd__(self, other): + self.data += other + return self + def copy(self): if self.__class__ is UserDict: return UserDict(self.data.copy()) From f59cdab22c7e331dd1c14165c54dea0ac0f1fab7 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 27 Feb 2019 19:48:52 -0800 Subject: [PATCH 04/36] Update test_dict.py. This adds one test with 5 new assertions. --- Lib/test/test_dict.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 03afd5b2a6c75e..da5ec7874c113a 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -37,6 +37,27 @@ def test_literal_constructor(self): dictliteral = '{' + ', '.join(formatted_items) + '}' self.assertEqual(eval(dictliteral), dict(items)) + def test_addition(self): + + a = {0: 0, 1: 1, 2: 1} + b = {1: 1, 2: 2, 3: 3} + + c = a.copy() + c += b + + self.assertEqual(a + b, {0: 0, 1: 1, 2: 2, 3: 3}) + self.assertEqual(c, {0: 0, 1: 1, 2: 2, 3: 3}) + + c = b.copy() + c += a + + self.assertEqual(b + a, {1: 1, 2: 1, 3: 3, 0: 0}) + self.assertEqual(c, {1: 1, 2: 1, 3: 3, 0: 0}) + + self.assertIs(a.__add__(None), NotImplemented) + self.assertIs(a.__radd__("BAD"), NotImplemented) + self.assertIs(a.__iadd__([]), NotImplemented) + def test_bool(self): self.assertIs(not {}, True) self.assertTrue({1: 2}) From d895b9bc6d4915cd7f4eefc9775e59944742ac49 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 27 Feb 2019 19:49:18 -0800 Subject: [PATCH 05/36] Update ACKS. In case this gets pulled. Very cool! --- Misc/ACKS | 1 + 1 file changed, 1 insertion(+) diff --git a/Misc/ACKS b/Misc/ACKS index c9fa08bd614138..519397e26f6a21 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -222,6 +222,7 @@ Ian Bruntlett Floris Bruynooghe Matt Bryant Stan Bubrouski +Brandt Bucher Colm Buckley Erik de Bueger Jan-Hein Bührman From da8e2e373e72caa075a31b2268d29267b3fcad81 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 27 Feb 2019 20:17:21 -0800 Subject: [PATCH 06/36] Fixed whitespace. Whoops. --- Lib/collections/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index c99e21494c12bc..105b4d01fbf203 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -1034,7 +1034,7 @@ def __contains__(self, key): # Now, add the methods in dicts but not in MutableMapping def __repr__(self): return repr(self.data) - + def __add__(self, other): return UserDict(self.data + other) From 91d64661ec101fc1c132425a0ff84d2c328fbe13 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 27 Feb 2019 20:51:22 -0800 Subject: [PATCH 07/36] Update doctest count for builtins. This whole patch adds 3 new objects with docstrings: dict.__add__, dict.__radd__, dict.__iadd__. On some builds, this puts us over the threshold, so bump the count! --- Lib/test/test_doctest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_doctest.py b/Lib/test/test_doctest.py index f1013f25725964..6d4655c693e989 100644 --- a/Lib/test/test_doctest.py +++ b/Lib/test/test_doctest.py @@ -661,7 +661,7 @@ def non_Python_modules(): r""" >>> import builtins >>> tests = doctest.DocTestFinder().find(builtins) - >>> 800 < len(tests) < 820 # approximate number of objects with docstrings + >>> 800 < len(tests) < 825 # approximate number of objects with docstrings True >>> real_tests = [t for t in tests if len(t.examples) > 0] >>> len(real_tests) # objects that actually have doctests From b2eced1be4fabebce09425fb27cb37aed5c2eb2b Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" Date: Thu, 28 Feb 2019 05:16:14 +0000 Subject: [PATCH 08/36] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Core and Builtins/2019-02-28-05-16-14.bpo-36144.wLV8fF.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-02-28-05-16-14.bpo-36144.wLV8fF.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-02-28-05-16-14.bpo-36144.wLV8fF.rst b/Misc/NEWS.d/next/Core and Builtins/2019-02-28-05-16-14.bpo-36144.wLV8fF.rst new file mode 100644 index 00000000000000..2da6c72df9ef62 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-02-28-05-16-14.bpo-36144.wLV8fF.rst @@ -0,0 +1,2 @@ +:class:`dict` objects now support addition operations, with semantics similar to :meth:`dict.update`. +Patch by Brandt Bucher. \ No newline at end of file From bf0b3839a14c90f360950e086b99e304936fb78a Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 28 Feb 2019 09:13:30 -0800 Subject: [PATCH 09/36] Fix error handling. Replaces two places where a failed PyDict_Update call returns NotImplemented instead of raising the error. --- Objects/dictobject.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 55d48e071e6841..6133dceff22e1f 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -3100,7 +3100,7 @@ dict_add(PyDictObject *self, PyObject *other) if (PyDict_Update(new, other)) { Py_DECREF(new); - Py_RETURN_NOTIMPLEMENTED; + return NULL; } return new; @@ -3114,7 +3114,7 @@ dict_iadd(PyDictObject *self, PyObject *other) } if (PyDict_Update((PyObject *)self, other)) { - Py_RETURN_NOTIMPLEMENTED; + return NULL; } Py_INCREF((PyObject *)self); From e6600462cf34d78364a779c1e20014d734a5e197 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sat, 2 Mar 2019 09:53:02 -0800 Subject: [PATCH 10/36] Be "nicer" to subclasses than other built-in types. This brings this reference implementation in line with PEP 584's spec. Separately, allows RHS of += to be any valid arg to dict.update. --- Lib/test/test_dict.py | 15 +++++++++++- Objects/dictobject.c | 55 ++++++++++++++++++++++++++++--------------- 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index da5ec7874c113a..e32570c767d31f 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -54,9 +54,22 @@ def test_addition(self): self.assertEqual(b + a, {1: 1, 2: 1, 3: 3, 0: 0}) self.assertEqual(c, {1: 1, 2: 1, 3: 3, 0: 0}) + c = a.copy() + c += [(1, 1), (2, 2), (3, 3)] + + self.assertEqual(c, {0: 0, 1: 1, 2: 2, 3: 3}) + self.assertIs(a.__add__(None), NotImplemented) + self.assertIs(a.__add__(()), NotImplemented) + self.assertIs(a.__add__("BAD"), NotImplemented) + + self.assertIs(a.__radd__(None), NotImplemented) + self.assertIs(a.__radd__(()), NotImplemented) self.assertIs(a.__radd__("BAD"), NotImplemented) - self.assertIs(a.__iadd__([]), NotImplemented) + + self.assertRaises(TypeError, a.__iadd__, None) + self.assertEqual(a.__iadd__(()), {0: 0, 1: 1, 2: 1}) + self.assertRaises(ValueError, a.__iadd__, "BAD") def test_bool(self): self.assertIs(not {}, True) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 6133dceff22e1f..1e71fbf7bfb24b 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2264,6 +2264,25 @@ dict_fromkeys_impl(PyTypeObject *type, PyObject *iterable, PyObject *value) return _PyDict_FromKeys((PyObject *)type, iterable, value); } +/* Single-arg dict update; used by dict_update_common and addition ops. */ +static int +dict_update_arg(PyObject *self, PyObject *arg) { + + PyObject *func; + _Py_IDENTIFIER(keys); + + if (_PyObject_LookupAttrId(arg, &PyId_keys, &func) < 0) { + return -1; + } + + if (func != NULL) { + Py_DECREF(func); + return PyDict_Merge(self, arg, 1); + } + + return PyDict_MergeFromSeq2(self, arg, 1); +} + static int dict_update_common(PyObject *self, PyObject *args, PyObject *kwds, const char *methname) @@ -2275,18 +2294,7 @@ dict_update_common(PyObject *self, PyObject *args, PyObject *kwds, result = -1; } else if (arg != NULL) { - _Py_IDENTIFIER(keys); - PyObject *func; - if (_PyObject_LookupAttrId(arg, &PyId_keys, &func) < 0) { - result = -1; - } - else if (func != NULL) { - Py_DECREF(func); - result = PyDict_Merge(self, arg, 1); - } - else { - result = PyDict_MergeFromSeq2(self, arg, 1); - } + result = dict_update_arg(self, arg); } if (result == 0 && kwds != NULL) { @@ -3084,7 +3092,7 @@ dict_sizeof(PyDictObject *mp, PyObject *Py_UNUSED(ignored)) } static PyObject * -dict_add(PyDictObject *self, PyObject *other) +dict_add(PyObject *self, PyObject *other) { PyObject *new; @@ -3092,13 +3100,18 @@ dict_add(PyDictObject *self, PyObject *other) Py_RETURN_NOTIMPLEMENTED; } - new = PyDict_Copy((PyObject *)self); + /* If subclass constructors/initializers have been overridden to require + * at least one arg, this next bit could fail with a confusing TypeError... + * dict.fromkeys currently has this issue, though, so nothing new. + */ + + new = _PyObject_CallNoArg((PyObject *)Py_TYPE(self)); if (new == NULL) { return NULL; } - if (PyDict_Update(new, other)) { + if (dict_update_arg(new, self) || dict_update_arg(new, other)) { Py_DECREF(new); return NULL; } @@ -3109,11 +3122,15 @@ dict_add(PyDictObject *self, PyObject *other) static PyObject * dict_iadd(PyDictObject *self, PyObject *other) { - if (!PyDict_Check(self) || !PyDict_Check(other)) { - Py_RETURN_NOTIMPLEMENTED; - } + /* Don't delegate to __add__ here? Could be confusing for subclasses... + * https://mail.python.org/pipermail/python-ideas/2019-March/055581.html + * + * if (!PyMapping_Check(other)) { + * Py_RETURN_NOTIMPLEMENTED; + * } + */ - if (PyDict_Update((PyObject *)self, other)) { + if (dict_update_arg((PyObject *)self, other)) { return NULL; } From 4c0223b65c44f216d932a6c02896bd6e3e0910bd Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sat, 2 Mar 2019 14:57:22 -0800 Subject: [PATCH 11/36] Add dict subtraction operations. These are implemented as they are currently laid out in PEP 584. --- .../2019-02-28-05-16-14.bpo-36144.wLV8fF.rst | 2 - Objects/dictobject.c | 99 ++++++++++++++++++- 2 files changed, 97 insertions(+), 4 deletions(-) delete mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-02-28-05-16-14.bpo-36144.wLV8fF.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-02-28-05-16-14.bpo-36144.wLV8fF.rst b/Misc/NEWS.d/next/Core and Builtins/2019-02-28-05-16-14.bpo-36144.wLV8fF.rst deleted file mode 100644 index 2da6c72df9ef62..00000000000000 --- a/Misc/NEWS.d/next/Core and Builtins/2019-02-28-05-16-14.bpo-36144.wLV8fF.rst +++ /dev/null @@ -1,2 +0,0 @@ -:class:`dict` objects now support addition operations, with semantics similar to :meth:`dict.update`. -Patch by Brandt Bucher. \ No newline at end of file diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 1e71fbf7bfb24b..49d57af2def775 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2264,9 +2264,60 @@ dict_fromkeys_impl(PyTypeObject *type, PyObject *iterable, PyObject *value) return _PyDict_FromKeys((PyObject *)type, iterable, value); } +static int +dict_diff_arg(PyObject *self, PyObject *arg) +{ + PyObject *iter_arg; + PyObject *item; + PyObject *keys; + int in; + + /* This first block might need to be removed. + * See "Bug or feature?" in PEP 584 draft. + */ + + keys = PyObject_GetAttrString(arg, "keys"); + + if (keys != NULL) { + arg = _PyObject_CallNoArg(keys); + Py_DECREF(keys); + + if (arg == NULL) { + return -1; + } + } + else { + PyErr_Clear(); + } + + iter_arg = PyObject_GetIter(arg); + + if (iter_arg == NULL) { + return -1; + } + + while ((item = PyIter_Next(iter_arg)) != NULL) { + + in = PyDict_Contains(self, item); + + if (in < 0 || (in && PyDict_DelItem(self, item))) { + Py_DECREF(iter_arg); + Py_DECREF(item); + return -1; + } + + Py_DECREF(item); + } + + Py_DECREF(iter_arg); + return 0; +} + + /* Single-arg dict update; used by dict_update_common and addition ops. */ static int -dict_update_arg(PyObject *self, PyObject *arg) { +dict_update_arg(PyObject *self, PyObject *arg) +{ PyObject *func; _Py_IDENTIFIER(keys); @@ -3138,6 +3189,49 @@ dict_iadd(PyDictObject *self, PyObject *other) return (PyObject *)self; } +static PyObject * +dict_sub(PyObject *self, PyObject *other) +{ + PyObject *new; + + if (!PyDict_Check(self) || !PyDict_Check(other)) { + Py_RETURN_NOTIMPLEMENTED; + } + + /* See dict_add for limitations of this construction: */ + + new = _PyObject_CallNoArg((PyObject *)Py_TYPE(self)); + + if (new == NULL) { + return NULL; + } + + if (dict_update_arg(new, self) || dict_diff_arg(new, other)) { + Py_DECREF(new); + return NULL; + } + + return new; +} + +static PyObject * +dict_isub(PyDictObject *self, PyObject *other) +{ + /* See dict_iadd for this removal: + * + * if (!PyDict_Check(other)) { + * Py_RETURN_NOTIMPLEMENTED; + * } + */ + + if (dict_diff_arg((PyObject *)self, other)) { + return NULL; + } + + Py_INCREF((PyObject *)self); + return (PyObject *)self; +} + PyDoc_STRVAR(getitem__doc__, "x.__getitem__(y) <==> x[y]"); PyDoc_STRVAR(sizeof__doc__, @@ -3294,7 +3388,7 @@ dict_iter(PyDictObject *dict) static PyNumberMethods dict_as_number = { (binaryfunc)dict_add, /*nb_add*/ - 0, /*nb_subtract*/ + (binaryfunc)dict_sub, /*nb_subtract*/ 0, /*nb_multiply*/ 0, /*nb_remainder*/ 0, /*nb_divmod*/ @@ -3313,6 +3407,7 @@ static PyNumberMethods dict_as_number = { 0, /*nb_reserved*/ 0, /*nb_float*/ (binaryfunc)dict_iadd, /*nb_inplace_add*/ + (binaryfunc)dict_isub, /*nb_inplace_subtract*/ }; PyDoc_STRVAR(dictionary_doc, From 2cb1629bab9baa77a2af151869beac9c42b72d01 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sat, 2 Mar 2019 14:58:53 -0800 Subject: [PATCH 12/36] Add new subtraction methods to collections.UserDict. This should get tests passing again. --- Lib/collections/__init__.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index 105b4d01fbf203..7bed28d1495784 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -1039,12 +1039,22 @@ def __add__(self, other): return UserDict(self.data + other) def __radd__(self, other): - return UserDict(other + self.data) + return other + self.data def __iadd__(self, other): self.data += other return self + def __sub__(self, other): + return UserDict(self.data - other) + + def __rsub__(self, other): + return other - self.data + + def __isub__(self, other): + self.data -= other + return self + def copy(self): if self.__class__ is UserDict: return UserDict(self.data.copy()) From 05ed4cf9269c3779b2f30cecfb59f724cccb7e5a Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sat, 2 Mar 2019 14:59:46 -0800 Subject: [PATCH 13/36] Add new dict subtraction tests. And... we're officially implemented! --- Lib/test/test_dict.py | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index e32570c767d31f..df051d384fc0fd 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -56,7 +56,7 @@ def test_addition(self): c = a.copy() c += [(1, 1), (2, 2), (3, 3)] - + self.assertEqual(c, {0: 0, 1: 1, 2: 2, 3: 3}) self.assertIs(a.__add__(None), NotImplemented) @@ -71,6 +71,40 @@ def test_addition(self): self.assertEqual(a.__iadd__(()), {0: 0, 1: 1, 2: 1}) self.assertRaises(ValueError, a.__iadd__, "BAD") + def test_subtraction(self): + + a = {0: 0, 1: 1, 2: 1} + b = {1: 1, 2: 2, 3: 3} + + c = a.copy() + c -= b + + self.assertEqual(a - b, {0: 0}) + self.assertEqual(c, {0: 0}) + + c = b.copy() + c -= a + + self.assertEqual(b - a, {3: 3}) + self.assertEqual(c, {3: 3}) + + c = a.copy() + c -= (1, 2, 3) + + self.assertEqual(c, {0: 0}) + + self.assertIs(a.__sub__(None), NotImplemented) + self.assertIs(a.__sub__(()), NotImplemented) + self.assertIs(a.__sub__("BAD"), NotImplemented) + + self.assertIs(a.__rsub__(None), NotImplemented) + self.assertIs(a.__rsub__(()), NotImplemented) + self.assertIs(a.__rsub__("BAD"), NotImplemented) + + self.assertRaises(TypeError, a.__isub__, None) + self.assertEqual(a.__isub__(()), {0: 0, 1: 1, 2: 1}) + self.assertEqual(a.__isub__("BAD"), {0: 0, 1: 1, 2: 1}) + def test_bool(self): self.assertIs(not {}, True) self.assertTrue({1: 2}) From 594ae2797789e5abd48c123dfe26efe05c983bb2 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" Date: Sat, 2 Mar 2019 23:03:35 +0000 Subject: [PATCH 14/36] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Core and Builtins/2019-03-02-23-03-34.bpo-36144.LRl4LS.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-03-02-23-03-34.bpo-36144.LRl4LS.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-03-02-23-03-34.bpo-36144.LRl4LS.rst b/Misc/NEWS.d/next/Core and Builtins/2019-03-02-23-03-34.bpo-36144.LRl4LS.rst new file mode 100644 index 00000000000000..4469bbbadb9c5f --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-03-02-23-03-34.bpo-36144.LRl4LS.rst @@ -0,0 +1,2 @@ +:class:`dict` objects now support PEP 584 addition and subtraction operations, with semantics similar to :meth:`dict.update`. +Patch by Brandt Bucher. \ No newline at end of file From f7ed0a25a1e6c08d77985984d888c06fb0438eea Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sun, 3 Mar 2019 14:59:05 -0800 Subject: [PATCH 15/36] Better error handling and "keys" method lookup. This fixes issues where iteration errors could be swallowed, and also stops a possible reference leak. --- Objects/dictobject.c | 63 +++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 18 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 49d57af2def775..68d065a8c21acf 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2269,30 +2269,35 @@ dict_diff_arg(PyObject *self, PyObject *arg) { PyObject *iter_arg; PyObject *item; - PyObject *keys; + PyObject *func; int in; - /* This first block might need to be removed. + _Py_IDENTIFIER(keys); + + /* These first two blocks might need to be removed. * See "Bug or feature?" in PEP 584 draft. */ + + if (_PyObject_LookupAttrId(arg, &PyId_keys, &func) < 0) { + return -1; + } - keys = PyObject_GetAttrString(arg, "keys"); + if (func != NULL) { - if (keys != NULL) { - arg = _PyObject_CallNoArg(keys); - Py_DECREF(keys); + arg = _PyObject_CallNoArg(func); + Py_DECREF(func); if (arg == NULL) { return -1; } } - else { - PyErr_Clear(); - } iter_arg = PyObject_GetIter(arg); if (iter_arg == NULL) { + if (func != NULL) { + Py_DECREF(arg); + } return -1; } @@ -2301,8 +2306,11 @@ dict_diff_arg(PyObject *self, PyObject *arg) in = PyDict_Contains(self, item); if (in < 0 || (in && PyDict_DelItem(self, item))) { - Py_DECREF(iter_arg); Py_DECREF(item); + Py_DECREF(iter_arg); + if (func != NULL) { + Py_DECREF(arg); + } return -1; } @@ -2310,6 +2318,14 @@ dict_diff_arg(PyObject *self, PyObject *arg) } Py_DECREF(iter_arg); + if (func != NULL) { + Py_DECREF(arg); + } + + if (PyErr_Occurred()) { + return -1; + } + return 0; } @@ -2318,7 +2334,6 @@ dict_diff_arg(PyObject *self, PyObject *arg) static int dict_update_arg(PyObject *self, PyObject *arg) { - PyObject *func; _Py_IDENTIFIER(keys); @@ -3151,12 +3166,18 @@ dict_add(PyObject *self, PyObject *other) Py_RETURN_NOTIMPLEMENTED; } - /* If subclass constructors/initializers have been overridden to require - * at least one arg, this next bit could fail with a confusing TypeError... - * dict.fromkeys currently has this issue, though, so nothing new. - */ + if (PyDict_CheckExact(self)) { + new = PyDict_New(); + } + else { + + /* If subclass constructors/initializers have been overridden to require + * at least one arg, this next bit could fail with a confusing TypeError... + * dict.fromkeys currently has this issue, though, so nothing new. + */ - new = _PyObject_CallNoArg((PyObject *)Py_TYPE(self)); + new = _PyObject_CallNoArg((PyObject *)Py_TYPE(self)); + } if (new == NULL) { return NULL; @@ -3198,9 +3219,15 @@ dict_sub(PyObject *self, PyObject *other) Py_RETURN_NOTIMPLEMENTED; } - /* See dict_add for limitations of this construction: */ + if (PyDict_CheckExact(self)) { + new = PyDict_New(); + } + else { - new = _PyObject_CallNoArg((PyObject *)Py_TYPE(self)); + /* See dict_add for limitations of this construction: */ + + new = _PyObject_CallNoArg((PyObject *)Py_TYPE(self)); + } if (new == NULL) { return NULL; From 8be454a8d12a7585d3e6ce2603aceb935d1f1c85 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 6 Mar 2019 07:54:17 -0800 Subject: [PATCH 16/36] Clean up comments, names, and logic. This also implements the complex keys/__iter__ dance that update uses, as an example of compatibility with dict.update. --- Objects/dictobject.c | 111 ++++++++++++++++++++++--------------------- 1 file changed, 58 insertions(+), 53 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 68d065a8c21acf..f274981aa2146f 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2264,63 +2264,61 @@ dict_fromkeys_impl(PyTypeObject *type, PyObject *iterable, PyObject *value) return _PyDict_FromKeys((PyObject *)type, iterable, value); } +/* Used for dict subtraction. */ static int dict_diff_arg(PyObject *self, PyObject *arg) { - PyObject *iter_arg; - PyObject *item; - PyObject *func; + PyObject *keys, *key; + PyObject *keys_method; int in; _Py_IDENTIFIER(keys); - /* These first two blocks might need to be removed. - * See "Bug or feature?" in PEP 584 draft. - */ - - if (_PyObject_LookupAttrId(arg, &PyId_keys, &func) < 0) { + // XXX: PEP 584 + + // This keys() lookup stuff is a bit complex... it mirrors dict.update: + // https://mail.python.org/pipermail/python-ideas/2019-March/055687.html + + if (_PyObject_LookupAttrId(arg, &PyId_keys, &keys_method) < 0) { return -1; } - if (func != NULL) { + if (keys_method && (!PyDict_Check(arg) || + Py_TYPE(arg)->tp_iter != (getiterfunc)dict_iter)) + { - arg = _PyObject_CallNoArg(func); - Py_DECREF(func); + arg = _PyObject_CallNoArg(keys_method); + Py_DECREF(keys_method); if (arg == NULL) { return -1; } - } - iter_arg = PyObject_GetIter(arg); + keys = PyObject_GetIter(arg); + Py_DECREF(arg); + } + else { + keys = PyObject_GetIter(arg); + } - if (iter_arg == NULL) { - if (func != NULL) { - Py_DECREF(arg); - } + if (keys == NULL) { return -1; } - while ((item = PyIter_Next(iter_arg)) != NULL) { + while ((key = PyIter_Next(keys))) { - in = PyDict_Contains(self, item); + in = PyDict_Contains(self, key); - if (in < 0 || (in && PyDict_DelItem(self, item))) { - Py_DECREF(item); - Py_DECREF(iter_arg); - if (func != NULL) { - Py_DECREF(arg); - } + if (in < 0 || (in && PyDict_DelItem(self, key))) { + Py_DECREF(keys); + Py_DECREF(key); return -1; } - Py_DECREF(item); + Py_DECREF(key); } - Py_DECREF(iter_arg); - if (func != NULL) { - Py_DECREF(arg); - } + Py_DECREF(keys); if (PyErr_Occurred()) { return -1; @@ -2334,15 +2332,15 @@ dict_diff_arg(PyObject *self, PyObject *arg) static int dict_update_arg(PyObject *self, PyObject *arg) { - PyObject *func; + PyObject *keys_method; _Py_IDENTIFIER(keys); - if (_PyObject_LookupAttrId(arg, &PyId_keys, &func) < 0) { + if (_PyObject_LookupAttrId(arg, &PyId_keys, &keys_method) < 0) { return -1; } - if (func != NULL) { - Py_DECREF(func); + if (keys_method) { + Py_DECREF(keys_method); return PyDict_Merge(self, arg, 1); } @@ -3171,11 +3169,12 @@ dict_add(PyObject *self, PyObject *other) } else { - /* If subclass constructors/initializers have been overridden to require - * at least one arg, this next bit could fail with a confusing TypeError... - * dict.fromkeys currently has this issue, though, so nothing new. - */ + // XXX: PEP 584 + // If subclass constructors/initializers have been overridden to require + // at least one arg, this next bit could fail with a confusing TypeError... + // dict.fromkeys currently has this issue, though, so nothing new. + new = _PyObject_CallNoArg((PyObject *)Py_TYPE(self)); } @@ -3194,19 +3193,21 @@ dict_add(PyObject *self, PyObject *other) static PyObject * dict_iadd(PyDictObject *self, PyObject *other) { - /* Don't delegate to __add__ here? Could be confusing for subclasses... - * https://mail.python.org/pipermail/python-ideas/2019-March/055581.html - * - * if (!PyMapping_Check(other)) { - * Py_RETURN_NOTIMPLEMENTED; - * } - */ + // XXX: PEP 584 + + // Don't fall back to __add__ here? Could be confusing for subclasses... + // https://mail.python.org/pipermail/python-ideas/2019-March/055581.html + + // if (!PyMapping_Check(other)) { + // Py_RETURN_NOTIMPLEMENTED; + // } if (dict_update_arg((PyObject *)self, other)) { return NULL; } Py_INCREF((PyObject *)self); + return (PyObject *)self; } @@ -3224,8 +3225,10 @@ dict_sub(PyObject *self, PyObject *other) } else { - /* See dict_add for limitations of this construction: */ - + // XXX: PEP 584 + + // See dict_add for limitations of this construction: + new = _PyObject_CallNoArg((PyObject *)Py_TYPE(self)); } @@ -3244,18 +3247,20 @@ dict_sub(PyObject *self, PyObject *other) static PyObject * dict_isub(PyDictObject *self, PyObject *other) { - /* See dict_iadd for this removal: - * - * if (!PyDict_Check(other)) { - * Py_RETURN_NOTIMPLEMENTED; - * } - */ + // XXX: PEP 584 + + // See dict_iadd for this removal: + + // if (!PyDict_Check(other)) { + // Py_RETURN_NOTIMPLEMENTED; + // } if (dict_diff_arg((PyObject *)self, other)) { return NULL; } Py_INCREF((PyObject *)self); + return (PyObject *)self; } From 63c36dd707f85eae933ad0695dd40a0137e842f7 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 6 Mar 2019 18:59:52 -0800 Subject: [PATCH 17/36] Update commented code. dict.__iadd__ can take any mapping (i.e., a list). --- Objects/dictobject.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index f274981aa2146f..f7d3e11bf6d82a 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2286,7 +2286,6 @@ dict_diff_arg(PyObject *self, PyObject *arg) if (keys_method && (!PyDict_Check(arg) || Py_TYPE(arg)->tp_iter != (getiterfunc)dict_iter)) { - arg = _PyObject_CallNoArg(keys_method); Py_DECREF(keys_method); @@ -3251,7 +3250,7 @@ dict_isub(PyDictObject *self, PyObject *other) // See dict_iadd for this removal: - // if (!PyDict_Check(other)) { + // if (!PyMapping_Check(other)) { // Py_RETURN_NOTIMPLEMENTED; // } From 2ba05805e74c63d8eac6f983fbd7dee4fbd9dff2 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 22 Mar 2019 09:43:11 -0700 Subject: [PATCH 18/36] Update new collections.UserDict operators. This is pretty much copied-and-pasted from UserDict, but it's better than what we had before. They may need some tweaking. --- Lib/collections/__init__.py | 54 ++++++++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 6 deletions(-) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index 7bed28d1495784..07125369e1ee02 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -1036,23 +1036,65 @@ def __contains__(self, key): def __repr__(self): return repr(self.data) def __add__(self, other): - return UserDict(self.data + other) + + if isinstance(other, UserDict): + return self.__class__(self.data + other.data) + + if isinstance(other, type(self.data)): + return self.__class__(self.data + other) + + return self.__class__(self.data + dict(other)) def __radd__(self, other): - return other + self.data + + if isinstance(other, UserDict): + return self.__class__(other.data + self.data) + + if isinstance(other, type(self.data)): + return self.__class__(other + self.data) + + return self.__class__(dict(other) + self.data) def __iadd__(self, other): - self.data += other + + if isinstance(other, UserDict): + self.data += other.data + elif isinstance(other, type(self.data)): + self.data += other + else: + self.data += dict(other) + return self def __sub__(self, other): - return UserDict(self.data - other) + + if isinstance(other, UserDict): + return self.__class__(self.data - other.data) + + if isinstance(other, type(self.data)): + return self.__class__(self.data - other) + + return self.__class__(self.data - dict(other)) def __rsub__(self, other): - return other - self.data + + if isinstance(other, UserDict): + return self.__class__(other.data - self.data) + + if isinstance(other, type(self.data)): + return self.__class__(other - self.data) + + return self.__class__(dict(other) - self.data) def __isub__(self, other): - self.data -= other + + if isinstance(other, UserDict): + self.data -= other.data + elif isinstance(other, type(self.data)): + self.data -= other + else: + self.data -= dict(other) + return self def copy(self): From fa44826862bb9344fd7e97c151dfc2f46abcc93f Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 22 Mar 2019 09:44:23 -0700 Subject: [PATCH 19/36] Add | and |= operators as aliases to + and +=. This was requested by Steven for comparison purposes. The final implementation will only keep one! --- Lib/collections/__init__.py | 31 +++++++++++++++++++++++++++++++ Lib/test/test_doctest.py | 2 +- Objects/dictobject.c | 10 +++++++++- 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index 07125369e1ee02..c48e871a03a8a2 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -1066,6 +1066,37 @@ def __iadd__(self, other): return self + def __or__(self, other): + + if isinstance(other, UserDict): + return self.__class__(self.data | other.data) + + if isinstance(other, type(self.data)): + return self.__class__(self.data | other) + + return self.__class__(self.data | dict(other)) + + def __ror__(self, other): + + if isinstance(other, UserDict): + return self.__class__(other.data | self.data) + + if isinstance(other, type(self.data)): + return self.__class__(other | self.data) + + return self.__class__(dict(other) | self.data) + + def __ior__(self, other): + + if isinstance(other, UserDict): + self.data |= other.data + elif isinstance(other, type(self.data)): + self.data |= other + else: + self.data |= dict(other) + + return self + def __sub__(self, other): if isinstance(other, UserDict): diff --git a/Lib/test/test_doctest.py b/Lib/test/test_doctest.py index 6d4655c693e989..fb08183c07b21d 100644 --- a/Lib/test/test_doctest.py +++ b/Lib/test/test_doctest.py @@ -661,7 +661,7 @@ def non_Python_modules(): r""" >>> import builtins >>> tests = doctest.DocTestFinder().find(builtins) - >>> 800 < len(tests) < 825 # approximate number of objects with docstrings + >>> 800 < len(tests) < 830 # approximate number of objects with docstrings True >>> real_tests = [t for t in tests if len(t.examples) > 0] >>> len(real_tests) # objects that actually have doctests diff --git a/Objects/dictobject.c b/Objects/dictobject.c index f7d3e11bf6d82a..a6a0e06f18fd7e 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -3433,12 +3433,20 @@ static PyNumberMethods dict_as_number = { 0, /*nb_rshift*/ 0, /*nb_and*/ 0, /*nb_xor*/ - 0, /*nb_or*/ + (binaryfunc)dict_add, /*nb_or*/ 0, /*nb_int*/ 0, /*nb_reserved*/ 0, /*nb_float*/ (binaryfunc)dict_iadd, /*nb_inplace_add*/ (binaryfunc)dict_isub, /*nb_inplace_subtract*/ + 0, /*nb_inplace_multiply*/ + 0, /*nb_inplace_remainder*/ + 0, /*nb_inplace_power*/ + 0, /*nb_inplace_lshift*/ + 0, /*nb_inplace_rshift*/ + 0, /*nb_inplace_and*/ + 0, /*nb_inplace_xor*/ + (binaryfunc)dict_iadd, /*nb_inplace_or*/ }; PyDoc_STRVAR(dictionary_doc, From 5d77bf8cadc08d4ee3326c4090c084036020fefe Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 22 Mar 2019 10:04:48 -0700 Subject: [PATCH 20/36] Fix whitespace... again. --- Lib/collections/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index c48e871a03a8a2..5766d70f669498 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -1083,7 +1083,7 @@ def __ror__(self, other): if isinstance(other, type(self.data)): return self.__class__(other | self.data) - + return self.__class__(dict(other) | self.data) def __ior__(self, other): @@ -1094,7 +1094,7 @@ def __ior__(self, other): self.data |= other else: self.data |= dict(other) - + return self def __sub__(self, other): @@ -1125,7 +1125,7 @@ def __isub__(self, other): self.data -= other else: self.data -= dict(other) - + return self def copy(self): From 362657ed95b30a08996391f5111108c958685682 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Mon, 14 Oct 2019 13:34:21 -0700 Subject: [PATCH 21/36] Remove sub/isub/or/ior operators. --- Lib/collections/__init__.py | 62 ------------- Lib/test/test_dict.py | 38 -------- Lib/test/test_doctest.py | 2 +- Objects/dictobject.c | 179 +++--------------------------------- 4 files changed, 16 insertions(+), 265 deletions(-) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index b7752ea6553281..2ccb5a589ead17 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -1013,68 +1013,6 @@ def __iadd__(self, other): return self - def __or__(self, other): - - if isinstance(other, UserDict): - return self.__class__(self.data | other.data) - - if isinstance(other, type(self.data)): - return self.__class__(self.data | other) - - return self.__class__(self.data | dict(other)) - - def __ror__(self, other): - - if isinstance(other, UserDict): - return self.__class__(other.data | self.data) - - if isinstance(other, type(self.data)): - return self.__class__(other | self.data) - - return self.__class__(dict(other) | self.data) - - def __ior__(self, other): - - if isinstance(other, UserDict): - self.data |= other.data - elif isinstance(other, type(self.data)): - self.data |= other - else: - self.data |= dict(other) - - return self - - def __sub__(self, other): - - if isinstance(other, UserDict): - return self.__class__(self.data - other.data) - - if isinstance(other, type(self.data)): - return self.__class__(self.data - other) - - return self.__class__(self.data - dict(other)) - - def __rsub__(self, other): - - if isinstance(other, UserDict): - return self.__class__(other.data - self.data) - - if isinstance(other, type(self.data)): - return self.__class__(other - self.data) - - return self.__class__(dict(other) - self.data) - - def __isub__(self, other): - - if isinstance(other, UserDict): - self.data -= other.data - elif isinstance(other, type(self.data)): - self.data -= other - else: - self.data -= dict(other) - - return self - def __copy__(self): inst = self.__class__.__new__(self.__class__) inst.__dict__.update(self.__dict__) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 4034dab9a173c8..5a08e73ff3bc95 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -63,48 +63,10 @@ def test_addition(self): self.assertIs(a.__add__(()), NotImplemented) self.assertIs(a.__add__("BAD"), NotImplemented) - self.assertIs(a.__radd__(None), NotImplemented) - self.assertIs(a.__radd__(()), NotImplemented) - self.assertIs(a.__radd__("BAD"), NotImplemented) - self.assertRaises(TypeError, a.__iadd__, None) self.assertEqual(a.__iadd__(()), {0: 0, 1: 1, 2: 1}) self.assertRaises(ValueError, a.__iadd__, "BAD") - def test_subtraction(self): - - a = {0: 0, 1: 1, 2: 1} - b = {1: 1, 2: 2, 3: 3} - - c = a.copy() - c -= b - - self.assertEqual(a - b, {0: 0}) - self.assertEqual(c, {0: 0}) - - c = b.copy() - c -= a - - self.assertEqual(b - a, {3: 3}) - self.assertEqual(c, {3: 3}) - - c = a.copy() - c -= (1, 2, 3) - - self.assertEqual(c, {0: 0}) - - self.assertIs(a.__sub__(None), NotImplemented) - self.assertIs(a.__sub__(()), NotImplemented) - self.assertIs(a.__sub__("BAD"), NotImplemented) - - self.assertIs(a.__rsub__(None), NotImplemented) - self.assertIs(a.__rsub__(()), NotImplemented) - self.assertIs(a.__rsub__("BAD"), NotImplemented) - - self.assertRaises(TypeError, a.__isub__, None) - self.assertEqual(a.__isub__(()), {0: 0, 1: 1, 2: 1}) - self.assertEqual(a.__isub__("BAD"), {0: 0, 1: 1, 2: 1}) - def test_bool(self): self.assertIs(not {}, True) self.assertTrue({1: 2}) diff --git a/Lib/test/test_doctest.py b/Lib/test/test_doctest.py index 492cbbb2aa3578..f7c399e526d17f 100644 --- a/Lib/test/test_doctest.py +++ b/Lib/test/test_doctest.py @@ -661,7 +661,7 @@ def non_Python_modules(): r""" >>> import builtins >>> tests = doctest.DocTestFinder().find(builtins) - >>> 800 < len(tests) < 830 # approximate number of objects with docstrings + >>> 800 < len(tests) < 820 # approximate number of objects with docstrings True >>> real_tests = [t for t in tests if len(t.examples) > 0] >>> len(real_tests) # objects that actually have doctests diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 7f0bc87751f143..cd70ccf99e31d0 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2310,69 +2310,6 @@ dict_fromkeys_impl(PyTypeObject *type, PyObject *iterable, PyObject *value) return _PyDict_FromKeys((PyObject *)type, iterable, value); } -/* Used for dict subtraction. */ -static int -dict_diff_arg(PyObject *self, PyObject *arg) -{ - PyObject *keys, *key; - PyObject *keys_method; - int in; - - _Py_IDENTIFIER(keys); - - // XXX: PEP 584 - - // This keys() lookup stuff is a bit complex... it mirrors dict.update: - // https://mail.python.org/pipermail/python-ideas/2019-March/055687.html - - if (_PyObject_LookupAttrId(arg, &PyId_keys, &keys_method) < 0) { - return -1; - } - - if (keys_method && (!PyDict_Check(arg) || - Py_TYPE(arg)->tp_iter != (getiterfunc)dict_iter)) - { - arg = _PyObject_CallNoArg(keys_method); - Py_DECREF(keys_method); - - if (arg == NULL) { - return -1; - } - - keys = PyObject_GetIter(arg); - Py_DECREF(arg); - } - else { - keys = PyObject_GetIter(arg); - } - - if (keys == NULL) { - return -1; - } - - while ((key = PyIter_Next(keys))) { - - in = PyDict_Contains(self, key); - - if (in < 0 || (in && PyDict_DelItem(self, key))) { - Py_DECREF(keys); - Py_DECREF(key); - return -1; - } - - Py_DECREF(key); - } - - Py_DECREF(keys); - - if (PyErr_Occurred()) { - return -1; - } - - return 0; -} - - /* Single-arg dict update; used by dict_update_common and addition ops. */ static int dict_update_arg(PyObject *self, PyObject *arg) @@ -3224,11 +3161,11 @@ dict_sizeof(PyDictObject *mp, PyObject *Py_UNUSED(ignored)) } static PyObject * -dict_add(PyObject *self, PyObject *other) +dict_concat(PyDictObject *self, PyObject *other) { PyObject *new; - if (!PyDict_Check(self) || !PyDict_Check(other)) { + if (!PyDict_Check(other)) { Py_RETURN_NOTIMPLEMENTED; } @@ -3250,7 +3187,7 @@ dict_add(PyObject *self, PyObject *other) return NULL; } - if (dict_update_arg(new, self) || dict_update_arg(new, other)) { + if (dict_update_arg(new, (PyObject*)self) || dict_update_arg(new, other)) { Py_DECREF(new); return NULL; } @@ -3259,7 +3196,7 @@ dict_add(PyObject *self, PyObject *other) } static PyObject * -dict_iadd(PyDictObject *self, PyObject *other) +dict_inplace_concat(PyDictObject *self, PyObject *other) { // XXX: PEP 584 @@ -3279,59 +3216,6 @@ dict_iadd(PyDictObject *self, PyObject *other) return (PyObject *)self; } -static PyObject * -dict_sub(PyObject *self, PyObject *other) -{ - PyObject *new; - - if (!PyDict_Check(self) || !PyDict_Check(other)) { - Py_RETURN_NOTIMPLEMENTED; - } - - if (PyDict_CheckExact(self)) { - new = PyDict_New(); - } - else { - - // XXX: PEP 584 - - // See dict_add for limitations of this construction: - - new = _PyObject_CallNoArg((PyObject *)Py_TYPE(self)); - } - - if (new == NULL) { - return NULL; - } - - if (dict_update_arg(new, self) || dict_diff_arg(new, other)) { - Py_DECREF(new); - return NULL; - } - - return new; -} - -static PyObject * -dict_isub(PyDictObject *self, PyObject *other) -{ - // XXX: PEP 584 - - // See dict_iadd for this removal: - - // if (!PyMapping_Check(other)) { - // Py_RETURN_NOTIMPLEMENTED; - // } - - if (dict_diff_arg((PyObject *)self, other)) { - return NULL; - } - - Py_INCREF((PyObject *)self); - - return (PyObject *)self; -} - PyDoc_STRVAR(getitem__doc__, "x.__getitem__(y) <==> x[y]"); PyDoc_STRVAR(sizeof__doc__, @@ -3423,18 +3307,17 @@ _PyDict_Contains(PyObject *op, PyObject *key, Py_hash_t hash) return (ix != DKIX_EMPTY && value != NULL); } -/* Hack to implement "key in dict" */ static PySequenceMethods dict_as_sequence = { - 0, /* sq_length */ - 0, /* sq_concat */ - 0, /* sq_repeat */ - 0, /* sq_item */ - 0, /* sq_slice */ - 0, /* sq_ass_item */ - 0, /* sq_ass_slice */ - PyDict_Contains, /* sq_contains */ - 0, /* sq_inplace_concat */ - 0, /* sq_inplace_repeat */ + 0, /* sq_length */ + (binaryfunc)dict_concat, /* sq_concat */ + 0, /* sq_repeat */ + 0, /* sq_item */ + 0, /* sq_slice */ + 0, /* sq_ass_item */ + 0, /* sq_ass_slice */ + PyDict_Contains, /* sq_contains */ + (binaryfunc)dict_inplace_concat, /* sq_inplace_concat */ + 0, /* sq_inplace_repeat */ }; static PyObject * @@ -3476,38 +3359,6 @@ dict_iter(PyDictObject *dict) return dictiter_new(dict, &PyDictIterKey_Type); } -static PyNumberMethods dict_as_number = { - (binaryfunc)dict_add, /*nb_add*/ - (binaryfunc)dict_sub, /*nb_subtract*/ - 0, /*nb_multiply*/ - 0, /*nb_remainder*/ - 0, /*nb_divmod*/ - 0, /*nb_power*/ - 0, /*nb_negative*/ - 0, /*nb_positive*/ - 0, /*nb_absolute*/ - 0, /*nb_bool*/ - 0, /*nb_invert*/ - 0, /*nb_lshift*/ - 0, /*nb_rshift*/ - 0, /*nb_and*/ - 0, /*nb_xor*/ - (binaryfunc)dict_add, /*nb_or*/ - 0, /*nb_int*/ - 0, /*nb_reserved*/ - 0, /*nb_float*/ - (binaryfunc)dict_iadd, /*nb_inplace_add*/ - (binaryfunc)dict_isub, /*nb_inplace_subtract*/ - 0, /*nb_inplace_multiply*/ - 0, /*nb_inplace_remainder*/ - 0, /*nb_inplace_power*/ - 0, /*nb_inplace_lshift*/ - 0, /*nb_inplace_rshift*/ - 0, /*nb_inplace_and*/ - 0, /*nb_inplace_xor*/ - (binaryfunc)dict_iadd, /*nb_inplace_or*/ -}; - PyDoc_STRVAR(dictionary_doc, "dict() -> new empty dictionary\n" "dict(mapping) -> new dictionary initialized from a mapping object's\n" @@ -3530,7 +3381,7 @@ PyTypeObject PyDict_Type = { 0, /* tp_setattr */ 0, /* tp_as_async */ (reprfunc)dict_repr, /* tp_repr */ - &dict_as_number, /* tp_as_number */ + 0, /* tp_as_number */ &dict_as_sequence, /* tp_as_sequence */ &dict_as_mapping, /* tp_as_mapping */ PyObject_HashNotImplemented, /* tp_hash */ From 04fb96e44c0c4ed7329914f7ae9a3fa4a4c0d146 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 16 Oct 2019 12:08:27 -0700 Subject: [PATCH 22/36] Remove extra whitespace. --- Lib/collections/__init__.py | 10 ---------- Lib/test/test_dict.py | 4 ---- Objects/dictobject.c | 14 +------------- 3 files changed, 1 insertion(+), 27 deletions(-) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index 2ccb5a589ead17..09758f34b33eca 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -983,34 +983,24 @@ def __contains__(self, key): def __repr__(self): return repr(self.data) def __add__(self, other): - if isinstance(other, UserDict): return self.__class__(self.data + other.data) - if isinstance(other, type(self.data)): return self.__class__(self.data + other) - return self.__class__(self.data + dict(other)) - def __radd__(self, other): - if isinstance(other, UserDict): return self.__class__(other.data + self.data) - if isinstance(other, type(self.data)): return self.__class__(other + self.data) - return self.__class__(dict(other) + self.data) - def __iadd__(self, other): - if isinstance(other, UserDict): self.data += other.data elif isinstance(other, type(self.data)): self.data += other else: self.data += dict(other) - return self def __copy__(self): diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 5a08e73ff3bc95..af73e44cbd202b 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -38,25 +38,21 @@ def test_literal_constructor(self): self.assertEqual(eval(dictliteral), dict(items)) def test_addition(self): - a = {0: 0, 1: 1, 2: 1} b = {1: 1, 2: 2, 3: 3} c = a.copy() c += b - self.assertEqual(a + b, {0: 0, 1: 1, 2: 2, 3: 3}) self.assertEqual(c, {0: 0, 1: 1, 2: 2, 3: 3}) c = b.copy() c += a - self.assertEqual(b + a, {1: 1, 2: 1, 3: 3, 0: 0}) self.assertEqual(c, {1: 1, 2: 1, 3: 3, 0: 0}) c = a.copy() c += [(1, 1), (2, 2), (3, 3)] - self.assertEqual(c, {0: 0, 1: 1, 2: 2, 3: 3}) self.assertIs(a.__add__(None), NotImplemented) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index cd70ccf99e31d0..4b760669d0961d 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -3168,30 +3168,23 @@ dict_concat(PyDictObject *self, PyObject *other) if (!PyDict_Check(other)) { Py_RETURN_NOTIMPLEMENTED; } - if (PyDict_CheckExact(self)) { new = PyDict_New(); } else { - // XXX: PEP 584 - // If subclass constructors/initializers have been overridden to require // at least one arg, this next bit could fail with a confusing TypeError... // dict.fromkeys currently has this issue, though, so nothing new. - new = _PyObject_CallNoArg((PyObject *)Py_TYPE(self)); } - if (new == NULL) { return NULL; } - if (dict_update_arg(new, (PyObject*)self) || dict_update_arg(new, other)) { Py_DECREF(new); return NULL; } - return new; } @@ -3199,20 +3192,15 @@ static PyObject * dict_inplace_concat(PyDictObject *self, PyObject *other) { // XXX: PEP 584 - // Don't fall back to __add__ here? Could be confusing for subclasses... // https://mail.python.org/pipermail/python-ideas/2019-March/055581.html - // if (!PyMapping_Check(other)) { // Py_RETURN_NOTIMPLEMENTED; // } - - if (dict_update_arg((PyObject *)self, other)) { + if (dict_update_arg((PyObject *)self, other) < 0) { return NULL; } - Py_INCREF((PyObject *)self); - return (PyObject *)self; } From 567a218d60b0600c19f4afaef7e7450f067ab34e Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 16 Oct 2019 12:11:44 -0700 Subject: [PATCH 23/36] Use copy() instead of __new__() and update(self). --- Objects/dictobject.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 4b760669d0961d..512539b109715e 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -3169,19 +3169,16 @@ dict_concat(PyDictObject *self, PyObject *other) Py_RETURN_NOTIMPLEMENTED; } if (PyDict_CheckExact(self)) { - new = PyDict_New(); + new = PyDict_Copy((PyObject*)self); } else { - // XXX: PEP 584 - // If subclass constructors/initializers have been overridden to require - // at least one arg, this next bit could fail with a confusing TypeError... - // dict.fromkeys currently has this issue, though, so nothing new. - new = _PyObject_CallNoArg((PyObject *)Py_TYPE(self)); + _Py_IDENTIFIER(copy); + new = _PyObject_CallMethodIdNoArgs((PyObject*)self, &PyId_copy); } if (new == NULL) { return NULL; } - if (dict_update_arg(new, (PyObject*)self) || dict_update_arg(new, other)) { + if (dict_update_arg(new, other) < 0) { Py_DECREF(new); return NULL; } From 159ebda140997a961bb453cf92c47c1bb7e3e11a Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Mon, 14 Oct 2019 17:58:05 -0700 Subject: [PATCH 24/36] Clarify NEWS entry. --- .../2019-03-02-23-03-34.bpo-36144.LRl4LS.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-03-02-23-03-34.bpo-36144.LRl4LS.rst b/Misc/NEWS.d/next/Core and Builtins/2019-03-02-23-03-34.bpo-36144.LRl4LS.rst index 4469bbbadb9c5f..1df4c77f6c6c3f 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2019-03-02-23-03-34.bpo-36144.LRl4LS.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2019-03-02-23-03-34.bpo-36144.LRl4LS.rst @@ -1,2 +1,2 @@ -:class:`dict` objects now support PEP 584 addition and subtraction operations, with semantics similar to :meth:`dict.update`. -Patch by Brandt Bucher. \ No newline at end of file +:class:`dict` (and :class:`collections.UserDict`) objects now support PEP 584's merge (``+``) and update (``+=``) operators. +Patch by Brandt Bucher. From 28c5c56af558fabc83067f811a2817a59ccad275 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 27 Dec 2019 16:17:04 -0800 Subject: [PATCH 25/36] + -> | --- Lib/test/test_builtin.py | 2 +- Lib/test/test_dict.py | 28 +++++---- .../2019-03-02-23-03-34.bpo-36144.LRl4LS.rst | 2 +- Objects/dictobject.c | 62 ++++++++----------- 4 files changed, 45 insertions(+), 49 deletions(-) diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index a4af0fcde44162..abccf322274448 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -1388,7 +1388,6 @@ def test_sum(self): self.assertEqual(sum(Squares(10)), 285) self.assertEqual(sum(iter(Squares(10))), 285) self.assertEqual(sum([[1], [2], [3]], []), [1, 2, 3]) - self.assertEqual(sum([{3:4}, {5:6}], {1:2}), {1:2, 3:4, 5:6}) self.assertEqual(sum(range(10), 1000), 1045) self.assertEqual(sum(range(10), start=1000), 1045) @@ -1420,6 +1419,7 @@ def test_sum(self): self.assertRaises(TypeError, sum, values, bytearray(b'')) self.assertRaises(TypeError, sum, [[1], [2], [3]]) self.assertRaises(TypeError, sum, [{2:3}]) + self.assertRaises(TypeError, sum, [{2:3}]*2, {2:3}) self.assertRaises(TypeError, sum, [], '') self.assertRaises(TypeError, sum, [], b'') self.assertRaises(TypeError, sum, [], bytearray()) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index af73e44cbd202b..9cde7102cf884d 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -37,31 +37,35 @@ def test_literal_constructor(self): dictliteral = '{' + ', '.join(formatted_items) + '}' self.assertEqual(eval(dictliteral), dict(items)) - def test_addition(self): + def test_pep_584(self): + a = {0: 0, 1: 1, 2: 1} b = {1: 1, 2: 2, 3: 3} c = a.copy() - c += b - self.assertEqual(a + b, {0: 0, 1: 1, 2: 2, 3: 3}) + c |= b + + self.assertEqual(a | b, {0: 0, 1: 1, 2: 2, 3: 3}) self.assertEqual(c, {0: 0, 1: 1, 2: 2, 3: 3}) c = b.copy() - c += a - self.assertEqual(b + a, {1: 1, 2: 1, 3: 3, 0: 0}) + c |= a + + self.assertEqual(b | a, {1: 1, 2: 1, 3: 3, 0: 0}) self.assertEqual(c, {1: 1, 2: 1, 3: 3, 0: 0}) c = a.copy() - c += [(1, 1), (2, 2), (3, 3)] + c |= [(1, 1), (2, 2), (3, 3)] + self.assertEqual(c, {0: 0, 1: 1, 2: 2, 3: 3}) - self.assertIs(a.__add__(None), NotImplemented) - self.assertIs(a.__add__(()), NotImplemented) - self.assertIs(a.__add__("BAD"), NotImplemented) + self.assertIs(a.__or__(None), NotImplemented) + self.assertIs(a.__or__(()), NotImplemented) + self.assertIs(a.__or__("BAD"), NotImplemented) - self.assertRaises(TypeError, a.__iadd__, None) - self.assertEqual(a.__iadd__(()), {0: 0, 1: 1, 2: 1}) - self.assertRaises(ValueError, a.__iadd__, "BAD") + self.assertRaises(TypeError, a.__ior__, None) + self.assertEqual(a.__ior__(()), {0: 0, 1: 1, 2: 1}) + self.assertRaises(ValueError, a.__ior__, "BAD") def test_bool(self): self.assertIs(not {}, True) diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-03-02-23-03-34.bpo-36144.LRl4LS.rst b/Misc/NEWS.d/next/Core and Builtins/2019-03-02-23-03-34.bpo-36144.LRl4LS.rst index 1df4c77f6c6c3f..7d6d076ea7d4d3 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2019-03-02-23-03-34.bpo-36144.LRl4LS.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2019-03-02-23-03-34.bpo-36144.LRl4LS.rst @@ -1,2 +1,2 @@ -:class:`dict` (and :class:`collections.UserDict`) objects now support PEP 584's merge (``+``) and update (``+=``) operators. +:class:`dict` (and :class:`collections.UserDict`) objects now support PEP 584's merge (``|``) and update (``|=``) operators. Patch by Brandt Bucher. diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 512539b109715e..4f3ba91c4cce18 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2310,7 +2310,7 @@ dict_fromkeys_impl(PyTypeObject *type, PyObject *iterable, PyObject *value) return _PyDict_FromKeys((PyObject *)type, iterable, value); } -/* Single-arg dict update; used by dict_update_common and addition ops. */ +/* Single-arg dict update; used by dict_update_common and operators. */ static int dict_update_arg(PyObject *self, PyObject *arg) { @@ -3161,24 +3161,16 @@ dict_sizeof(PyDictObject *mp, PyObject *Py_UNUSED(ignored)) } static PyObject * -dict_concat(PyDictObject *self, PyObject *other) +dict_or(PyObject *self, PyObject *other) { - PyObject *new; - - if (!PyDict_Check(other)) { + if (!PyDict_Check(self) || !PyDict_Check(other)) { Py_RETURN_NOTIMPLEMENTED; } - if (PyDict_CheckExact(self)) { - new = PyDict_Copy((PyObject*)self); - } - else { - _Py_IDENTIFIER(copy); - new = _PyObject_CallMethodIdNoArgs((PyObject*)self, &PyId_copy); - } + PyObject *new = PyDict_Copy(self); if (new == NULL) { return NULL; } - if (dict_update_arg(new, other) < 0) { + if (dict_update_arg(new, other)) { Py_DECREF(new); return NULL; } @@ -3186,19 +3178,13 @@ dict_concat(PyDictObject *self, PyObject *other) } static PyObject * -dict_inplace_concat(PyDictObject *self, PyObject *other) -{ - // XXX: PEP 584 - // Don't fall back to __add__ here? Could be confusing for subclasses... - // https://mail.python.org/pipermail/python-ideas/2019-March/055581.html - // if (!PyMapping_Check(other)) { - // Py_RETURN_NOTIMPLEMENTED; - // } - if (dict_update_arg((PyObject *)self, other) < 0) { +dict_ior(PyObject *self, PyObject *other) +{ + if (dict_update_arg(self, other)) { return NULL; } - Py_INCREF((PyObject *)self); - return (PyObject *)self; + Py_INCREF(self); + return self; } PyDoc_STRVAR(getitem__doc__, "x.__getitem__(y) <==> x[y]"); @@ -3292,17 +3278,23 @@ _PyDict_Contains(PyObject *op, PyObject *key, Py_hash_t hash) return (ix != DKIX_EMPTY && value != NULL); } +/* Hack to implement "key in dict" */ static PySequenceMethods dict_as_sequence = { - 0, /* sq_length */ - (binaryfunc)dict_concat, /* sq_concat */ - 0, /* sq_repeat */ - 0, /* sq_item */ - 0, /* sq_slice */ - 0, /* sq_ass_item */ - 0, /* sq_ass_slice */ - PyDict_Contains, /* sq_contains */ - (binaryfunc)dict_inplace_concat, /* sq_inplace_concat */ - 0, /* sq_inplace_repeat */ + 0, /* sq_length */ + 0, /* sq_concat */ + 0, /* sq_repeat */ + 0, /* sq_item */ + 0, /* sq_slice */ + 0, /* sq_ass_item */ + 0, /* sq_ass_slice */ + PyDict_Contains, /* sq_contains */ + 0, /* sq_inplace_concat */ + 0, /* sq_inplace_repeat */ +}; + +static PyNumberMethods dict_as_number = { + .nb_or = dict_or, + .nb_inplace_or = dict_ior, }; static PyObject * @@ -3366,7 +3358,7 @@ PyTypeObject PyDict_Type = { 0, /* tp_setattr */ 0, /* tp_as_async */ (reprfunc)dict_repr, /* tp_repr */ - 0, /* tp_as_number */ + &dict_as_number, /* tp_as_number */ &dict_as_sequence, /* tp_as_sequence */ &dict_as_mapping, /* tp_as_mapping */ PyObject_HashNotImplemented, /* tp_hash */ From 695138089f9f1495b904a711e5053f3fff3cf011 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 27 Dec 2019 16:23:45 -0800 Subject: [PATCH 26/36] Update UserDict. --- Lib/collections/__init__.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index 09758f34b33eca..eb0ab07b05ef96 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -982,25 +982,25 @@ def __contains__(self, key): # Now, add the methods in dicts but not in MutableMapping def __repr__(self): return repr(self.data) - def __add__(self, other): + def __or__(self, other): if isinstance(other, UserDict): - return self.__class__(self.data + other.data) + return self.__class__(self.data | other.data) if isinstance(other, type(self.data)): - return self.__class__(self.data + other) - return self.__class__(self.data + dict(other)) - def __radd__(self, other): + return self.__class__(self.data | other) + return self.__class__(self.data | dict(other)) + def __ror__(self, other): if isinstance(other, UserDict): - return self.__class__(other.data + self.data) + return self.__class__(other.data | self.data) if isinstance(other, type(self.data)): - return self.__class__(other + self.data) - return self.__class__(dict(other) + self.data) - def __iadd__(self, other): + return self.__class__(other | self.data) + return self.__class__(dict(other) | self.data) + def __ior__(self, other): if isinstance(other, UserDict): - self.data += other.data + self.data |= other.data elif isinstance(other, type(self.data)): - self.data += other + self.data |= other else: - self.data += dict(other) + self.data |= dict(other) return self def __copy__(self): From 35672479061b978e53b9e21a5acfe266e9d62f0d Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 4 Feb 2020 22:09:29 -0800 Subject: [PATCH 27/36] Modernize UserDict.__or__ trio. --- Lib/collections/__init__.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index ea8fe95439dbd2..b0c6f7ce658703 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -985,20 +985,18 @@ def __or__(self, other): return self.__class__(self.data | other.data) if isinstance(other, type(self.data)): return self.__class__(self.data | other) - return self.__class__(self.data | dict(other)) + return NotImplemented def __ror__(self, other): if isinstance(other, UserDict): return self.__class__(other.data | self.data) if isinstance(other, type(self.data)): return self.__class__(other | self.data) - return self.__class__(dict(other) | self.data) + return NotImplemented def __ior__(self, other): if isinstance(other, UserDict): self.data |= other.data - elif isinstance(other, type(self.data)): - self.data |= other else: - self.data |= dict(other) + self.data |= other return self def __copy__(self): From 4588b563985a78e68ff99ec165430d18187b849d Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 4 Feb 2020 22:17:07 -0800 Subject: [PATCH 28/36] Rename test. --- Lib/test/test_dict.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index fda95420ddee2a..6ec57598fad0db 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -37,7 +37,7 @@ def test_literal_constructor(self): dictliteral = '{' + ', '.join(formatted_items) + '}' self.assertEqual(eval(dictliteral), dict(items)) - def test_pep_584(self): + def test_merge_operator(self): a = {0: 0, 1: 1, 2: 1} b = {1: 1, 2: 2, 3: 3} From 977ffcd9bf0dc64e52a366d70bd211bdb2fc8e34 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 5 Feb 2020 08:45:48 -0800 Subject: [PATCH 29/36] Feedback from code review. --- Lib/collections/__init__.py | 4 ++-- Lib/test/test_dict.py | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index b0c6f7ce658703..ac236047544652 100644 --- a/Lib/collections/__init__.py +++ b/Lib/collections/__init__.py @@ -983,13 +983,13 @@ def __repr__(self): return repr(self.data) def __or__(self, other): if isinstance(other, UserDict): return self.__class__(self.data | other.data) - if isinstance(other, type(self.data)): + if isinstance(other, dict): return self.__class__(self.data | other) return NotImplemented def __ror__(self, other): if isinstance(other, UserDict): return self.__class__(other.data | self.data) - if isinstance(other, type(self.data)): + if isinstance(other, dict): return self.__class__(other | self.data) return NotImplemented def __ior__(self, other): diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 6ec57598fad0db..0b5935eafa3f6b 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -62,10 +62,12 @@ def test_merge_operator(self): self.assertIs(a.__or__(None), NotImplemented) self.assertIs(a.__or__(()), NotImplemented) self.assertIs(a.__or__("BAD"), NotImplemented) + self.assertIs(a.__or__(""), NotImplemented) self.assertRaises(TypeError, a.__ior__, None) self.assertEqual(a.__ior__(()), {0: 0, 1: 1, 2: 1}) self.assertRaises(ValueError, a.__ior__, "BAD") + self.assertEqual(a.__ior__(""), {0: 0, 1: 1, 2: 1}) def test_bool(self): self.assertIs(not {}, True) From a26a98e786ca0222058c2ad6f8cb95bd4257ed78 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 5 Feb 2020 09:41:45 -0800 Subject: [PATCH 30/36] Actually call the "copy" and "update" methods of subclasses. --- Objects/dictobject.c | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index a3e24e323bec69..e4ec76e5f0f78e 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -3160,17 +3160,42 @@ dict_sizeof(PyDictObject *mp, PyObject *Py_UNUSED(ignored)) return PyLong_FromSsize_t(_PyDict_SizeOf(mp)); } +static int +subclass_update_arg(PyObject *self, PyObject *other) +{ + _Py_IDENTIFIER(update); + PyObject *tmp = _PyObject_CallMethodIdOneArg(self, &PyId_update, other); + if (tmp == NULL) { + return -1; + } + Py_DECREF(tmp); + return 0; +} + static PyObject * dict_or(PyObject *self, PyObject *other) { if (!PyDict_Check(self) || !PyDict_Check(other)) { Py_RETURN_NOTIMPLEMENTED; } - PyObject *new = PyDict_Copy(self); + PyObject *new; + if (PyDict_CheckExact(self)) { + new = PyDict_Copy(self); + } + else { + _Py_IDENTIFIER(copy); + new = _PyObject_CallMethodIdNoArgs(self, &PyId_copy); + } if (new == NULL) { return NULL; } - if (dict_update_arg(new, other)) { + if (PyDict_CheckExact(new)) { + if (dict_update_arg(new, other)) { + Py_DECREF(new); + return NULL; + } + } + else if (subclass_update_arg(new, other)) { Py_DECREF(new); return NULL; } @@ -3180,7 +3205,12 @@ dict_or(PyObject *self, PyObject *other) static PyObject * dict_ior(PyObject *self, PyObject *other) { - if (dict_update_arg(self, other)) { + if (PyDict_CheckExact(self)) { + if (dict_update_arg(self, other)) { + return NULL; + } + } + else if (subclass_update_arg(self, other)) { return NULL; } Py_INCREF(self); From dfa7009d77641d83ac9ad9ca9ae70f92415a283d Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 5 Feb 2020 10:59:54 -0800 Subject: [PATCH 31/36] Consolidate shared logic. --- Objects/dictobject.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index e4ec76e5f0f78e..3df1a9db72dd65 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -3160,9 +3160,17 @@ dict_sizeof(PyDictObject *mp, PyObject *Py_UNUSED(ignored)) return PyLong_FromSsize_t(_PyDict_SizeOf(mp)); } +/* A subclass-friendly dict_update_arg. */ static int subclass_update_arg(PyObject *self, PyObject *other) { + if (PyDict_CheckExact(self)) { + return dict_update_arg(self, other); + } + if (!PyDict_Check(self)) { + PyErr_BadInternalCall(); + return -1; + } _Py_IDENTIFIER(update); PyObject *tmp = _PyObject_CallMethodIdOneArg(self, &PyId_update, other); if (tmp == NULL) { @@ -3189,13 +3197,7 @@ dict_or(PyObject *self, PyObject *other) if (new == NULL) { return NULL; } - if (PyDict_CheckExact(new)) { - if (dict_update_arg(new, other)) { - Py_DECREF(new); - return NULL; - } - } - else if (subclass_update_arg(new, other)) { + if (subclass_update_arg(new, other)) { Py_DECREF(new); return NULL; } @@ -3205,12 +3207,7 @@ dict_or(PyObject *self, PyObject *other) static PyObject * dict_ior(PyObject *self, PyObject *other) { - if (PyDict_CheckExact(self)) { - if (dict_update_arg(self, other)) { - return NULL; - } - } - else if (subclass_update_arg(self, other)) { + if (subclass_update_arg(self, other)) { return NULL; } Py_INCREF(self); From 7c40ba6f208846151af35054d443c345b851513c Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 5 Feb 2020 22:11:19 -0800 Subject: [PATCH 32/36] Check the return type of self.copy(). --- Objects/dictobject.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 3df1a9db72dd65..ea9e97e4f758a9 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -3197,6 +3197,10 @@ dict_or(PyObject *self, PyObject *other) if (new == NULL) { return NULL; } + if (!PyDict_Check(new)) { + return PyErr_Format(PyExc_TypeError, + "copy() returned %s (expected dict)", Py_TYPE(new)->tp_name); + } if (subclass_update_arg(new, other)) { Py_DECREF(new); return NULL; From b977a2757457c933929e693556e07aa0c791161a Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 5 Feb 2020 23:07:42 -0800 Subject: [PATCH 33/36] Add tests for subclasses. --- Lib/test/test_dict.py | 61 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 0b5935eafa3f6b..81ed120ff370f0 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -69,6 +69,67 @@ def test_merge_operator(self): self.assertRaises(ValueError, a.__ior__, "BAD") self.assertEqual(a.__ior__(""), {0: 0, 1: 1, 2: 1}) + def test_merge_operator_subclass(self): + + class DictSubclass(dict): + """Normally we'd use a mock, but we need this to be a real dict subclass.""" + + def __init__(self, /, *args, **kwargs): + super().__init__(*args, **kwargs) + self.copied = self.updated = False + + def copy(self): + self.copied = True + return DictSubclass(super().copy()) + + def update(self, /, *args, **kwargs): + self.updated = True + return super().update(*args, **kwargs) + + a = DictSubclass({0: 0, 1: 1, 2: 1}) + b = {1: 1, 2: 2, 3: 3} + + c = a | b + + self.assertDictEqual(c, {0: 0, 1: 1, 2: 2, 3: 3}) + self.assertIs(type(c), DictSubclass) + self.assertTrue(a.copied) + self.assertFalse(a.updated) + self.assertFalse(c.copied) + self.assertTrue(c.updated) + + a.copied = False # Reset. + + c = b | a + + self.assertDictEqual(c, {1: 1, 2: 1, 3: 3, 0: 0}) + self.assertIs(type(c), dict) + self.assertFalse(a.copied) + self.assertFalse(a.updated) + + c = a.copy() + c |= [(1, 1), (2, 2), (3, 3)] + + self.assertEqual(c, {0: 0, 1: 1, 2: 2, 3: 3}) + self.assertIs(type(c), DictSubclass) + self.assertTrue(a.copied) + self.assertFalse(a.updated) + self.assertFalse(c.copied) + self.assertTrue(c.updated) + + def test_merge_operator_subclass_bad_copy(self): + + class DictSubclassBad(dict): + + def copy(self): + return None + + a = DictSubclassBad({0: 0, 1: 1, 2: 1}) + b = {1: 1, 2: 2, 3: 3} + + self.assertRaises(TypeError, a.__or__, b) + self.assertEqual(b | a, {1: 1, 2: 1, 3: 3, 0: 0}) + def test_bool(self): self.assertIs(not {}, True) self.assertTrue({1: 2}) From f09ba7ad79ad4a9eeddd34fc16a5d6d661f91b04 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 6 Feb 2020 08:20:48 -0800 Subject: [PATCH 34/36] Strip whitespace. --- Lib/test/test_dict.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 81ed120ff370f0..3875248ff1b9f3 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -73,7 +73,7 @@ def test_merge_operator_subclass(self): class DictSubclass(dict): """Normally we'd use a mock, but we need this to be a real dict subclass.""" - + def __init__(self, /, *args, **kwargs): super().__init__(*args, **kwargs) self.copied = self.updated = False From ec3366485460296d760fe703384e44402fbb915e Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 6 Feb 2020 16:09:03 -0800 Subject: [PATCH 35/36] Remove calls to "update" method when merging. --- Lib/test/test_dict.py | 20 +------------------- Objects/dictobject.c | 26 +++----------------------- 2 files changed, 4 insertions(+), 42 deletions(-) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 3875248ff1b9f3..5ea3f52140ca45 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -76,16 +76,12 @@ class DictSubclass(dict): def __init__(self, /, *args, **kwargs): super().__init__(*args, **kwargs) - self.copied = self.updated = False + self.copied = False def copy(self): self.copied = True return DictSubclass(super().copy()) - def update(self, /, *args, **kwargs): - self.updated = True - return super().update(*args, **kwargs) - a = DictSubclass({0: 0, 1: 1, 2: 1}) b = {1: 1, 2: 2, 3: 3} @@ -94,9 +90,6 @@ def update(self, /, *args, **kwargs): self.assertDictEqual(c, {0: 0, 1: 1, 2: 2, 3: 3}) self.assertIs(type(c), DictSubclass) self.assertTrue(a.copied) - self.assertFalse(a.updated) - self.assertFalse(c.copied) - self.assertTrue(c.updated) a.copied = False # Reset. @@ -105,17 +98,6 @@ def update(self, /, *args, **kwargs): self.assertDictEqual(c, {1: 1, 2: 1, 3: 3, 0: 0}) self.assertIs(type(c), dict) self.assertFalse(a.copied) - self.assertFalse(a.updated) - - c = a.copy() - c |= [(1, 1), (2, 2), (3, 3)] - - self.assertEqual(c, {0: 0, 1: 1, 2: 2, 3: 3}) - self.assertIs(type(c), DictSubclass) - self.assertTrue(a.copied) - self.assertFalse(a.updated) - self.assertFalse(c.copied) - self.assertTrue(c.updated) def test_merge_operator_subclass_bad_copy(self): diff --git a/Objects/dictobject.c b/Objects/dictobject.c index ea9e97e4f758a9..957e032a381d64 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -3160,26 +3160,6 @@ dict_sizeof(PyDictObject *mp, PyObject *Py_UNUSED(ignored)) return PyLong_FromSsize_t(_PyDict_SizeOf(mp)); } -/* A subclass-friendly dict_update_arg. */ -static int -subclass_update_arg(PyObject *self, PyObject *other) -{ - if (PyDict_CheckExact(self)) { - return dict_update_arg(self, other); - } - if (!PyDict_Check(self)) { - PyErr_BadInternalCall(); - return -1; - } - _Py_IDENTIFIER(update); - PyObject *tmp = _PyObject_CallMethodIdOneArg(self, &PyId_update, other); - if (tmp == NULL) { - return -1; - } - Py_DECREF(tmp); - return 0; -} - static PyObject * dict_or(PyObject *self, PyObject *other) { @@ -3198,10 +3178,10 @@ dict_or(PyObject *self, PyObject *other) return NULL; } if (!PyDict_Check(new)) { - return PyErr_Format(PyExc_TypeError, + return PyErr_Format(PyExc_TypeError, "copy() returned %s (expected dict)", Py_TYPE(new)->tp_name); } - if (subclass_update_arg(new, other)) { + if (dict_update_arg(new, other)) { Py_DECREF(new); return NULL; } @@ -3211,7 +3191,7 @@ dict_or(PyObject *self, PyObject *other) static PyObject * dict_ior(PyObject *self, PyObject *other) { - if (subclass_update_arg(self, other)) { + if (dict_update_arg(self, other)) { return NULL; } Py_INCREF(self); From 4afd3b6dafb340c4f62ed9ac007cdf45f5d56b5a Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sun, 16 Feb 2020 22:30:36 -0800 Subject: [PATCH 36/36] Revert subclass-preserving stuff. --- Lib/test/test_dict.py | 43 ------------------------------------------- Objects/dictobject.c | 13 +------------ 2 files changed, 1 insertion(+), 55 deletions(-) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 5ea3f52140ca45..0b5935eafa3f6b 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -69,49 +69,6 @@ def test_merge_operator(self): self.assertRaises(ValueError, a.__ior__, "BAD") self.assertEqual(a.__ior__(""), {0: 0, 1: 1, 2: 1}) - def test_merge_operator_subclass(self): - - class DictSubclass(dict): - """Normally we'd use a mock, but we need this to be a real dict subclass.""" - - def __init__(self, /, *args, **kwargs): - super().__init__(*args, **kwargs) - self.copied = False - - def copy(self): - self.copied = True - return DictSubclass(super().copy()) - - a = DictSubclass({0: 0, 1: 1, 2: 1}) - b = {1: 1, 2: 2, 3: 3} - - c = a | b - - self.assertDictEqual(c, {0: 0, 1: 1, 2: 2, 3: 3}) - self.assertIs(type(c), DictSubclass) - self.assertTrue(a.copied) - - a.copied = False # Reset. - - c = b | a - - self.assertDictEqual(c, {1: 1, 2: 1, 3: 3, 0: 0}) - self.assertIs(type(c), dict) - self.assertFalse(a.copied) - - def test_merge_operator_subclass_bad_copy(self): - - class DictSubclassBad(dict): - - def copy(self): - return None - - a = DictSubclassBad({0: 0, 1: 1, 2: 1}) - b = {1: 1, 2: 2, 3: 3} - - self.assertRaises(TypeError, a.__or__, b) - self.assertEqual(b | a, {1: 1, 2: 1, 3: 3, 0: 0}) - def test_bool(self): self.assertIs(not {}, True) self.assertTrue({1: 2}) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 957e032a381d64..a3e24e323bec69 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -3166,21 +3166,10 @@ dict_or(PyObject *self, PyObject *other) if (!PyDict_Check(self) || !PyDict_Check(other)) { Py_RETURN_NOTIMPLEMENTED; } - PyObject *new; - if (PyDict_CheckExact(self)) { - new = PyDict_Copy(self); - } - else { - _Py_IDENTIFIER(copy); - new = _PyObject_CallMethodIdNoArgs(self, &PyId_copy); - } + PyObject *new = PyDict_Copy(self); if (new == NULL) { return NULL; } - if (!PyDict_Check(new)) { - return PyErr_Format(PyExc_TypeError, - "copy() returned %s (expected dict)", Py_TYPE(new)->tp_name); - } if (dict_update_arg(new, other)) { Py_DECREF(new); return NULL;