From 4a9c19444f289e86d457219c802916a9c71d9f7d Mon Sep 17 00:00:00 2001 From: sobolevn Date: Wed, 28 Dec 2022 15:05:02 +0300 Subject: [PATCH 1/6] gh-87447: Fix walrus comprehension rebind checking --- Lib/test/test_named_expressions.py | 84 ++++++++++++++++++- ...2-12-28-15-02-53.gh-issue-87447.7-aekA.rst | 2 + Python/symtable.c | 3 +- 3 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-12-28-15-02-53.gh-issue-87447.7-aekA.rst diff --git a/Lib/test/test_named_expressions.py b/Lib/test/test_named_expressions.py index 20ac2e699f0c35..f383f44aaa56a0 100644 --- a/Lib/test/test_named_expressions.py +++ b/Lib/test/test_named_expressions.py @@ -124,12 +124,53 @@ def test_named_expression_invalid_rebinding_list_comprehension_iteration_variabl ("Unreachable reuse", 'i', "[False or (i:=0) for i in range(5)]"), ("Unreachable nested reuse", 'i', "[(i, j) for i in range(5) for j in range(5) if True or (i:=10)]"), + # Regression tests from https://github.com/python/cpython/issues/87447 + ("Complex expression: a", "a", + "[(a := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), + ("Complex expression: b", "b", + "[(b := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), ] for case, target, code in cases: msg = f"assignment expression cannot rebind comprehension iteration variable '{target}'" with self.subTest(case=case): with self.assertRaisesRegex(SyntaxError, msg): - exec(code, {}, {}) + exec(code, {}) # Module scope + with self.assertRaisesRegex(SyntaxError, msg): + exec(code, {}, {}) # Class scope + with self.assertRaisesRegex(SyntaxError, msg): + exec(f"lambda: {code}", {}) # Function scope + + def test_named_expression_valid_rebinding_list_comprehension_iteration_variable(self): + cases = [ + # Regression tests from https://github.com/python/cpython/issues/87447 + ("Complex expression: c", + "[(c := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), + ("Complex expression: d", + "[(d := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), + ("Complex expression: e", + "[(e := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), + ("Complex expression: f", + "[(f := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), + ("Complex expression: g", + "[(g := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), + ("Complex expression: h", + "[(h := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), + ("Complex expression: i", + "[(i := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), + ("Complex expression: j", + "[(j := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), + ] + for case, code in cases: + with self.subTest(case=case): + # Names used in snippets are not defined, + # but we are fine with it: just must not be a SyntaxError. + # Names used in snippets are not defined, + # but we are fine with it: just must not be a SyntaxError. + with self.assertRaises(NameError): + exec(code, {}) # Module scope + with self.assertRaises(NameError): + exec(code, {}, {}) # Class scope + exec(f"lambda: {code}", {}) # Function scope def test_named_expression_invalid_rebinding_list_comprehension_inner_loop(self): cases = [ @@ -178,12 +219,51 @@ def test_named_expression_invalid_rebinding_set_comprehension_iteration_variable ("Unreachable reuse", 'i', "{False or (i:=0) for i in range(5)}"), ("Unreachable nested reuse", 'i', "{(i, j) for i in range(5) for j in range(5) if True or (i:=10)}"), + # Regression tests from https://github.com/python/cpython/issues/87447 + ("Complex expression: a", "a", + "{(a := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), + ("Complex expression: b", "b", + "{(b := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), ] for case, target, code in cases: msg = f"assignment expression cannot rebind comprehension iteration variable '{target}'" with self.subTest(case=case): with self.assertRaisesRegex(SyntaxError, msg): - exec(code, {}, {}) + exec(code, {}) # Module scope + with self.assertRaisesRegex(SyntaxError, msg): + exec(code, {}, {}) # Class scope + with self.assertRaisesRegex(SyntaxError, msg): + exec(f"lambda: {code}", {}) # Function scope + + def test_named_expression_valid_rebinding_set_comprehension_iteration_variable(self): + cases = [ + # Regression tests from https://github.com/python/cpython/issues/87447 + ("Complex expression: c", + "{(c := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), + ("Complex expression: d", + "{(d := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), + ("Complex expression: e", + "{(e := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), + ("Complex expression: f", + "{(f := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), + ("Complex expression: g", + "{(g := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), + ("Complex expression: h", + "{(h := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), + ("Complex expression: i", + "{(i := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), + ("Complex expression: j", + "{(j := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), + ] + for case, code in cases: + with self.subTest(case=case): + # Names used in snippets are not defined, + # but we are fine with it: just must not be a SyntaxError. + with self.assertRaises(NameError): + exec(code, {}) # Module scope + with self.assertRaises(NameError): + exec(code, {}, {}) # Class scope + exec(f"lambda: {code}", {}) # Function scope def test_named_expression_invalid_rebinding_set_comprehension_inner_loop(self): cases = [ diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-12-28-15-02-53.gh-issue-87447.7-aekA.rst b/Misc/NEWS.d/next/Core and Builtins/2022-12-28-15-02-53.gh-issue-87447.7-aekA.rst new file mode 100644 index 00000000000000..71cd5551e90c3c --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-12-28-15-02-53.gh-issue-87447.7-aekA.rst @@ -0,0 +1,2 @@ +Fix :exc:`SyntaxError` on comprehension rebind checking with names that are +not actually redefined. diff --git a/Python/symtable.c b/Python/symtable.c index fb2bb7d83835de..61651d03940b04 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -1488,7 +1488,8 @@ symtable_extend_namedexpr_scope(struct symtable *st, expr_ty e) */ if (ste->ste_comprehension) { long target_in_scope = _PyST_GetSymbol(ste, target_name); - if (target_in_scope & DEF_COMP_ITER) { + if ((target_in_scope & DEF_COMP_ITER) && + (target_in_scope & (DEF_LOCAL | DEF_GLOBAL))) { PyErr_Format(PyExc_SyntaxError, NAMED_EXPR_COMP_CONFLICT, target_name); PyErr_RangedSyntaxLocationObject(st->st_filename, e->lineno, From 148167879bbbde396c941167f2858da7ae5c12c5 Mon Sep 17 00:00:00 2001 From: Nikita Sobolev Date: Thu, 29 Dec 2022 09:29:56 +0300 Subject: [PATCH 2/6] Update Python/symtable.c --- Python/symtable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/symtable.c b/Python/symtable.c index 61651d03940b04..8efd1c00288f8b 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -1489,7 +1489,7 @@ symtable_extend_namedexpr_scope(struct symtable *st, expr_ty e) if (ste->ste_comprehension) { long target_in_scope = _PyST_GetSymbol(ste, target_name); if ((target_in_scope & DEF_COMP_ITER) && - (target_in_scope & (DEF_LOCAL | DEF_GLOBAL))) { + (target_in_scope & DEF_LOCAL)) { PyErr_Format(PyExc_SyntaxError, NAMED_EXPR_COMP_CONFLICT, target_name); PyErr_RangedSyntaxLocationObject(st->st_filename, e->lineno, From 8e167b0953c7ef9e7d801db978e75dcce59ff8dd Mon Sep 17 00:00:00 2001 From: sobolevn Date: Wed, 4 Jan 2023 12:53:20 +0300 Subject: [PATCH 3/6] Address review --- Doc/whatsnew/3.12.rst | 9 ++ Lib/test/test_named_expressions.py | 130 +++++++++--------- ...2-12-28-15-02-53.gh-issue-87447.7-aekA.rst | 3 + 3 files changed, 75 insertions(+), 67 deletions(-) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 69f97debd69408..748a5d1e209afc 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -182,6 +182,15 @@ Other Language Changes arguments of any type instead of just :class:`bool` and :class:`int`. (Contributed by Serhiy Storchaka in :gh:`60203`.) +* It is now allowed to reassing (with ``:=``) variables mentioned + in iterable part of comprehensions. + For example, in ``[(b := 1) for a, b.prop in some_iter]`` + it is now possible to reassign variable ``b``. + However, reassing variables directly defined + in iterable part of comprehensions (like ``a``) + is still not allowed due to :pep:`572`. + (Contributed by Nikita Sobolev in :gh:`100581`.) + New Modules =========== diff --git a/Lib/test/test_named_expressions.py b/Lib/test/test_named_expressions.py index f383f44aaa56a0..7b2fa844827ae9 100644 --- a/Lib/test/test_named_expressions.py +++ b/Lib/test/test_named_expressions.py @@ -114,6 +114,69 @@ def test_named_expression_invalid_in_class_body(self): "assignment expression within a comprehension cannot be used in a class body"): exec(code, {}, {}) + def test_named_expression_valid_rebinding_iteration_variable(self): + # This test covers that we can reassign variables + # that are not directly assigned in the + # iterable part of a comprehension. + cases = [ + # Regression tests from https://github.com/python/cpython/issues/87447 + ("Complex expression: c", + "{0}(c := 1) for a, (*b, c[d+e::f(g)], h.i) in j{1}"), + ("Complex expression: d", + "{0}(d := 1) for a, (*b, c[d+e::f(g)], h.i) in j{1}"), + ("Complex expression: e", + "{0}(e := 1) for a, (*b, c[d+e::f(g)], h.i) in j{1}"), + ("Complex expression: f", + "{0}(f := 1) for a, (*b, c[d+e::f(g)], h.i) in j{1}"), + ("Complex expression: g", + "{0}(g := 1) for a, (*b, c[d+e::f(g)], h.i) in j{1}"), + ("Complex expression: h", + "{0}(h := 1) for a, (*b, c[d+e::f(g)], h.i) in j{1}"), + ("Complex expression: i", + "{0}(i := 1) for a, (*b, c[d+e::f(g)], h.i) in j{1}"), + ("Complex expression: j", + "{0}(j := 1) for a, (*b, c[d+e::f(g)], h.i) in j{1}"), + ] + for test_case, code in cases: + for lpar, rpar in [('(', ')'), ('[', ']'), ('{', '}')]: + code = code.format(lpar, rpar) + with self.subTest(case=test_case, lpar=lpar, rpar=rpar): + # Names used in snippets are not defined, + # but we are fine with it: just must not be a SyntaxError. + # Names used in snippets are not defined, + # but we are fine with it: just must not be a SyntaxError. + with self.assertRaises(NameError): + exec(code, {}) # Module scope + with self.assertRaises(NameError): + exec(code, {}, {}) # Class scope + exec(f"lambda: {code}", {}) # Function scope + + def test_named_expression_invalid_rebinding_iteration_variable(self): + # This test covers that we cannot reassign variables + # that are directly assigned in the iterable part of a comprehension. + cases = [ + # Regression tests from https://github.com/python/cpython/issues/87447 + ("Complex expression: a", "a", + "{0}(a := 1) for a, (*b, c[d+e::f(g)], h.i) in j{1}"), + ("Complex expression: b", "b", + "{0}(b := 1) for a, (*b, c[d+e::f(g)], h.i) in j{1}"), + ] + for test_case, target, code in cases: + msg = f"assignment expression cannot rebind comprehension iteration variable '{target}'" + for lpar, rpar in [('(', ')'), ('[', ']'), ('{', '}')]: + code = code.format(lpar, rpar) + with self.subTest(case=test_case, lpar=lpar, rpar=rpar): + # Names used in snippets are not defined, + # but we are fine with it: just must not be a SyntaxError. + # Names used in snippets are not defined, + # but we are fine with it: just must not be a SyntaxError. + with self.assertRaisesRegex(SyntaxError, msg): + exec(code, {}) # Module scope + with self.assertRaisesRegex(SyntaxError, msg): + exec(code, {}, {}) # Class scope + with self.assertRaisesRegex(SyntaxError, msg): + exec(f"lambda: {code}", {}) # Function scope + def test_named_expression_invalid_rebinding_list_comprehension_iteration_variable(self): cases = [ ("Local reuse", 'i', "[i := 0 for i in range(5)]"), @@ -124,11 +187,6 @@ def test_named_expression_invalid_rebinding_list_comprehension_iteration_variabl ("Unreachable reuse", 'i', "[False or (i:=0) for i in range(5)]"), ("Unreachable nested reuse", 'i', "[(i, j) for i in range(5) for j in range(5) if True or (i:=10)]"), - # Regression tests from https://github.com/python/cpython/issues/87447 - ("Complex expression: a", "a", - "[(a := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), - ("Complex expression: b", "b", - "[(b := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), ] for case, target, code in cases: msg = f"assignment expression cannot rebind comprehension iteration variable '{target}'" @@ -140,38 +198,6 @@ def test_named_expression_invalid_rebinding_list_comprehension_iteration_variabl with self.assertRaisesRegex(SyntaxError, msg): exec(f"lambda: {code}", {}) # Function scope - def test_named_expression_valid_rebinding_list_comprehension_iteration_variable(self): - cases = [ - # Regression tests from https://github.com/python/cpython/issues/87447 - ("Complex expression: c", - "[(c := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), - ("Complex expression: d", - "[(d := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), - ("Complex expression: e", - "[(e := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), - ("Complex expression: f", - "[(f := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), - ("Complex expression: g", - "[(g := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), - ("Complex expression: h", - "[(h := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), - ("Complex expression: i", - "[(i := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), - ("Complex expression: j", - "[(j := 1) for a, (*b, c[d+e::f(g)], h.i) in j]"), - ] - for case, code in cases: - with self.subTest(case=case): - # Names used in snippets are not defined, - # but we are fine with it: just must not be a SyntaxError. - # Names used in snippets are not defined, - # but we are fine with it: just must not be a SyntaxError. - with self.assertRaises(NameError): - exec(code, {}) # Module scope - with self.assertRaises(NameError): - exec(code, {}, {}) # Class scope - exec(f"lambda: {code}", {}) # Function scope - def test_named_expression_invalid_rebinding_list_comprehension_inner_loop(self): cases = [ ("Inner reuse", 'j', "[i for i in range(5) if (j := 0) for j in range(5)]"), @@ -235,36 +261,6 @@ def test_named_expression_invalid_rebinding_set_comprehension_iteration_variable with self.assertRaisesRegex(SyntaxError, msg): exec(f"lambda: {code}", {}) # Function scope - def test_named_expression_valid_rebinding_set_comprehension_iteration_variable(self): - cases = [ - # Regression tests from https://github.com/python/cpython/issues/87447 - ("Complex expression: c", - "{(c := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), - ("Complex expression: d", - "{(d := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), - ("Complex expression: e", - "{(e := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), - ("Complex expression: f", - "{(f := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), - ("Complex expression: g", - "{(g := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), - ("Complex expression: h", - "{(h := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), - ("Complex expression: i", - "{(i := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), - ("Complex expression: j", - "{(j := 1) for a, (*b, c[d+e::f(g)], h.i) in j}"), - ] - for case, code in cases: - with self.subTest(case=case): - # Names used in snippets are not defined, - # but we are fine with it: just must not be a SyntaxError. - with self.assertRaises(NameError): - exec(code, {}) # Module scope - with self.assertRaises(NameError): - exec(code, {}, {}) # Class scope - exec(f"lambda: {code}", {}) # Function scope - def test_named_expression_invalid_rebinding_set_comprehension_inner_loop(self): cases = [ ("Inner reuse", 'j', "{i for i in range(5) if (j := 0) for j in range(5)}"), diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-12-28-15-02-53.gh-issue-87447.7-aekA.rst b/Misc/NEWS.d/next/Core and Builtins/2022-12-28-15-02-53.gh-issue-87447.7-aekA.rst index 71cd5551e90c3c..47209bf5f91684 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2022-12-28-15-02-53.gh-issue-87447.7-aekA.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2022-12-28-15-02-53.gh-issue-87447.7-aekA.rst @@ -1,2 +1,5 @@ Fix :exc:`SyntaxError` on comprehension rebind checking with names that are not actually redefined. + +Now reassing ``b`` in ``[(b := 1) for a, b.prop in some_iter]`` is allowed. +But, reassing ``a`` is still not allowed due to :pep:`572`. From 84a413aee4947ca9cf2fb9fda53a09cd0bb5b028 Mon Sep 17 00:00:00 2001 From: Nikita Sobolev Date: Thu, 5 Jan 2023 11:37:56 +0300 Subject: [PATCH 4/6] Update Doc/whatsnew/3.12.rst Co-authored-by: Pablo Galindo Salgado --- Doc/whatsnew/3.12.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 748a5d1e209afc..fc4bca2c74dfa0 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -182,12 +182,12 @@ Other Language Changes arguments of any type instead of just :class:`bool` and :class:`int`. (Contributed by Serhiy Storchaka in :gh:`60203`.) -* It is now allowed to reassing (with ``:=``) variables mentioned +* It is now allowed to reassign (with ``:=``) variables mentioned in iterable part of comprehensions. For example, in ``[(b := 1) for a, b.prop in some_iter]`` it is now possible to reassign variable ``b``. - However, reassing variables directly defined - in iterable part of comprehensions (like ``a``) + However, reassigning variables directly defined + in iterable parts of comprehensions (like ``a``) is still not allowed due to :pep:`572`. (Contributed by Nikita Sobolev in :gh:`100581`.) From da61b6b0d6a6300f49991449edc7a1ab43167646 Mon Sep 17 00:00:00 2001 From: Shantanu <12621235+hauntsaninja@users.noreply.github.com> Date: Thu, 5 Jan 2023 13:46:12 -0800 Subject: [PATCH 5/6] Update Misc/NEWS.d/next/Core and Builtins/2022-12-28-15-02-53.gh-issue-87447.7-aekA.rst --- .../2022-12-28-15-02-53.gh-issue-87447.7-aekA.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-12-28-15-02-53.gh-issue-87447.7-aekA.rst b/Misc/NEWS.d/next/Core and Builtins/2022-12-28-15-02-53.gh-issue-87447.7-aekA.rst index 47209bf5f91684..af60acf1103aea 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2022-12-28-15-02-53.gh-issue-87447.7-aekA.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2022-12-28-15-02-53.gh-issue-87447.7-aekA.rst @@ -1,5 +1,5 @@ Fix :exc:`SyntaxError` on comprehension rebind checking with names that are not actually redefined. -Now reassing ``b`` in ``[(b := 1) for a, b.prop in some_iter]`` is allowed. -But, reassing ``a`` is still not allowed due to :pep:`572`. +Now reassigning ``b`` in ``[(b := 1) for a, b.prop in some_iter]`` is allowed. +Reassigning ``a`` is still disallowed as per :pep:`572`. From e941439a2f7a70a9cf54aa2a317604b55b73895a Mon Sep 17 00:00:00 2001 From: Nikita Sobolev Date: Fri, 6 Jan 2023 00:54:53 +0300 Subject: [PATCH 6/6] Update Doc/whatsnew/3.12.rst Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com> --- Doc/whatsnew/3.12.rst | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index d268fedc84bfa4..df1bac79ba7a7c 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -182,13 +182,11 @@ Other Language Changes arguments of any type instead of just :class:`bool` and :class:`int`. (Contributed by Serhiy Storchaka in :gh:`60203`.) -* It is now allowed to reassign (with ``:=``) variables mentioned - in iterable part of comprehensions. - For example, in ``[(b := 1) for a, b.prop in some_iter]`` - it is now possible to reassign variable ``b``. - However, reassigning variables directly defined - in iterable parts of comprehensions (like ``a``) - is still not allowed due to :pep:`572`. +* Variables used in the target part of comprehensions that are not stored to + can now be used in assignment expressions (``:=``). + For example, in ``[(b := 1) for a, b.prop in some_iter]``, the assignment to + ``b`` is now allowed. Note that assigning to variables stored to in the target + part of comprehensions (like ``a``) is still disallowed, as per :pep:`572`. (Contributed by Nikita Sobolev in :gh:`100581`.)