diff options
author | Carl Meyer <carl@oddbird.net> | 2024-05-03 14:05:19 (GMT) |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-05-03 14:05:19 (GMT) |
commit | c8deb1e4b495bf97ab00c710dfd63f227e1fb645 (patch) | |
tree | 9ee53d752cc465a45c6df665e65ff08c32a253cc | |
parent | 37ccf167869d101c4021c435868b7f89ccda8148 (diff) | |
download | cpython-c8deb1e4b495bf97ab00c710dfd63f227e1fb645.zip cpython-c8deb1e4b495bf97ab00c710dfd63f227e1fb645.tar.gz cpython-c8deb1e4b495bf97ab00c710dfd63f227e1fb645.tar.bz2 |
gh-118513: Fix sibling comprehensions with a name bound in one and global in the other (#118526)
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
-rw-r--r-- | Lib/test/test_listcomps.py | 14 | ||||
-rw-r--r-- | Misc/NEWS.d/next/Core and Builtins/2024-05-02-21-19-35.gh-issue-118513.qHODjb.rst | 1 | ||||
-rw-r--r-- | Python/compile.c | 81 |
3 files changed, 57 insertions, 39 deletions
diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index 2868dd0..df1debf 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -666,6 +666,20 @@ class ListComprehensionTest(unittest.TestCase): self._check_in_scopes(code, expected) self._check_in_scopes(code, expected, exec_func=self._replacing_exec) + def test_multiple_comprehension_name_reuse(self): + code = """ + [x for x in [1]] + y = [x for _ in [1]] + """ + self._check_in_scopes(code, {"y": [3]}, ns={"x": 3}) + + code = """ + x = 2 + [x for x in [1]] + y = [x for _ in [1]] + """ + self._check_in_scopes(code, {"x": 2, "y": [3]}, ns={"x": 3}, scopes=["class"]) + self._check_in_scopes(code, {"x": 2, "y": [2]}, ns={"x": 3}, scopes=["function", "module"]) __test__ = {'doctests' : doctests} diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-05-02-21-19-35.gh-issue-118513.qHODjb.rst b/Misc/NEWS.d/next/Core and Builtins/2024-05-02-21-19-35.gh-issue-118513.qHODjb.rst new file mode 100644 index 0000000..b7155b4 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-05-02-21-19-35.gh-issue-118513.qHODjb.rst @@ -0,0 +1 @@ +Fix incorrect :exc:`UnboundLocalError` when two comprehensions in the same function both reference the same name, and in one comprehension the name is bound while in the other it's an implicit global. diff --git a/Python/compile.c b/Python/compile.c index ec47af1..35a7848 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -5561,10 +5561,48 @@ push_inlined_comprehension_state(struct compiler *c, location loc, while (PyDict_Next(entry->ste_symbols, &pos, &k, &v)) { assert(PyLong_Check(v)); long symbol = PyLong_AS_LONG(v); - // only values bound in the comprehension (DEF_LOCAL) need to be handled - // at all; DEF_LOCAL | DEF_NONLOCAL can occur in the case of an - // assignment expression to a nonlocal in the comprehension, these don't - // need handling here since they shouldn't be isolated + long scope = (symbol >> SCOPE_OFFSET) & SCOPE_MASK; + PyObject *outv = PyDict_GetItemWithError(c->u->u_ste->ste_symbols, k); + if (outv == NULL) { + if (PyErr_Occurred()) { + return ERROR; + } + outv = _PyLong_GetZero(); + } + assert(PyLong_CheckExact(outv)); + long outsc = (PyLong_AS_LONG(outv) >> SCOPE_OFFSET) & SCOPE_MASK; + // If a name has different scope inside than outside the comprehension, + // we need to temporarily handle it with the right scope while + // compiling the comprehension. If it's free in the comprehension + // scope, no special handling; it should be handled the same as the + // enclosing scope. (If it's free in outer scope and cell in inner + // scope, we can't treat it as both cell and free in the same function, + // but treating it as free throughout is fine; it's *_DEREF + // either way.) + if ((scope != outsc && scope != FREE && !(scope == CELL && outsc == FREE)) + || in_class_block) { + if (state->temp_symbols == NULL) { + state->temp_symbols = PyDict_New(); + if (state->temp_symbols == NULL) { + return ERROR; + } + } + // update the symbol to the in-comprehension version and save + // the outer version; we'll restore it after running the + // comprehension + Py_INCREF(outv); + if (PyDict_SetItem(c->u->u_ste->ste_symbols, k, v) < 0) { + Py_DECREF(outv); + return ERROR; + } + if (PyDict_SetItem(state->temp_symbols, k, outv) < 0) { + Py_DECREF(outv); + return ERROR; + } + Py_DECREF(outv); + } + // locals handling for names bound in comprehension (DEF_LOCAL | + // DEF_NONLOCAL occurs in assignment expression to nonlocal) if ((symbol & DEF_LOCAL && !(symbol & DEF_NONLOCAL)) || in_class_block) { if (!_PyST_IsFunctionLike(c->u->u_ste)) { // non-function scope: override this name to use fast locals @@ -5589,41 +5627,6 @@ push_inlined_comprehension_state(struct compiler *c, location loc, } } } - long scope = (symbol >> SCOPE_OFFSET) & SCOPE_MASK; - PyObject *outv = PyDict_GetItemWithError(c->u->u_ste->ste_symbols, k); - if (outv == NULL) { - outv = _PyLong_GetZero(); - } - assert(PyLong_Check(outv)); - long outsc = (PyLong_AS_LONG(outv) >> SCOPE_OFFSET) & SCOPE_MASK; - if (scope != outsc && !(scope == CELL && outsc == FREE)) { - // If a name has different scope inside than outside the - // comprehension, we need to temporarily handle it with the - // right scope while compiling the comprehension. (If it's free - // in outer scope and cell in inner scope, we can't treat it as - // both cell and free in the same function, but treating it as - // free throughout is fine; it's *_DEREF either way.) - - if (state->temp_symbols == NULL) { - state->temp_symbols = PyDict_New(); - if (state->temp_symbols == NULL) { - return ERROR; - } - } - // update the symbol to the in-comprehension version and save - // the outer version; we'll restore it after running the - // comprehension - Py_INCREF(outv); - if (PyDict_SetItem(c->u->u_ste->ste_symbols, k, v) < 0) { - Py_DECREF(outv); - return ERROR; - } - if (PyDict_SetItem(state->temp_symbols, k, outv) < 0) { - Py_DECREF(outv); - return ERROR; - } - Py_DECREF(outv); - } // local names bound in comprehension must be isolated from // outer scope; push existing value (which may be NULL if // not defined) on stack |