diff --git a/Doc/c-api/exceptions.rst b/Doc/c-api/exceptions.rst index d8173935596..25bb657dca0 100644 --- a/Doc/c-api/exceptions.rst +++ b/Doc/c-api/exceptions.rst @@ -809,7 +809,6 @@ the variables: single: PyExc_SystemError single: PyExc_SystemExit single: PyExc_TabError - single: PyExc_TargetScopeError single: PyExc_TimeoutError single: PyExc_TypeError single: PyExc_UnboundLocalError @@ -911,8 +910,6 @@ the variables: +-----------------------------------------+---------------------------------+----------+ | :c:data:`PyExc_TabError` | :exc:`TabError` | | +-----------------------------------------+---------------------------------+----------+ -| :c:data:`PyExc_TargetScopeError` | :exc:`TargetScopeError` | | -+-----------------------------------------+---------------------------------+----------+ | :c:data:`PyExc_TimeoutError` | :exc:`TimeoutError` | | +-----------------------------------------+---------------------------------+----------+ | :c:data:`PyExc_TypeError` | :exc:`TypeError` | | diff --git a/Include/pyerrors.h b/Include/pyerrors.h index 94af3cb3420..5125a51ec1a 100644 --- a/Include/pyerrors.h +++ b/Include/pyerrors.h @@ -97,7 +97,6 @@ PyAPI_DATA(PyObject *) PyExc_NotImplementedError; PyAPI_DATA(PyObject *) PyExc_SyntaxError; PyAPI_DATA(PyObject *) PyExc_IndentationError; PyAPI_DATA(PyObject *) PyExc_TabError; -PyAPI_DATA(PyObject *) PyExc_TargetScopeError; PyAPI_DATA(PyObject *) PyExc_ReferenceError; PyAPI_DATA(PyObject *) PyExc_SystemError; PyAPI_DATA(PyObject *) PyExc_SystemExit; diff --git a/Include/symtable.h b/Include/symtable.h index 9392e643877..5dcfa7e2c2b 100644 --- a/Include/symtable.h +++ b/Include/symtable.h @@ -58,6 +58,8 @@ typedef struct _symtable_entry { unsigned ste_needs_class_closure : 1; /* for class scopes, true if a closure over __class__ should be created */ + unsigned ste_comp_iter_target : 1; /* true if visiting comprehension target */ + int ste_comp_iter_expr; /* non-zero if visiting a comprehension range expression */ int ste_lineno; /* first line of block */ int ste_col_offset; /* offset of first line of block */ int ste_opt_lineno; /* lineno of last exec or import * */ @@ -94,6 +96,7 @@ PyAPI_FUNC(void) PySymtable_Free(struct symtable *); #define DEF_FREE_CLASS 2<<5 /* free variable from class's method */ #define DEF_IMPORT 2<<6 /* assignment occurred via import */ #define DEF_ANNOT 2<<7 /* this name is annotated */ +#define DEF_COMP_ITER 2<<8 /* this name is a comprehension iteration variable */ #define DEF_BOUND (DEF_LOCAL | DEF_PARAM | DEF_IMPORT) diff --git a/Lib/_compat_pickle.py b/Lib/_compat_pickle.py index 8bb1cf80afa..f68496ae639 100644 --- a/Lib/_compat_pickle.py +++ b/Lib/_compat_pickle.py @@ -128,7 +128,6 @@ PYTHON2_EXCEPTIONS = ( "SystemError", "SystemExit", "TabError", - "TargetScopeError", "TypeError", "UnboundLocalError", "UnicodeDecodeError", diff --git a/Lib/test/exception_hierarchy.txt b/Lib/test/exception_hierarchy.txt index 15f4491cf23..763a6c899b4 100644 --- a/Lib/test/exception_hierarchy.txt +++ b/Lib/test/exception_hierarchy.txt @@ -42,7 +42,6 @@ BaseException | +-- NotImplementedError | +-- RecursionError +-- SyntaxError - | +-- TargetScopeError | +-- IndentationError | +-- TabError +-- SystemError diff --git a/Lib/test/test_named_expressions.py b/Lib/test/test_named_expressions.py index f73e6fee70c..b1027ce7800 100644 --- a/Lib/test/test_named_expressions.py +++ b/Lib/test/test_named_expressions.py @@ -104,15 +104,69 @@ class NamedExpressionInvalidTest(unittest.TestCase): with self.assertRaisesRegex(SyntaxError, "invalid syntax"): exec(code, {}, {}) - def test_named_expression_invalid_18(self): + def test_named_expression_invalid_in_class_body(self): code = """class Foo(): [(42, 1 + ((( j := i )))) for i in range(5)] """ - with self.assertRaisesRegex(TargetScopeError, - "named expression within a comprehension cannot be used in a class body"): + with self.assertRaisesRegex(SyntaxError, + "assignment expression within a comprehension cannot be used in a class body"): exec(code, {}, {}) + def test_named_expression_invalid_rebinding_comprehension_iteration_variable(self): + cases = [ + ("Local reuse", 'i', "[i := 0 for i in range(5)]"), + ("Nested reuse", 'j', "[[(j := 0) for i in range(5)] for j in range(5)]"), + ("Reuse inner loop target", 'j', "[(j := 0) for i in range(5) for j in range(5)]"), + ("Unpacking reuse", 'i', "[i := 0 for i, j in [(0, 1)]]"), + ("Reuse in loop condition", 'i', "[i+1 for i in range(5) if (i := 0)]"), + ("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)]"), + ] + 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, {}, {}) + + def test_named_expression_invalid_rebinding_comprehension_inner_loop(self): + cases = [ + ("Inner reuse", 'j', "[i for i in range(5) if (j := 0) for j in range(5)]"), + ("Inner unpacking reuse", 'j', "[i for i in range(5) if (j := 0) for j, k in [(0, 1)]]"), + ] + for case, target, code in cases: + msg = f"comprehension inner loop cannot rebind assignment expression target '{target}'" + with self.subTest(case=case): + 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_comprehension_iterable_expression(self): + cases = [ + ("Top level", "[i for i in (i := range(5))]"), + ("Inside tuple", "[i for i in (2, 3, i := range(5))]"), + ("Inside list", "[i for i in [2, 3, i := range(5)]]"), + ("Different name", "[i for i in (j := range(5))]"), + ("Lambda expression", "[i for i in (lambda:(j := range(5)))()]"), + ("Inner loop", "[i for i in range(5) for j in (i := range(5))]"), + ("Nested comprehension", "[i for i in [j for j in (k := range(5))]]"), + ("Nested comprehension condition", "[i for i in [j for j in range(5) if (j := True)]]"), + ("Nested comprehension body", "[i for i in [(j := True) for j in range(5)]]"), + ] + msg = "assignment expression cannot be used in a comprehension iterable expression" + for case, code in cases: + with self.subTest(case=case): + 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 + class NamedExpressionAssignmentTest(unittest.TestCase): @@ -306,39 +360,6 @@ print(a)""" self.assertEqual(res, [0, 1, 2, 3, 4]) self.assertEqual(j, 4) - def test_named_expression_scope_12(self): - res = [i := i for i in range(5)] - - self.assertEqual(res, [0, 1, 2, 3, 4]) - self.assertEqual(i, 4) - - def test_named_expression_scope_13(self): - res = [i := 0 for i, j in [(1, 2), (3, 4)]] - - self.assertEqual(res, [0, 0]) - self.assertEqual(i, 0) - - def test_named_expression_scope_14(self): - res = [(i := 0, j := 1) for i, j in [(1, 2), (3, 4)]] - - self.assertEqual(res, [(0, 1), (0, 1)]) - self.assertEqual(i, 0) - self.assertEqual(j, 1) - - def test_named_expression_scope_15(self): - res = [(i := i, j := j) for i, j in [(1, 2), (3, 4)]] - - self.assertEqual(res, [(1, 2), (3, 4)]) - self.assertEqual(i, 3) - self.assertEqual(j, 4) - - def test_named_expression_scope_16(self): - res = [(i := j, j := i) for i, j in [(1, 2), (3, 4)]] - - self.assertEqual(res, [(2, 2), (4, 4)]) - self.assertEqual(i, 4) - self.assertEqual(j, 4) - def test_named_expression_scope_17(self): b = 0 res = [b := i + b for i in range(5)] @@ -421,6 +442,33 @@ spam()""" self.assertEqual(ns["a"], 20) + def test_named_expression_variable_reuse_in_comprehensions(self): + # The compiler is expected to raise syntax error for comprehension + # iteration variables, but should be fine with rebinding of other + # names (e.g. globals, nonlocals, other assignment expressions) + + # The cases are all defined to produce the same expected result + # Each comprehension is checked at both function scope and module scope + rebinding = "[x := i for i in range(3) if (x := i) or not x]" + filter_ref = "[x := i for i in range(3) if x or not x]" + body_ref = "[x for i in range(3) if (x := i) or not x]" + nested_ref = "[j for i in range(3) if x or not x for j in range(3) if (x := i)][:-3]" + cases = [ + ("Rebind global", f"x = 1; result = {rebinding}"), + ("Rebind nonlocal", f"result, x = (lambda x=1: ({rebinding}, x))()"), + ("Filter global", f"x = 1; result = {filter_ref}"), + ("Filter nonlocal", f"result, x = (lambda x=1: ({filter_ref}, x))()"), + ("Body global", f"x = 1; result = {body_ref}"), + ("Body nonlocal", f"result, x = (lambda x=1: ({body_ref}, x))()"), + ("Nested global", f"x = 1; result = {nested_ref}"), + ("Nested nonlocal", f"result, x = (lambda x=1: ({nested_ref}, x))()"), + ] + for case, code in cases: + with self.subTest(case=case): + ns = {} + exec(code, ns) + self.assertEqual(ns["x"], 2) + self.assertEqual(ns["result"], [0, 1, 2]) if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-08-05-14-22-59.bpo-37757.lRv5HX.rst b/Misc/NEWS.d/next/Core and Builtins/2019-08-05-14-22-59.bpo-37757.lRv5HX.rst new file mode 100644 index 00000000000..258df0dc09b --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-08-05-14-22-59.bpo-37757.lRv5HX.rst @@ -0,0 +1,7 @@ +:pep:`572`: As described in the PEP, assignment expressions now raise +:exc:`SyntaxError` when their interaction with comprehension scoping results +in an ambiguous target scope. + +The ``TargetScopeError`` subclass originally proposed by the PEP has been +removed in favour of just raising regular syntax errors for the disallowed +cases. diff --git a/Objects/exceptions.c b/Objects/exceptions.c index ef9dd512dc9..631f5375f73 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -1521,13 +1521,6 @@ MiddlingExtendsException(PyExc_SyntaxError, IndentationError, SyntaxError, "Improper indentation."); -/* - * TargetScopeError extends SyntaxError - */ -MiddlingExtendsException(PyExc_SyntaxError, TargetScopeError, SyntaxError, - "Improper scope target."); - - /* * TabError extends IndentationError */ @@ -2539,7 +2532,6 @@ _PyExc_Init(void) PRE_INIT(AttributeError); PRE_INIT(SyntaxError); PRE_INIT(IndentationError); - PRE_INIT(TargetScopeError); PRE_INIT(TabError); PRE_INIT(LookupError); PRE_INIT(IndexError); @@ -2680,7 +2672,6 @@ _PyBuiltins_AddExceptions(PyObject *bltinmod) POST_INIT(AttributeError); POST_INIT(SyntaxError); POST_INIT(IndentationError); - POST_INIT(TargetScopeError); POST_INIT(TabError); POST_INIT(LookupError); POST_INIT(IndexError); diff --git a/PC/python3.def b/PC/python3.def index 6844cf1f3b8..1f355bffc9b 100644 --- a/PC/python3.def +++ b/PC/python3.def @@ -235,7 +235,6 @@ EXPORTS PyExc_SystemError=python39.PyExc_SystemError DATA PyExc_SystemExit=python39.PyExc_SystemExit DATA PyExc_TabError=python39.PyExc_TabError DATA - PyExc_TargetScopeError=python39.PyExc_TargetScopeError DATA PyExc_TimeoutError=python39.PyExc_TimeoutError DATA PyExc_TypeError=python39.PyExc_TypeError DATA PyExc_UnboundLocalError=python39.PyExc_UnboundLocalError DATA diff --git a/Python/symtable.c b/Python/symtable.c index 668cc21b2df..e48baa808d4 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -32,7 +32,16 @@ #define IMPORT_STAR_WARNING "import * only allowed at module level" #define NAMED_EXPR_COMP_IN_CLASS \ -"named expression within a comprehension cannot be used in a class body" +"assignment expression within a comprehension cannot be used in a class body" + +#define NAMED_EXPR_COMP_CONFLICT \ +"assignment expression cannot rebind comprehension iteration variable '%U'" + +#define NAMED_EXPR_COMP_INNER_LOOP_CONFLICT \ +"comprehension inner loop cannot rebind assignment expression target '%U'" + +#define NAMED_EXPR_COMP_ITER_EXPR \ +"assignment expression cannot be used in a comprehension iterable expression" static PySTEntryObject * ste_new(struct symtable *st, identifier name, _Py_block_ty block, @@ -81,6 +90,8 @@ ste_new(struct symtable *st, identifier name, _Py_block_ty block, ste->ste_comprehension = 0; ste->ste_returns_value = 0; ste->ste_needs_class_closure = 0; + ste->ste_comp_iter_target = 0; + ste->ste_comp_iter_expr = 0; ste->ste_symbols = PyDict_New(); ste->ste_varnames = PyList_New(0); @@ -373,14 +384,21 @@ PySymtable_Lookup(struct symtable *st, void *key) return (PySTEntryObject *)v; } -int -PyST_GetScope(PySTEntryObject *ste, PyObject *name) +static long +_PyST_GetSymbol(PySTEntryObject *ste, PyObject *name) { PyObject *v = PyDict_GetItem(ste->ste_symbols, name); if (!v) return 0; assert(PyLong_Check(v)); - return (PyLong_AS_LONG(v) >> SCOPE_OFFSET) & SCOPE_MASK; + return PyLong_AS_LONG(v); +} + +int +PyST_GetScope(PySTEntryObject *ste, PyObject *name) +{ + long symbol = _PyST_GetSymbol(ste, name); + return (symbol >> SCOPE_OFFSET) & SCOPE_MASK; } static int @@ -955,6 +973,13 @@ symtable_enter_block(struct symtable *st, identifier name, _Py_block_ty block, return 0; } prev = st->st_cur; + /* bpo-37757: For now, disallow *all* assignment expressions in the + * outermost iterator expression of a comprehension, even those inside + * a nested comprehension or a lambda expression. + */ + if (prev) { + ste->ste_comp_iter_expr = prev->ste_comp_iter_expr; + } /* The entry is owned by the stack. Borrow it for st_cur. */ Py_DECREF(ste); st->st_cur = ste; @@ -971,15 +996,10 @@ symtable_enter_block(struct symtable *st, identifier name, _Py_block_ty block, static long symtable_lookup(struct symtable *st, PyObject *name) { - PyObject *o; PyObject *mangled = _Py_Mangle(st->st_private, name); if (!mangled) return 0; - o = PyDict_GetItem(st->st_cur->ste_symbols, mangled); - Py_DECREF(mangled); - if (!o) - return 0; - return PyLong_AsLong(o); + return _PyST_GetSymbol(st->st_cur, mangled); } static int @@ -1012,6 +1032,22 @@ symtable_add_def_helper(struct symtable *st, PyObject *name, int flag, struct _s else { val = flag; } + if (ste->ste_comp_iter_target) { + /* This name is an iteration variable in a comprehension, + * so check for a binding conflict with any named expressions. + * Otherwise, mark it as an iteration variable so subsequent + * named expressions can check for conflicts. + */ + if (val & (DEF_GLOBAL | DEF_NONLOCAL)) { + PyErr_Format(PyExc_SyntaxError, + NAMED_EXPR_COMP_INNER_LOOP_CONFLICT, name); + PyErr_SyntaxLocationObject(st->st_filename, + ste->ste_lineno, + ste->ste_col_offset + 1); + goto error; + } + val |= DEF_COMP_ITER; + } o = PyLong_FromLong(val); if (o == NULL) goto error; @@ -1392,7 +1428,9 @@ static int symtable_extend_namedexpr_scope(struct symtable *st, expr_ty e) { assert(st->st_stack); + assert(e->kind == Name_kind); + PyObject *target_name = e->v.Name.id; Py_ssize_t i, size; struct _symtable_entry *ste; size = PyList_GET_SIZE(st->st_stack); @@ -1402,32 +1440,42 @@ symtable_extend_namedexpr_scope(struct symtable *st, expr_ty e) for (i = size - 1; i >= 0; i--) { ste = (struct _symtable_entry *) PyList_GET_ITEM(st->st_stack, i); - /* If our current entry is a comprehension, skip it */ + /* If we find a comprehension scope, check for a target + * binding conflict with iteration variables, otherwise skip it + */ if (ste->ste_comprehension) { + long target_in_scope = _PyST_GetSymbol(ste, target_name); + if (target_in_scope & DEF_COMP_ITER) { + PyErr_Format(PyExc_SyntaxError, NAMED_EXPR_COMP_CONFLICT, target_name); + PyErr_SyntaxLocationObject(st->st_filename, + e->lineno, + e->col_offset); + VISIT_QUIT(st, 0); + } continue; } /* If we find a FunctionBlock entry, add as NONLOCAL/LOCAL */ if (ste->ste_type == FunctionBlock) { - if (!symtable_add_def(st, e->v.Name.id, DEF_NONLOCAL)) + if (!symtable_add_def(st, target_name, DEF_NONLOCAL)) VISIT_QUIT(st, 0); - if (!symtable_record_directive(st, e->v.Name.id, e->lineno, e->col_offset)) + if (!symtable_record_directive(st, target_name, e->lineno, e->col_offset)) VISIT_QUIT(st, 0); - return symtable_add_def_helper(st, e->v.Name.id, DEF_LOCAL, ste); + return symtable_add_def_helper(st, target_name, DEF_LOCAL, ste); } /* If we find a ModuleBlock entry, add as GLOBAL */ if (ste->ste_type == ModuleBlock) { - if (!symtable_add_def(st, e->v.Name.id, DEF_GLOBAL)) + if (!symtable_add_def(st, target_name, DEF_GLOBAL)) VISIT_QUIT(st, 0); - if (!symtable_record_directive(st, e->v.Name.id, e->lineno, e->col_offset)) + if (!symtable_record_directive(st, target_name, e->lineno, e->col_offset)) VISIT_QUIT(st, 0); - return symtable_add_def_helper(st, e->v.Name.id, DEF_GLOBAL, ste); + return symtable_add_def_helper(st, target_name, DEF_GLOBAL, ste); } /* Disallow usage in ClassBlock */ if (ste->ste_type == ClassBlock) { - PyErr_Format(PyExc_TargetScopeError, NAMED_EXPR_COMP_IN_CLASS, e->v.Name.id); + PyErr_Format(PyExc_SyntaxError, NAMED_EXPR_COMP_IN_CLASS); PyErr_SyntaxLocationObject(st->st_filename, e->lineno, e->col_offset); @@ -1442,6 +1490,26 @@ symtable_extend_namedexpr_scope(struct symtable *st, expr_ty e) return 0; } +static int +symtable_handle_namedexpr(struct symtable *st, expr_ty e) +{ + if (st->st_cur->ste_comp_iter_expr > 0) { + /* Assignment isn't allowed in a comprehension iterable expression */ + PyErr_Format(PyExc_SyntaxError, NAMED_EXPR_COMP_ITER_EXPR); + PyErr_SyntaxLocationObject(st->st_filename, + e->lineno, + e->col_offset); + VISIT_QUIT(st, 0); + } + if (st->st_cur->ste_comprehension) { + /* Inside a comprehension body, so find the right target scope */ + if (!symtable_extend_namedexpr_scope(st, e->v.NamedExpr.target)) + VISIT_QUIT(st, 0); + } + VISIT(st, expr, e->v.NamedExpr.value); + VISIT(st, expr, e->v.NamedExpr.target); +} + static int symtable_visit_expr(struct symtable *st, expr_ty e) { @@ -1452,12 +1520,7 @@ symtable_visit_expr(struct symtable *st, expr_ty e) } switch (e->kind) { case NamedExpr_kind: - if (st->st_cur->ste_comprehension) { - if (!symtable_extend_namedexpr_scope(st, e->v.NamedExpr.target)) - VISIT_QUIT(st, 0); - } - VISIT(st, expr, e->v.NamedExpr.value); - VISIT(st, expr, e->v.NamedExpr.target); + symtable_handle_namedexpr(st, e); break; case BoolOp_kind: VISIT_SEQ(st, expr, e->v.BoolOp.values); @@ -1739,8 +1802,12 @@ symtable_visit_alias(struct symtable *st, alias_ty a) static int symtable_visit_comprehension(struct symtable *st, comprehension_ty lc) { + st->st_cur->ste_comp_iter_target = 1; VISIT(st, expr, lc->target); + st->st_cur->ste_comp_iter_target = 0; + st->st_cur->ste_comp_iter_expr++; VISIT(st, expr, lc->iter); + st->st_cur->ste_comp_iter_expr--; VISIT_SEQ(st, expr, lc->ifs); if (lc->is_async) { st->st_cur->ste_coroutine = 1; @@ -1788,7 +1855,9 @@ symtable_handle_comprehension(struct symtable *st, expr_ty e, comprehension_ty outermost = ((comprehension_ty) asdl_seq_GET(generators, 0)); /* Outermost iterator is evaluated in current scope */ + st->st_cur->ste_comp_iter_expr++; VISIT(st, expr, outermost->iter); + st->st_cur->ste_comp_iter_expr--; /* Create comprehension scope for the rest */ if (!scope_name || !symtable_enter_block(st, scope_name, FunctionBlock, (void *)e, @@ -1805,7 +1874,11 @@ symtable_handle_comprehension(struct symtable *st, expr_ty e, symtable_exit_block(st, (void *)e); return 0; } + /* Visit iteration variable target, and mark them as such */ + st->st_cur->ste_comp_iter_target = 1; VISIT(st, expr, outermost->target); + st->st_cur->ste_comp_iter_target = 0; + /* Visit the rest of the comprehension body */ VISIT_SEQ(st, expr, outermost->ifs); VISIT_SEQ_TAIL(st, comprehension, generators, 1); if (value)