From 2aaf98c16ae3070378de523a173e29644037d8bd Mon Sep 17 00:00:00 2001 From: INADA Naoki Date: Wed, 26 Sep 2018 12:59:00 +0900 Subject: [PATCH] bpo-34320: Fix dict(o) didn't copy order of dict subclass (GH-8624) When dict subclass overrides order (`__iter__()`, `keys()`, and `items()`), `dict(o)` should use it instead of dict ordering. https://bugs.python.org/issue34320 --- Lib/test/test_builtin.py | 9 ++++++ Lib/test/test_call.py | 17 +++++++++++ Lib/test/test_dict.py | 30 +++++++++++++++++++ .../2018-08-02-22-34-59.bpo-34320.hNshAA.rst | 1 + Objects/dictobject.c | 4 ++- 5 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-08-02-22-34-59.bpo-34320.hNshAA.rst diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index 46cf2d32836..dafcf004c09 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -1968,6 +1968,15 @@ class TestType(unittest.TestCase): with self.assertRaises(TypeError): type('A', (B,), {'__slots__': '__weakref__'}) + def test_namespace_order(self): + # bpo-34320: namespace should preserve order + od = collections.OrderedDict([('a', 1), ('b', 2)]) + od.move_to_end('a') + expected = list(od.items()) + + C = type('C', (), od) + self.assertEqual(list(C.__dict__.items())[:2], [('b', 2), ('a', 1)]) + def load_tests(loader, tests, pattern): from doctest import DocTestSuite diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index ec9697a444f..45b34e46a5a 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -9,6 +9,23 @@ import struct import collections import itertools + +class FunctionCalls(unittest.TestCase): + + def test_kwargs_order(self): + # bpo-34320: **kwargs should preserve order of passed OrderedDict + od = collections.OrderedDict([('a', 1), ('b', 2)]) + od.move_to_end('a') + expected = list(od.items()) + + def fn(**kw): + return kw + + res = fn(**od) + self.assertIsInstance(res, dict) + self.assertEqual(list(res.items()), expected) + + # The test cases here cover several paths through the function calling # code. They depend on the METH_XXX flag that is used to define a C # function, which can't be verified from Python. If the METH_XXX decl diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 38521bbf663..90c0a3131a7 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -1222,6 +1222,36 @@ class DictTest(unittest.TestCase): self.assertRaises(RuntimeError, iter_and_mutate) + def test_dict_copy_order(self): + # bpo-34320 + od = collections.OrderedDict([('a', 1), ('b', 2)]) + od.move_to_end('a') + expected = list(od.items()) + + copy = dict(od) + self.assertEqual(list(copy.items()), expected) + + # dict subclass doesn't override __iter__ + class CustomDict(dict): + pass + + pairs = [('a', 1), ('b', 2), ('c', 3)] + + d = CustomDict(pairs) + self.assertEqual(pairs, list(dict(d).items())) + + class CustomReversedDict(dict): + def keys(self): + return reversed(list(dict.keys(self))) + + __iter__ = keys + + def items(self): + return reversed(dict.items(self)) + + d = CustomReversedDict(pairs) + self.assertEqual(pairs[::-1], list(dict(d).items())) + class CAPITest(unittest.TestCase): diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-08-02-22-34-59.bpo-34320.hNshAA.rst b/Misc/NEWS.d/next/Core and Builtins/2018-08-02-22-34-59.bpo-34320.hNshAA.rst new file mode 100644 index 00000000000..ce5b3397e65 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-08-02-22-34-59.bpo-34320.hNshAA.rst @@ -0,0 +1 @@ +Fix ``dict(od)`` didn't copy iteration order of OrderedDict. diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 413557d6674..fec69678c50 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -235,6 +235,8 @@ static Py_ssize_t lookdict_split(PyDictObject *mp, PyObject *key, static int dictresize(PyDictObject *mp, Py_ssize_t minused); +static PyObject* dict_iter(PyDictObject *dict); + /*Global counter used to set ma_version_tag field of dictionary. * It is incremented each time that a dictionary is created and each * time that a dictionary is modified. */ @@ -2379,7 +2381,7 @@ dict_merge(PyObject *a, PyObject *b, int override) return -1; } mp = (PyDictObject*)a; - if (PyDict_Check(b)) { + if (PyDict_Check(b) && (Py_TYPE(b)->tp_iter == (getiterfunc)dict_iter)) { other = (PyDictObject*)b; if (other == mp || other->ma_used == 0) /* a.update(a) or a.update({}); nothing to do */