Skip to content

gh-87447: Fix walrus comprehension rebind checking #100581

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jan 8, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Doc/whatsnew/3.12.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
===========
Expand Down
80 changes: 78 additions & 2 deletions Lib/test/test_named_expressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)]"),
Expand All @@ -129,7 +192,11 @@ def test_named_expression_invalid_rebinding_list_comprehension_iteration_variabl
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_invalid_rebinding_list_comprehension_inner_loop(self):
cases = [
Expand Down Expand Up @@ -178,12 +245,21 @@ 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_invalid_rebinding_set_comprehension_inner_loop(self):
cases = [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +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`.
3 changes: 2 additions & 1 deletion Python/symtable.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
PyErr_Format(PyExc_SyntaxError, NAMED_EXPR_COMP_CONFLICT, target_name);
PyErr_RangedSyntaxLocationObject(st->st_filename,
e->lineno,
Expand Down