From 3fad7b07095b87034c69111d975a5703b545140f Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Mon, 15 May 2023 21:57:22 -0700 Subject: [PATCH 1/8] gh-104374: Remove access to class scopes for inlined comprehensions --- Lib/test/test_listcomps.py | 21 +++++++++++++++++++-- Python/compile.c | 22 +++++++++++++++++----- Python/symtable.c | 5 +++-- 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index 23e1b8c1ce3193..b7b8561ef82e19 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -200,7 +200,7 @@ def f(): y = [g for x in [1]] """ outputs = {"y": [2]} - self._check_in_scopes(code, outputs) + self._check_in_scopes(code, outputs, scopes=["module", "function"]) def test_inner_cell_shadows_outer_redefined(self): code = """ @@ -328,7 +328,7 @@ def test_nested_2(self): y = [x for [x ** x for x in range(x)][x - 1] in l] """ outputs = {"y": [3, 3, 3]} - self._check_in_scopes(code, outputs) + self._check_in_scopes(code, outputs, scopes=["module", "function"]) def test_nested_3(self): code = """ @@ -379,6 +379,23 @@ def f(): with self.assertRaises(UnboundLocalError): f() + def test_unbound_local_in_class_scope(self): + class X: + y = 1 + with self.assertRaises(NameError): + [x + y for x in range(2)] + + def test_comprehension_in_class_scope(self): + code = """ + y = 1 + class X: + y = 2 + vals = [(x, y) for x in range(2)] + vals = X.vals + """ + self._check_in_scopes(code, {"vals": [(0, 1), (1, 1)]}, + scopes=["module", "fucntion"]) + __test__ = {'doctests' : doctests} diff --git a/Python/compile.c b/Python/compile.c index 7adbf92089895e..9f3731bbd620b3 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -388,6 +388,8 @@ struct compiler_unit { instr_sequence u_instr_sequence; /* codegen output */ int u_nfblocks; + int u_in_inlined_comp; + struct fblockinfo u_fblock[CO_MAXBLOCKS]; _PyCompile_CodeUnitMetadata u_metadata; @@ -1290,6 +1292,7 @@ compiler_enter_scope(struct compiler *c, identifier name, } u->u_nfblocks = 0; + u->u_in_inlined_comp = 0; u->u_metadata.u_firstlineno = lineno; u->u_metadata.u_consts = PyDict_New(); if (!u->u_metadata.u_consts) { @@ -4137,7 +4140,7 @@ compiler_nameop(struct compiler *c, location loc, case OP_DEREF: switch (ctx) { case Load: - if (c->u->u_ste->ste_type == ClassBlock) { + if (c->u->u_ste->ste_type == ClassBlock && !c->u->u_in_inlined_comp) { op = LOAD_FROM_DICT_OR_DEREF; // First load the locals if (codegen_addop_noarg(INSTR_SEQUENCE(c), LOAD_LOCALS, loc) < 0) { @@ -4188,7 +4191,12 @@ compiler_nameop(struct compiler *c, location loc, break; case OP_NAME: switch (ctx) { - case Load: op = LOAD_NAME; break; + case Load: + op = (c->u->u_ste->ste_type == ClassBlock + && c->u->u_in_inlined_comp) + ? LOAD_GLOBAL + : LOAD_NAME; + break; case Store: op = STORE_NAME; break; case Del: op = DELETE_NAME; break; } @@ -5413,6 +5421,7 @@ push_inlined_comprehension_state(struct compiler *c, location loc, PySTEntryObject *entry, inlined_comprehension_state *state) { + c->u->u_in_inlined_comp++; // iterate over names bound in the comprehension and ensure we isolate // them from the outer scope as needed PyObject *k, *v; @@ -5424,7 +5433,7 @@ push_inlined_comprehension_state(struct compiler *c, location loc, // 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 - if (symbol & DEF_LOCAL && !(symbol & DEF_NONLOCAL)) { + if ((symbol & DEF_LOCAL && !(symbol & DEF_NONLOCAL)) || c->u->u_ste->ste_type == ClassBlock) { if (!_PyST_IsFunctionLike(c->u->u_ste)) { // non-function scope: override this name to use fast locals PyObject *orig = PyDict_GetItem(c->u->u_metadata.u_fasthidden, k); @@ -5444,10 +5453,12 @@ push_inlined_comprehension_state(struct compiler *c, location loc, } } long scope = (symbol >> SCOPE_OFFSET) & SCOPE_MASK; + if (scope == FREE && c->u->u_ste->ste_type == ClassBlock) { + dict_add_o(c->u->u_metadata.u_freevars, k); + } PyObject *outv = PyDict_GetItemWithError(c->u->u_ste->ste_symbols, k); if (outv == NULL) { - assert(PyErr_Occurred()); - return ERROR; + outv = _PyLong_GetZero(); } assert(PyLong_Check(outv)); long outsc = (PyLong_AS_LONG(outv) >> SCOPE_OFFSET) & SCOPE_MASK; @@ -5521,6 +5532,7 @@ static int pop_inlined_comprehension_state(struct compiler *c, location loc, inlined_comprehension_state state) { + c->u->u_in_inlined_comp--; PyObject *k, *v; Py_ssize_t pos = 0; if (state.temp_symbols) { diff --git a/Python/symtable.c b/Python/symtable.c index 3451f6c7bffb6d..11fd5d36893cdf 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -674,8 +674,9 @@ inline_comprehension(PySTEntryObject *ste, PySTEntryObject *comp, } // free vars in comprehension that are locals in outer scope can - // now simply be locals, unless they are free in comp children - if (!is_free_in_any_child(comp, k)) { + // now simply be locals, unless they are free in comp children, + // or if the outer scope is a class block + if (!is_free_in_any_child(comp, k) && ste->ste_type != ClassBlock) { if (PySet_Discard(comp_free, k) < 0) { return 0; } From fa11ca9686457e0a33823f46b23c5266a622ff6b Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Tue, 16 May 2023 06:34:01 -0700 Subject: [PATCH 2/8] Update Lib/test/test_listcomps.py Co-authored-by: Carl Meyer --- Lib/test/test_listcomps.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index b7b8561ef82e19..242c4316033a4b 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -394,7 +394,7 @@ class X: vals = X.vals """ self._check_in_scopes(code, {"vals": [(0, 1), (1, 1)]}, - scopes=["module", "fucntion"]) + scopes=["module", "function"]) __test__ = {'doctests' : doctests} From 1d776fdc86e800714873ccc23691f10fe6014208 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Tue, 16 May 2023 06:37:50 -0700 Subject: [PATCH 3/8] Remove unnecessary change --- Python/compile.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 9f3731bbd620b3..4dc1eedaaa28a4 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -5453,9 +5453,6 @@ push_inlined_comprehension_state(struct compiler *c, location loc, } } long scope = (symbol >> SCOPE_OFFSET) & SCOPE_MASK; - if (scope == FREE && c->u->u_ste->ste_type == ClassBlock) { - dict_add_o(c->u->u_metadata.u_freevars, k); - } PyObject *outv = PyDict_GetItemWithError(c->u->u_ste->ste_symbols, k); if (outv == NULL) { outv = _PyLong_GetZero(); From 932a2daf99738633f2855d81a7e550d63533a772 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Tue, 16 May 2023 06:40:04 -0700 Subject: [PATCH 4/8] Better tests, copied from Carl's PR --- Lib/test/test_listcomps.py | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index 242c4316033a4b..b632516098e052 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -379,22 +379,42 @@ def f(): with self.assertRaises(UnboundLocalError): f() - def test_unbound_local_in_class_scope(self): - class X: + def test_name_error_in_class_scope(self): + code = """ y = 1 - with self.assertRaises(NameError): - [x + y for x in range(2)] + [x + y for x in range(2)] + """ + self._check_in_scopes(code, raises=NameError, scopes=["class"]) + + def test_global_in_class_scope(self): + code = """ + y = 2 + vals = [(x, y) for x in range(2)] + """ + outputs = {"vals": [(0, 1), (1, 1)]} + self._check_in_scopes(code, outputs, ns={"y": 1}, scopes=["class"]) - def test_comprehension_in_class_scope(self): + def test_in_class_scope_inside_function_1(self): + code = """ + class C: + y = 2 + vals = [(x, y) for x in range(2)] + vals = C.vals + """ + outputs = {"vals": [(0, 1), (1, 1)]} + self._check_in_scopes(code, outputs, ns={"y": 1}, scopes=["function"]) + + def test_in_class_scope_inside_function_2(self): code = """ y = 1 - class X: + class C: y = 2 vals = [(x, y) for x in range(2)] - vals = X.vals + vals = C.vals """ - self._check_in_scopes(code, {"vals": [(0, 1), (1, 1)]}, - scopes=["module", "function"]) + outputs = {"vals": [(0, 1), (1, 1)]} + self._check_in_scopes(code, outputs, scopes=["function"]) + __test__ = {'doctests' : doctests} From d5653aeb6f8715051ae58e40cd2592e0f594447f Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Tue, 16 May 2023 07:23:24 -0700 Subject: [PATCH 5/8] A few more tests --- Lib/test/test_listcomps.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index b632516098e052..94865ab9fe7255 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -201,6 +201,7 @@ def f(): """ outputs = {"y": [2]} self._check_in_scopes(code, outputs, scopes=["module", "function"]) + self._check_in_scopes(code, scopes=["class"], raises=NameError) def test_inner_cell_shadows_outer_redefined(self): code = """ @@ -329,6 +330,7 @@ def test_nested_2(self): """ outputs = {"y": [3, 3, 3]} self._check_in_scopes(code, outputs, scopes=["module", "function"]) + self._check_in_scopes(code, scopes=["class"], raises=NameError) def test_nested_3(self): code = """ @@ -415,6 +417,38 @@ class C: outputs = {"vals": [(0, 1), (1, 1)]} self._check_in_scopes(code, outputs, scopes=["function"]) + def test_in_class_scope_with_global(self): + code = """ + y = 1 + class C: + global y + y = 2 + # Ensure the listcomp uses the global, not the value in the + # class namespace + locals()['y'] = 3 + vals = [(x, y) for x in range(2)] + vals = C.vals + """ + outputs = {"vals": [(0, 2), (1, 2)]} + self._check_in_scopes(code, outputs, scopes=["module", "class"]) + outputs = {"vals": [(0, 1), (1, 1)]} + self._check_in_scopes(code, outputs, scopes=["function"]) + + def test_in_class_scope_with_nonlocal(self): + code = """ + y = 1 + class C: + nonlocal y + y = 2 + # Ensure the listcomp uses the global, not the value in the + # class namespace + locals()['y'] = 3 + vals = [(x, y) for x in range(2)] + vals = C.vals + """ + outputs = {"vals": [(0, 2), (1, 2)]} + self._check_in_scopes(code, outputs, scopes=["function"]) + __test__ = {'doctests' : doctests} From 948bc1a9e846bad2a86d4e61bf5ff3b3ef6c218e Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Wed, 17 May 2023 08:25:30 -0700 Subject: [PATCH 6/8] fix definition of 'in a class block' --- Lib/test/test_listcomps.py | 7 +++++++ Python/compile.c | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index 94865ab9fe7255..f5c0c795d37d69 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -449,6 +449,13 @@ class C: outputs = {"vals": [(0, 2), (1, 2)]} self._check_in_scopes(code, outputs, scopes=["function"]) + def test_nested_has_free_var(self): + code = """ + items = [a for a in [1] if [a for _ in [0]]] + """ + outputs = {"items": [1]} + self._check_in_scopes(code, outputs, scopes=["class"]) + __test__ = {'doctests' : doctests} diff --git a/Python/compile.c b/Python/compile.c index 4dc1eedaaa28a4..3247cda8ed2c36 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -5421,6 +5421,7 @@ push_inlined_comprehension_state(struct compiler *c, location loc, PySTEntryObject *entry, inlined_comprehension_state *state) { + int in_class_block = (c->u->u_ste->ste_type == ClassBlock) && !c->u->u_in_inlined_comp; c->u->u_in_inlined_comp++; // iterate over names bound in the comprehension and ensure we isolate // them from the outer scope as needed @@ -5433,7 +5434,7 @@ push_inlined_comprehension_state(struct compiler *c, location loc, // 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 - if ((symbol & DEF_LOCAL && !(symbol & DEF_NONLOCAL)) || c->u->u_ste->ste_type == ClassBlock) { + 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 PyObject *orig = PyDict_GetItem(c->u->u_metadata.u_fasthidden, k); From 045478f8979385441a4be0423b84e3548a0cc5f7 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Wed, 17 May 2023 09:01:35 -0700 Subject: [PATCH 7/8] a couple more tests --- Lib/test/test_listcomps.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index f5c0c795d37d69..7ded3b185767fa 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -456,6 +456,19 @@ def test_nested_has_free_var(self): outputs = {"items": [1]} self._check_in_scopes(code, outputs, scopes=["class"]) + def test_nested_free_var_not_bound_in_outer_comp(self): + code = """ + z = 1 + items = [a for a in [1] if [x for x in [1] if z]] + """ + self._check_in_scopes(code, {"items": [1]}, scopes=["module", "function"]) + self._check_in_scopes(code, {"items": []}, ns={"z": 0}, scopes=["class"]) + + def test_nested_free_var_in_iter(self): + code = """ + items = [_C for _C in [1] for [0, 1][[x for x in [1] if _C][0]] in [2]] + """ + self._check_in_scopes(code, {"items": [1]}) __test__ = {'doctests' : doctests} From afe0277eafb49307d3b19949ec954b8b59c67b2d Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Wed, 17 May 2023 21:54:27 -0700 Subject: [PATCH 8/8] A few more tests --- Lib/test/test_listcomps.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index 7ded3b185767fa..985274dfd6cb93 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -470,6 +470,20 @@ def test_nested_free_var_in_iter(self): """ self._check_in_scopes(code, {"items": [1]}) + def test_nested_free_var_in_expr(self): + code = """ + items = [(_C, [x for x in [1] if _C]) for _C in [0, 1]] + """ + self._check_in_scopes(code, {"items": [(0, []), (1, [1])]}) + + def test_nested_listcomp_in_lambda(self): + code = """ + f = [(z, lambda y: [(x, y, z) for x in [3]]) for z in [1]] + (z, func), = f + out = func(2) + """ + self._check_in_scopes(code, {"z": 1, "out": [(3, 2, 1)]}) + __test__ = {'doctests' : doctests}