bpo-35224: Reverse evaluation order of key: value in dict comprehensions (GH-14139)

… as proposed in PEP 572; key is now evaluated before value.

https://bugs.python.org/issue35224

diff --git a/Doc/library/dis.rst b/Doc/library/dis.rst
index 5b79be626626de74995280b85cabbd421e0fb6a9..39a3e130afd3e0fcab3466b88473779f99246f3e 100644 (file)
--- a/Doc/library/dis.rst
+++ b/Doc/library/dis.rst
@@ -645,10 +645,12 @@ the original TOS1.
 
 .. opcode:: MAP_ADD (i)
 
-   Calls ``dict.setitem(TOS1[-i], TOS, TOS1)``.  Used to implement dict
+   Calls ``dict.__setitem__(TOS1[-i], TOS1, TOS)``.  Used to implement dict
    comprehensions.
 
    .. versionadded:: 3.1
+   .. versionchanged:: 3.8
+      Map value is TOS and map key is TOS1. Before, those were reversed.
 
 For all of the :opcode:`SET_ADD`, :opcode:`LIST_APPEND` and :opcode:`MAP_ADD`
 instructions, while the added value or key/value pair is popped off, the

diff --git a/Doc/reference/expressions.rst b/Doc/reference/expressions.rst
index 8b7110615240ee988c76e668f5e40a2af3b591c4..432327a87c3748461d173126e715b65a8bef562a 100644 (file)
--- a/Doc/reference/expressions.rst
+++ b/Doc/reference/expressions.rst
@@ -337,6 +337,12 @@ all mutable objects.)  Clashes between duplicate keys are not detected; the last
 datum (textually rightmost in the display) stored for a given key value
 prevails.
 
+.. versionchanged:: 3.8
+   Prior to Python 3.8, in dict comprehensions, the evaluation order of key
+   and value was not well-defined.  In CPython, the value was evaluated before
+   the key.  Starting with 3.8, the key is evaluated before the value, as
+   proposed by :pep:`572`.
+
 
 .. _genexpr:
 

diff --git a/Lib/test/test_dictcomps.py b/Lib/test/test_dictcomps.py
index afe68a8de75371793a6913f36eb2d8e4cef4ffa8..927e3103e664b0a677794b2d653c913231ae1d7b 100644 (file)
--- a/Lib/test/test_dictcomps.py
+++ b/Lib/test/test_dictcomps.py
@@ -81,6 +81,35 @@ class DictComprehensionTest(unittest.TestCase):
             compile("{x: y for y, x in ((1, 2), (3, 4))} += 5", "<test>",
                     "exec")
 
+    def test_evaluation_order(self):
+        expected = {
+            'H': 'W',
+            'e': 'o',
+            'l': 'l',
+            'o': 'd',
+        }
+
+        expected_calls = [
+            ('key', 'H'), ('value', 'W'),
+            ('key', 'e'), ('value', 'o'),
+            ('key', 'l'), ('value', 'r'),
+            ('key', 'l'), ('value', 'l'),
+            ('key', 'o'), ('value', 'd'),
+        ]
+
+        actual_calls = []
+
+        def add_call(pos, value):
+            actual_calls.append((pos, value))
+            return value
+
+        actual = {
+            add_call('key', k): add_call('value', v)
+            for k, v in zip('Hello', 'World')
+        }
+
+        self.assertEqual(actual, expected)
+        self.assertEqual(actual_calls, expected_calls)
 
 if __name__ == "__main__":
     unittest.main()

diff --git a/Lib/test/test_named_expressions.py b/Lib/test/test_named_expressions.py
index e15111cf383984853a74381fedc5ee25d28af5a0..f73e6fee70ce0b595fae730ecdc4cabd05536fb3 100644 (file)
--- a/Lib/test/test_named_expressions.py
+++ b/Lib/test/test_named_expressions.py
@@ -212,6 +212,11 @@ class NamedExpressionAssignmentTest(unittest.TestCase):
 
         self.assertEqual(a, False)
 
+    def test_named_expression_assignment_16(self):
+        a, b = 1, 2
+        fib = {(c := a): (a := b) + (b := a + c) - b for __ in range(6)}
+        self.assertEqual(fib, {1: 2, 2: 3, 3: 5, 5: 8, 8: 13, 13: 21})
+
 
 class NamedExpressionScopeTest(unittest.TestCase):
 

diff --git a/Lib/test/test_parser.py b/Lib/test/test_parser.py
index b830459b190f4e4a2f78f2f4e089d1bd68e0cf08..e5285c6360229d7434460e99c85edc2d037bcf61 100644 (file)
--- a/Lib/test/test_parser.py
+++ b/Lib/test/test_parser.py
@@ -473,6 +473,8 @@ class RoundtripLegalSyntaxTestCase(unittest.TestCase):
         self.check_suite("foo(b := 2, a=1)")
         self.check_suite("foo((b := 2), a=1)")
         self.check_suite("foo(c=(b := 2), a=1)")
+        self.check_suite("{(x := C(i)).q: x for i in y}")
+
 
 #
 #  Second, we take *invalid* trees and make sure we get ParserError

diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-06-17-06-03-55.bpo-35224.FHWPGv.rst b/Misc/NEWS.d/next/Core and Builtins/2019-06-17-06-03-55.bpo-35224.FHWPGv.rst
new file mode 100644 (file)
index 0000000..5a1a79b
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2019-06-17-06-03-55.bpo-35224.FHWPGv.rst
@@ -0,0 +1,2 @@
+Reverse evaluation order of key: value in dict comprehensions as proposed in PEP 572.
+I.e. in ``{k: v for ...}``, ``k`` will be evaluated before ``v``.

diff --git a/Python/ceval.c b/Python/ceval.c
index 4ca986a94517f23cde0b3241d41cd122219f0956..5d29d41cf80c3a9dc77bcfd3a3b370b254941985 100644 (file)
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -2940,8 +2940,8 @@ main_loop:
         }
 
         case TARGET(MAP_ADD): {
-            PyObject *key = TOP();
-            PyObject *value = SECOND();
+            PyObject *value = TOP();
+            PyObject *key = SECOND();
             PyObject *map;
             int err;
             STACK_SHRINK(2);

diff --git a/Python/compile.c b/Python/compile.c
index 4d3ecfe5d6fc9d30e837cedb9c7830dd177a791f..7bdf406079d3f56c11868142e2471b8c6762173b 100644 (file)
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -4238,10 +4238,10 @@ compiler_sync_comprehension_generator(struct compiler *c,
             ADDOP_I(c, SET_ADD, gen_index + 1);
             break;
         case COMP_DICTCOMP:
-            /* With 'd[k] = v', v is evaluated before k, so we do
+            /* With '{k: v}', k is evaluated before v, so we do
                the same. */
-            VISIT(c, expr, val);
             VISIT(c, expr, elt);
+            VISIT(c, expr, val);
             ADDOP_I(c, MAP_ADD, gen_index + 1);
             break;
         default:
@@ -4327,10 +4327,10 @@ compiler_async_comprehension_generator(struct compiler *c,
             ADDOP_I(c, SET_ADD, gen_index + 1);
             break;
         case COMP_DICTCOMP:
-            /* With 'd[k] = v', v is evaluated before k, so we do
+            /* With '{k: v}', k is evaluated before v, so we do
                the same. */
-            VISIT(c, expr, val);
             VISIT(c, expr, elt);
+            VISIT(c, expr, val);
             ADDOP_I(c, MAP_ADD, gen_index + 1);
             break;
         default: