From 72afa83e59ab1ea782afe89ef1105b84e5c006f6 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Mon, 30 Jan 2023 15:40:54 -0800 Subject: [PATCH 01/35] gh-97933: inline sync list/dict/set comprehensions --- Doc/library/dis.rst | 10 + Include/internal/pycore_opcode.h | 8 +- Include/internal/pycore_symtable.h | 1 + Include/opcode.h | 13 +- Lib/importlib/_bootstrap_external.py | 3 +- Lib/opcode.py | 2 + Lib/test/test_dis.py | 32 ++-- Lib/test/test_inspect.py | 10 +- Lib/test/test_listcomps.py | 42 +++++ Lib/test/test_trace.py | 4 +- ...3-01-30-15-40-29.gh-issue-97933.nUlp3r.rst | 2 + Python/bytecodes.c | 5 + Python/compile.c | 177 ++++++++++++++++-- Python/generated_cases.c.h | 9 + Python/opcode_metadata.h | 5 + Python/opcode_targets.h | 6 +- Python/symtable.c | 139 +++++++++++--- 17 files changed, 391 insertions(+), 77 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst diff --git a/Doc/library/dis.rst b/Doc/library/dis.rst index 6a68ec4b14be31..b4200cd9ae4b76 100644 --- a/Doc/library/dis.rst +++ b/Doc/library/dis.rst @@ -1214,6 +1214,16 @@ iterations of the loop. .. versionadded:: 3.12 +.. opcode:: LOAD_FAST_OR_NULL (var_num) + + Pushes a reference to the local ``co_varnames[var_num]`` onto the stack, or + pushes ``NULL`` onto the stack if the local variable has not been + initialized. This opcode has the same runtime effect as ``LOAD_FAST``; it + exists to maintain the invariant that ``LOAD_FAST`` will never load ``NULL`` + and may appear only where the variable is guaranteed to be initialized. + + .. versionadded:: 3.12 + .. opcode:: STORE_FAST (var_num) Stores ``STACK.pop()`` into the local ``co_varnames[var_num]``. diff --git a/Include/internal/pycore_opcode.h b/Include/internal/pycore_opcode.h index 05c0485b0641d8..ea4c2ca259e04d 100644 --- a/Include/internal/pycore_opcode.h +++ b/Include/internal/pycore_opcode.h @@ -166,6 +166,7 @@ const uint8_t _PyOpcode_Deopt[256] = { [LOAD_DEREF] = LOAD_DEREF, [LOAD_FAST] = LOAD_FAST, [LOAD_FAST_CHECK] = LOAD_FAST_CHECK, + [LOAD_FAST_OR_NULL] = LOAD_FAST_OR_NULL, [LOAD_FAST__LOAD_CONST] = LOAD_FAST, [LOAD_FAST__LOAD_FAST] = LOAD_FAST, [LOAD_GLOBAL] = LOAD_GLOBAL, @@ -371,7 +372,7 @@ static const char *const _PyOpcode_OpName[263] = { [JUMP_BACKWARD] = "JUMP_BACKWARD", [COMPARE_AND_BRANCH] = "COMPARE_AND_BRANCH", [CALL_FUNCTION_EX] = "CALL_FUNCTION_EX", - [STORE_FAST__STORE_FAST] = "STORE_FAST__STORE_FAST", + [LOAD_FAST_OR_NULL] = "LOAD_FAST_OR_NULL", [EXTENDED_ARG] = "EXTENDED_ARG", [LIST_APPEND] = "LIST_APPEND", [SET_ADD] = "SET_ADD", @@ -381,15 +382,15 @@ static const char *const _PyOpcode_OpName[263] = { [YIELD_VALUE] = "YIELD_VALUE", [RESUME] = "RESUME", [MATCH_CLASS] = "MATCH_CLASS", + [STORE_FAST__STORE_FAST] = "STORE_FAST__STORE_FAST", [STORE_SUBSCR_DICT] = "STORE_SUBSCR_DICT", - [STORE_SUBSCR_LIST_INT] = "STORE_SUBSCR_LIST_INT", [FORMAT_VALUE] = "FORMAT_VALUE", [BUILD_CONST_KEY_MAP] = "BUILD_CONST_KEY_MAP", [BUILD_STRING] = "BUILD_STRING", + [STORE_SUBSCR_LIST_INT] = "STORE_SUBSCR_LIST_INT", [UNPACK_SEQUENCE_LIST] = "UNPACK_SEQUENCE_LIST", [UNPACK_SEQUENCE_TUPLE] = "UNPACK_SEQUENCE_TUPLE", [UNPACK_SEQUENCE_TWO_TUPLE] = "UNPACK_SEQUENCE_TWO_TUPLE", - [161] = "<161>", [LIST_EXTEND] = "LIST_EXTEND", [SET_UPDATE] = "SET_UPDATE", [DICT_MERGE] = "DICT_MERGE", @@ -495,7 +496,6 @@ static const char *const _PyOpcode_OpName[263] = { #endif #define EXTRA_CASES \ - case 161: \ case 166: \ case 167: \ case 168: \ diff --git a/Include/internal/pycore_symtable.h b/Include/internal/pycore_symtable.h index 8532646ce7d95c..97e2cc6469a5bb 100644 --- a/Include/internal/pycore_symtable.h +++ b/Include/internal/pycore_symtable.h @@ -64,6 +64,7 @@ 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_inlined : 1; /* true if this comprehension is inlined */ 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 */ diff --git a/Include/opcode.h b/Include/opcode.h index 827f9931beb3e6..03c9e48cf4264b 100644 --- a/Include/opcode.h +++ b/Include/opcode.h @@ -97,6 +97,7 @@ extern "C" { #define JUMP_BACKWARD 140 #define COMPARE_AND_BRANCH 141 #define CALL_FUNCTION_EX 142 +#define LOAD_FAST_OR_NULL 143 #define EXTENDED_ARG 144 #define LIST_APPEND 145 #define SET_ADD 146 @@ -180,12 +181,12 @@ extern "C" { #define STORE_ATTR_SLOT 87 #define STORE_ATTR_WITH_HINT 113 #define STORE_FAST__LOAD_FAST 121 -#define STORE_FAST__STORE_FAST 143 -#define STORE_SUBSCR_DICT 153 -#define STORE_SUBSCR_LIST_INT 154 -#define UNPACK_SEQUENCE_LIST 158 -#define UNPACK_SEQUENCE_TUPLE 159 -#define UNPACK_SEQUENCE_TWO_TUPLE 160 +#define STORE_FAST__STORE_FAST 153 +#define STORE_SUBSCR_DICT 154 +#define STORE_SUBSCR_LIST_INT 158 +#define UNPACK_SEQUENCE_LIST 159 +#define UNPACK_SEQUENCE_TUPLE 160 +#define UNPACK_SEQUENCE_TWO_TUPLE 161 #define DO_TRACING 255 #define HAS_ARG(op) ((((op) >= HAVE_ARGUMENT) && (!IS_PSEUDO_OPCODE(op)))\ diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index e760fbb15759d4..8c79eb42d97df2 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -431,6 +431,7 @@ def _write_atomic(path, data, mode=0o666): # Python 3.12a5 3515 (Embed jump mask in COMPARE_OP oparg) # Python 3.12a5 3516 (Add COMPARE_AND_BRANCH instruction) # Python 3.12a5 3517 (Change YIELD_VALUE oparg to exception block depth) +# Python 3.12a5 3518 (Inline sync list/dict/set comprehensions in the calling function) # Python 3.13 will start with 3550 @@ -443,7 +444,7 @@ def _write_atomic(path, data, mode=0o666): # Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array # in PC/launcher.c must also be updated. -MAGIC_NUMBER = (3517).to_bytes(2, 'little') + b'\r\n' +MAGIC_NUMBER = (3518).to_bytes(2, 'little') + b'\r\n' _RAW_MAGIC_NUMBER = int.from_bytes(MAGIC_NUMBER, 'little') # For import.c diff --git a/Lib/opcode.py b/Lib/opcode.py index c317e23beae62b..f04aab308fc981 100644 --- a/Lib/opcode.py +++ b/Lib/opcode.py @@ -196,6 +196,8 @@ def pseudo_op(name, op, real_ops): hascompare.append(141) def_op('CALL_FUNCTION_EX', 142) # Flags +def_op('LOAD_FAST_OR_NULL', 143) # Local variable number, may load NULL if undefined +haslocal.append(143) def_op('EXTENDED_ARG', 144) EXTENDED_ARG = 144 diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index bdf48c15309296..7095d28edcf977 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -161,7 +161,7 @@ def bug708901(): def bug1333982(x=[]): - assert 0, ([s for s in x] + + assert 0, ((s for s in x) + 1) pass @@ -169,7 +169,7 @@ def bug1333982(x=[]): %3d RESUME 0 %3d LOAD_ASSERTION_ERROR - LOAD_CONST 1 ( at 0x..., file "%s", line %d>) + LOAD_CONST 1 ( at 0x..., file "%s", line %d>) MAKE_FUNCTION 0 LOAD_FAST 0 (x) GET_ITER @@ -642,7 +642,7 @@ async def _co(x): def _h(y): def foo(x): '''funcdoc''' - return [x + z for z in y] + return list(x + z for z in y) return foo dis_nested_0 = """\ @@ -672,13 +672,15 @@ def foo(x): %3d RESUME 0 -%3d LOAD_CLOSURE 0 (x) +%3d LOAD_GLOBAL 1 (NULL + list) + LOAD_CLOSURE 0 (x) BUILD_TUPLE 1 - LOAD_CONST 1 ( at 0x..., file "%s", line %d>) + LOAD_CONST 1 ( at 0x..., file "%s", line %d>) MAKE_FUNCTION 8 (closure) LOAD_DEREF 1 (y) GET_ITER CALL 0 + CALL 1 RETURN_VALUE """ % (dis_nested_0, __file__, @@ -690,21 +692,29 @@ def foo(x): ) dis_nested_2 = """%s -Disassembly of at 0x..., file "%s", line %d>: +Disassembly of at 0x..., file "%s", line %d>: COPY_FREE_VARS 1 -%3d RESUME 0 - BUILD_LIST 0 +%3d RETURN_GENERATOR + POP_TOP + RESUME 0 LOAD_FAST 0 (.0) - >> FOR_ITER 7 (to 26) + >> FOR_ITER 9 (to 32) STORE_FAST 1 (z) LOAD_DEREF 2 (x) LOAD_FAST 1 (z) BINARY_OP 0 (+) - LIST_APPEND 2 - JUMP_BACKWARD 9 (to 8) + YIELD_VALUE 1 + RESUME 1 + POP_TOP + JUMP_BACKWARD 11 (to 10) >> END_FOR + LOAD_CONST 0 (None) RETURN_VALUE + >> CALL_INTRINSIC_1 3 + RERAISE 1 +ExceptionTable: +1 row """ % (dis_nested_1, __file__, _h.__code__.co_firstlineno + 3, diff --git a/Lib/test/test_inspect.py b/Lib/test/test_inspect.py index 92aba519d28a08..3db72e41f26d22 100644 --- a/Lib/test/test_inspect.py +++ b/Lib/test/test_inspect.py @@ -4106,14 +4106,14 @@ def test(*args, **kwargs): @cpython_only def test_signature_bind_implicit_arg(self): - # Issue #19611: getcallargs should work with set comprehensions + # Issue #19611: getcallargs should work with comprehensions def make_set(): - return {z * z for z in range(5)} - setcomp_code = make_set.__code__.co_consts[1] - setcomp_func = types.FunctionType(setcomp_code, {}) + return set(z * z for z in range(5)) + gencomp_code = make_set.__code__.co_consts[1] + gencomp_func = types.FunctionType(gencomp_code, {}) iterator = iter(range(5)) - self.assertEqual(self.call(setcomp_func, iterator), {0, 1, 4, 9, 16}) + self.assertEqual(set(self.call(gencomp_func, iterator)), {0, 1, 4, 9, 16}) def test_signature_bind_posonly_kwargs(self): def foo(bar, /, **kwargs): diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index 91bf2547edc4ae..0fef1d55aec9e9 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -143,6 +143,48 @@ >>> test_func() [2, 2, 2, 2, 2] +A comprehension's iteration var, if in a cell, doesn't stomp on a previous value: + + >>> def test_func(): + ... y = 10 + ... items = [(lambda: y) for y in range(5)] + ... x = y + ... y = 20 + ... return x, [z() for z in items] + >>> test_func() + (10, [4, 4, 4, 4, 4]) + +A comprehension's iteration var doesn't shadow implicit globals for sibling scopes: + + >>> g = -1 + >>> def test_func(): + ... def inner(): + ... return g + ... [g for g in range(5)] + ... return inner + >>> test_func()() + -1 + +A modification to a closed-over variable is visible in the outer scope: + + >>> def test_func(): + ... x = -1 + ... items = [(x:=y) for y in range(3)] + ... return x + >>> test_func() + 2 + +Comprehensions' scopes don't interact with each other: + + >>> def test_func(): + ... lst = list(range(3)) + ... ret = [lambda: x for x in lst] + ... inc = [x + 1 for x in lst] + ... [x for x in inc] + ... return ret + >>> test_func()[0]() + 2 + """ diff --git a/Lib/test/test_trace.py b/Lib/test/test_trace.py index fad2b3b8379ffc..73339ebdb7c4e9 100644 --- a/Lib/test/test_trace.py +++ b/Lib/test/test_trace.py @@ -187,9 +187,7 @@ def test_trace_list_comprehension(self): firstlineno_called = get_firstlineno(traced_doubler) expected = { (self.my_py_filename, firstlineno_calling + 1): 1, - # List comprehensions work differently in 3.x, so the count - # below changed compared to 2.x. - (self.my_py_filename, firstlineno_calling + 2): 12, + (self.my_py_filename, firstlineno_calling + 2): 11, (self.my_py_filename, firstlineno_calling + 3): 1, (self.my_py_filename, firstlineno_called + 1): 10, } diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst b/Misc/NEWS.d/next/Core and Builtins/2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst new file mode 100644 index 00000000000000..48fdca6cd438da --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst @@ -0,0 +1,2 @@ +Inline (non-async) list, dict and set comprehensions to reduce performance +and bytecode size overhead. diff --git a/Python/bytecodes.c b/Python/bytecodes.c index d1e59f7908b580..d054c9ff6717e8 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -114,6 +114,11 @@ dummy_func( Py_INCREF(value); } + inst(LOAD_FAST_OR_NULL, (-- value)) { + value = GETLOCAL(oparg); + Py_XINCREF(value); + } + inst(LOAD_CONST, (-- value)) { value = GETITEM(consts, oparg); Py_INCREF(value); diff --git a/Python/compile.c b/Python/compile.c index 70d05af58161f9..cef3906d508d64 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -551,13 +551,15 @@ static int compiler_sync_comprehension_generator( struct compiler *c, location loc, asdl_comprehension_seq *generators, int gen_index, int depth, - expr_ty elt, expr_ty val, int type); + expr_ty elt, expr_ty val, int type, + int outermost_iter_is_param); static int compiler_async_comprehension_generator( struct compiler *c, location loc, asdl_comprehension_seq *generators, int gen_index, int depth, - expr_ty elt, expr_ty val, int type); + expr_ty elt, expr_ty val, int type, + int outermost_iter_is_param); static int compiler_pattern(struct compiler *, pattern_ty, pattern_context *); static int compiler_match(struct compiler *, stmt_ty); @@ -1228,6 +1230,7 @@ stack_effect(int opcode, int oparg, int jump) case LOAD_FAST: case LOAD_FAST_CHECK: + case LOAD_FAST_OR_NULL: return 1; case STORE_FAST: return -1; @@ -5149,16 +5152,19 @@ static int compiler_comprehension_generator(struct compiler *c, location loc, asdl_comprehension_seq *generators, int gen_index, int depth, - expr_ty elt, expr_ty val, int type) + expr_ty elt, expr_ty val, int type, + int outermost_iter_is_param) { comprehension_ty gen; gen = (comprehension_ty)asdl_seq_GET(generators, gen_index); if (gen->is_async) { return compiler_async_comprehension_generator( - c, loc, generators, gen_index, depth, elt, val, type); + c, loc, generators, gen_index, depth, elt, val, type, + outermost_iter_is_param); } else { return compiler_sync_comprehension_generator( - c, loc, generators, gen_index, depth, elt, val, type); + c, loc, generators, gen_index, depth, elt, val, type, + outermost_iter_is_param); } } @@ -5166,7 +5172,8 @@ static int compiler_sync_comprehension_generator(struct compiler *c, location loc, asdl_comprehension_seq *generators, int gen_index, int depth, - expr_ty elt, expr_ty val, int type) + expr_ty elt, expr_ty val, int type, + int outermost_iter_is_param) { /* generate code for the iterator, then each of the ifs, and then write to the element */ @@ -5178,7 +5185,7 @@ compiler_sync_comprehension_generator(struct compiler *c, location loc, comprehension_ty gen = (comprehension_ty)asdl_seq_GET(generators, gen_index); - if (gen_index == 0) { + if (gen_index == 0 && outermost_iter_is_param) { /* Receive outermost iter as an implicit argument */ c->u->u_argcount = 1; ADDOP_I(c, loc, LOAD_FAST, 0); @@ -5229,7 +5236,7 @@ compiler_sync_comprehension_generator(struct compiler *c, location loc, RETURN_IF_ERROR( compiler_comprehension_generator(c, loc, generators, gen_index, depth, - elt, val, type)); + elt, val, type, 0)); } location elt_loc = LOC(elt); @@ -5282,7 +5289,8 @@ static int compiler_async_comprehension_generator(struct compiler *c, location loc, asdl_comprehension_seq *generators, int gen_index, int depth, - expr_ty elt, expr_ty val, int type) + expr_ty elt, expr_ty val, int type, + int outermost_iter_is_param) { NEW_JUMP_TARGET_LABEL(c, start); NEW_JUMP_TARGET_LABEL(c, except); @@ -5291,7 +5299,7 @@ compiler_async_comprehension_generator(struct compiler *c, location loc, comprehension_ty gen = (comprehension_ty)asdl_seq_GET(generators, gen_index); - if (gen_index == 0) { + if (gen_index == 0 && outermost_iter_is_param) { /* Receive outermost iter as an implicit argument */ c->u->u_argcount = 1; ADDOP_I(c, loc, LOAD_FAST, 0); @@ -5326,7 +5334,7 @@ compiler_async_comprehension_generator(struct compiler *c, location loc, RETURN_IF_ERROR( compiler_comprehension_generator(c, loc, generators, gen_index, depth, - elt, val, type)); + elt, val, type, 0)); } location elt_loc = LOC(elt); @@ -5375,26 +5383,144 @@ compiler_async_comprehension_generator(struct compiler *c, location loc, return SUCCESS; } +typedef struct { + PyObject *pushed_locals; + PyObject *temp_symbols; +} inlined_comprehension_state; + +static int +push_inlined_comprehension_state(struct compiler *c, location loc, + PySTEntryObject *entry, + inlined_comprehension_state *state) +{ + // iterate over names bound in the comprehension and ensure we isolate + // them from the outer scope as needed + PyObject *k, *v; + Py_ssize_t pos = 0; + 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; DEF_LOCAL | DEF_NONLOCAL (oddly) can occur in the case + // of an assignment expression to a nonlocal in the comprehension, + // these don't need handling here + if (symbol & DEF_LOCAL && ~symbol & DEF_NONLOCAL) { + long scope = (symbol >> SCOPE_OFFSET) & SCOPE_MASK; + PyObject *outv = PyDict_GetItemWithError(c->u->u_ste->ste_symbols, k); + if (outv == NULL) { + return ERROR; + } + assert(PyLong_Check(outv)); + long outsc = (PyLong_AS_LONG(outv) >> SCOPE_OFFSET) & SCOPE_MASK; + if (outsc == GLOBAL_IMPLICIT || outsc == GLOBAL_EXPLICIT) { + if (state->temp_symbols == NULL) { + state->temp_symbols = PyDict_New(); + if (state->temp_symbols == NULL) { + return ERROR; + } + } + // if a name is global in the outer scope but local in the + // comprehension scope, we need to keep it global in outer + // scope but ensure the comprehension writes to the local, + // not the global + Py_INCREF(outv); + if (PyDict_SetItem(c->u->u_ste->ste_symbols, k, v)) { + return ERROR; + } + if (PyDict_SetItem(state->temp_symbols, k, outv)) { + return ERROR; + } + Py_DECREF(outv); + } else if (scope == LOCAL || scope == CELL) { + // local names bound in comprehension must be isolated from + // outer scope; push existing value (which may be NULL if + // not defined) on stack + if (state->pushed_locals == NULL) { + state->pushed_locals = PyList_New(0); + if (state->pushed_locals == NULL) { + return ERROR; + } + } + if (scope == CELL) { + ADDOP_NAME(c, loc, LOAD_FAST_OR_NULL, k, varnames); + ADDOP_NAME(c, loc, MAKE_CELL, k, cellvars); + } else { + ADDOP_NAME(c, loc, LOAD_FAST_OR_NULL, k, varnames); + } + if (PyList_Append(state->pushed_locals, k)) { + return ERROR; + } + } + } + } + return SUCCESS; +} + +static int +pop_inlined_comprehension_state(struct compiler *c, location loc, + inlined_comprehension_state state) +{ + PyObject *k, *v; + Py_ssize_t pos = 0; + if (state.temp_symbols) { + // restore scope for globals that we temporarily set as locals + while (PyDict_Next(state.temp_symbols, &pos, &k, &v)) { + if (PyDict_SetItem(c->u->u_ste->ste_symbols, k, v)) { + return ERROR; + } + } + } + if (state.pushed_locals) { + // pop names we pushed to stack earlier + for (Py_ssize_t i = PyList_GET_SIZE(state.pushed_locals); i > 0; --i) { + k = PyList_GetItem(state.pushed_locals, i - 1); + if (k == NULL) { + return ERROR; + } + // preserve the built result of the comprehension; we could be + // cleverer about minimizing swaps but it doesn't matter because + // `apply_static_swaps` will eliminate all of these anyway + ADDOP_I(c, loc, SWAP, 2); + ADDOP_NAME(c, loc, STORE_FAST, k, varnames); + } + } + return SUCCESS; +} + static int compiler_comprehension(struct compiler *c, expr_ty e, int type, identifier name, asdl_comprehension_seq *generators, expr_ty elt, expr_ty val) { PyCodeObject *co = NULL; + inlined_comprehension_state inline_state = {NULL, NULL}; comprehension_ty outermost; int scope_type = c->u->u_scope_type; - int is_async_generator = 0; int is_top_level_await = IS_TOP_LEVEL_AWAIT(c); - - outermost = (comprehension_ty) asdl_seq_GET(generators, 0); - if (compiler_enter_scope(c, name, COMPILER_SCOPE_COMPREHENSION, - (void *)e, e->lineno) < 0) - { + PySTEntryObject *entry = PySymtable_Lookup(c->c_st, (void *)e); + if (entry == NULL) { goto error; } + int is_inlined = entry->ste_comp_inlined; + int is_async_generator = entry->ste_coroutine; + // we only inline comprehensions in functions + assert(!is_inlined || c->u->u_ste->ste_type == FunctionBlock); + location loc = LOC(e); - is_async_generator = c->u->u_ste->ste_coroutine; + outermost = (comprehension_ty) asdl_seq_GET(generators, 0); + if (is_inlined) { + if (push_inlined_comprehension_state(c, loc, entry, &inline_state)) { + goto error; + } + } else { + if (compiler_enter_scope(c, name, COMPILER_SCOPE_COMPREHENSION, + (void *)e, e->lineno) < 0) + { + goto error; + } + } + Py_CLEAR(entry); if (is_async_generator && type != COMP_GENEXP && scope_type != COMPILER_SCOPE_ASYNC_FUNCTION && @@ -5428,10 +5554,17 @@ compiler_comprehension(struct compiler *c, expr_ty e, int type, } if (compiler_comprehension_generator(c, loc, generators, 0, 0, - elt, val, type) < 0) { + elt, val, type, !is_inlined) < 0) { goto error_in_scope; } + if (is_inlined) { + if (pop_inlined_comprehension_state(c, loc, inline_state)) { + goto error; + } + return SUCCESS; + } + if (type != COMP_GENEXP) { ADDOP(c, LOC(e), RETURN_VALUE); } @@ -5475,9 +5608,13 @@ compiler_comprehension(struct compiler *c, expr_ty e, int type, return SUCCESS; error_in_scope: - compiler_exit_scope(c); + if (!is_inlined) { + compiler_exit_scope(c); + } error: Py_XDECREF(co); + Py_XDECREF(inline_state.pushed_locals); + Py_XDECREF(inline_state.temp_symbols); return ERROR; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 3ee30ae8df9e3c..0f9febaf2e2a71 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -46,6 +46,15 @@ DISPATCH(); } + TARGET(LOAD_FAST_OR_NULL) { + PyObject *value; + value = GETLOCAL(oparg); + Py_XINCREF(value); + STACK_GROW(1); + POKE(1, value); + DISPATCH(); + } + TARGET(LOAD_CONST) { PREDICTED(LOAD_CONST); PyObject *value; diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index e76ddda2f0292d..78b678e92fffd3 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -16,6 +16,8 @@ _PyOpcode_num_popped(int opcode, int oparg) { return 0; case LOAD_FAST: return 0; + case LOAD_FAST_OR_NULL: + return 0; case LOAD_CONST: return 0; case STORE_FAST: @@ -362,6 +364,8 @@ _PyOpcode_num_pushed(int opcode, int oparg) { return 1; case LOAD_FAST: return 1; + case LOAD_FAST_OR_NULL: + return 1; case LOAD_CONST: return 1; case STORE_FAST: @@ -707,6 +711,7 @@ struct opcode_metadata { [LOAD_CLOSURE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [LOAD_FAST_CHECK] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [LOAD_FAST] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, + [LOAD_FAST_OR_NULL] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [LOAD_CONST] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [STORE_FAST] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [LOAD_FAST__LOAD_FAST] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IBIB }, diff --git a/Python/opcode_targets.h b/Python/opcode_targets.h index f1c3f3e0c4ee17..e04df563c0df91 100644 --- a/Python/opcode_targets.h +++ b/Python/opcode_targets.h @@ -142,7 +142,7 @@ static void *opcode_targets[256] = { &&TARGET_JUMP_BACKWARD, &&TARGET_COMPARE_AND_BRANCH, &&TARGET_CALL_FUNCTION_EX, - &&TARGET_STORE_FAST__STORE_FAST, + &&TARGET_LOAD_FAST_OR_NULL, &&TARGET_EXTENDED_ARG, &&TARGET_LIST_APPEND, &&TARGET_SET_ADD, @@ -152,15 +152,15 @@ static void *opcode_targets[256] = { &&TARGET_YIELD_VALUE, &&TARGET_RESUME, &&TARGET_MATCH_CLASS, + &&TARGET_STORE_FAST__STORE_FAST, &&TARGET_STORE_SUBSCR_DICT, - &&TARGET_STORE_SUBSCR_LIST_INT, &&TARGET_FORMAT_VALUE, &&TARGET_BUILD_CONST_KEY_MAP, &&TARGET_BUILD_STRING, + &&TARGET_STORE_SUBSCR_LIST_INT, &&TARGET_UNPACK_SEQUENCE_LIST, &&TARGET_UNPACK_SEQUENCE_TUPLE, &&TARGET_UNPACK_SEQUENCE_TWO_TUPLE, - &&_unknown_opcode, &&TARGET_LIST_EXTEND, &&TARGET_SET_UPDATE, &&TARGET_DICT_MERGE, diff --git a/Python/symtable.c b/Python/symtable.c index 89a2bc437dfa9b..efbe8611204748 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -104,6 +104,7 @@ ste_new(struct symtable *st, identifier name, _Py_block_ty block, ste->ste_comprehension = NoComprehension; ste->ste_returns_value = 0; ste->ste_needs_class_closure = 0; + ste->ste_comp_inlined = 0; ste->ste_comp_iter_target = 0; ste->ste_comp_iter_expr = 0; @@ -559,6 +560,76 @@ analyze_name(PySTEntryObject *ste, PyObject *scopes, PyObject *name, long flags, return 1; } +static int +is_free_in_children(PySTEntryObject *entry, PyObject *key) { + for (Py_ssize_t i = 0; i < PyList_GET_SIZE(entry->ste_children); i++) { + PySTEntryObject *child_ste = (PySTEntryObject *)PyList_GET_ITEM( + entry->ste_children, i); + long scope = _PyST_GetScope(child_ste, key); + if (scope == FREE) { + return 1; + } + } + return 0; +} + +static int +inline_comprehension(PySTEntryObject *ste, PySTEntryObject *comp, + PyObject *scopes, PyObject *comp_free) +{ + PyObject *k, *v; + Py_ssize_t pos = 0; + while (PyDict_Next(comp->ste_symbols, &pos, &k, &v)) { + // skip comprehension parameter + long comp_flags = PyLong_AS_LONG(v); + if (comp_flags & DEF_PARAM) { + assert(_PyUnicode_EqualToASCIIString(k, ".0")); + continue; + } + int scope = (comp_flags >> SCOPE_OFFSET) & SCOPE_MASK; + int only_flags = comp_flags & ((1 << SCOPE_OFFSET) - 1); + PyObject *existing = PyDict_GetItemWithError(ste->ste_symbols, k); + if (existing == NULL && PyErr_Occurred()) { + return 0; + } + if (!existing) { + // name does not exist in scope, copy from comprehension + assert(scope != FREE || PySet_Contains(comp_free, k) == 1); + PyObject *v_flags = PyLong_FromLong(only_flags); + if (v_flags == NULL) { + return 0; + } + int ok = PyDict_SetItem(ste->ste_symbols, k, v_flags); + Py_DECREF(v_flags); + if (ok < 0) { + return 0; + } + SET_SCOPE(scopes, k, scope); + } else { + // name already exists in scope + PyObject *v_existing_scope = PyDict_GetItemWithError(scopes, k); + if (v_existing_scope == NULL) { + return 0; + } + long existing_scope = PyLong_AsLong(v_existing_scope); + // if name in comprehension was a cell, promote to cell + if (scope == CELL && existing_scope != CELL) { + SET_SCOPE(scopes, k, CELL); + } else { + // free vars in comprehension that are locals in outer scope can + // now simply be locals, unless they are free in comp children + if ((PyLong_AsLong(existing) && DEF_BOUND) && + !is_free_in_children(comp, k)) { + if (PySet_Discard(comp_free, k) < 0) { + return 0; + } + } + } + } + } + return 1; +} + #undef SET_SCOPE /* If a name is defined in free and also in locals, then this block @@ -728,14 +799,14 @@ update_symbols(PyObject *symbols, PyObject *scopes, static int analyze_child_block(PySTEntryObject *entry, PyObject *bound, PyObject *free, - PyObject *global, PyObject* child_free); + PyObject *global, PyObject **child_free); static int analyze_block(PySTEntryObject *ste, PyObject *bound, PyObject *free, PyObject *global) { PyObject *name, *v, *local = NULL, *scopes = NULL, *newbound = NULL; - PyObject *newglobal = NULL, *newfree = NULL, *allfree = NULL; + PyObject *newglobal = NULL, *newfree = NULL; PyObject *temp; int i, success = 0; Py_ssize_t pos = 0; @@ -747,8 +818,8 @@ analyze_block(PySTEntryObject *ste, PyObject *bound, PyObject *free, if (!scopes) goto error; - /* Allocate new global and bound variable dictionaries. These - dictionaries hold the names visible in nested blocks. For + /* Allocate new global, bound and free variable sets. These + sets hold the names visible in nested blocks. For ClassBlocks, the bound and global names are initialized before analyzing names, because class bindings aren't visible in methods. For other blocks, they are initialized @@ -827,28 +898,54 @@ analyze_block(PySTEntryObject *ste, PyObject *bound, PyObject *free, newbound, newglobal now contain the names visible in nested blocks. The free variables in the children will - be collected in allfree. + be added to newfree. */ - allfree = PySet_New(NULL); - if (!allfree) - goto error; for (i = 0; i < PyList_GET_SIZE(ste->ste_children); ++i) { + PyObject *child_free = NULL; PyObject *c = PyList_GET_ITEM(ste->ste_children, i); PySTEntryObject* entry; assert(c && PySTEntry_Check(c)); entry = (PySTEntryObject*)c; + + // we inline comprehensions if they are inside a function, are not a + // generator, and are not async + int inline_comp = + entry->ste_comprehension && + ste->ste_type == FunctionBlock && + !entry->ste_generator && + !entry->ste_coroutine; + if (!analyze_child_block(entry, newbound, newfree, newglobal, - allfree)) + &child_free)) + goto error; + if (inline_comp) { + if (!inline_comprehension(ste, entry, scopes, child_free)) { + Py_DECREF(child_free); + goto error; + } + entry->ste_comp_inlined = 1; + } + temp = PyNumber_InPlaceOr(newfree, child_free); + Py_DECREF(child_free); + if (!temp) goto error; /* Check if any children have free variables */ if (entry->ste_free || entry->ste_child_free) ste->ste_child_free = 1; } - temp = PyNumber_InPlaceOr(newfree, allfree); - if (!temp) - goto error; - Py_DECREF(temp); + /* Splice children of inlined comprehensions into our children list */ + for (i = PyList_GET_SIZE(ste->ste_children) - 1; i >= 0; --i) { + PyObject* c = PyList_GET_ITEM(ste->ste_children, i); + PySTEntryObject* entry; + assert(c && PySTEntry_Check(c)); + entry = (PySTEntryObject*)c; + if (entry->ste_comp_inlined && + PyList_SetSlice(ste->ste_children, i, i + 1, + entry->ste_children) < 0) { + goto error; + } + } /* Check if any local variables must be converted to cell variables */ if (ste->ste_type == FunctionBlock && !analyze_cells(scopes, newfree)) @@ -871,7 +968,6 @@ analyze_block(PySTEntryObject *ste, PyObject *bound, PyObject *free, Py_XDECREF(newbound); Py_XDECREF(newglobal); Py_XDECREF(newfree); - Py_XDECREF(allfree); if (!success) assert(PyErr_Occurred()); return success; @@ -879,16 +975,15 @@ analyze_block(PySTEntryObject *ste, PyObject *bound, PyObject *free, static int analyze_child_block(PySTEntryObject *entry, PyObject *bound, PyObject *free, - PyObject *global, PyObject* child_free) + PyObject *global, PyObject** child_free) { PyObject *temp_bound = NULL, *temp_global = NULL, *temp_free = NULL; - PyObject *temp; - /* Copy the bound and global dictionaries. + /* Copy the bound/global/free sets. - These dictionaries are used by all blocks enclosed by the + These sets are used by all blocks enclosed by the current block. The analyze_block() call modifies these - dictionaries. + sets. */ temp_bound = PySet_New(bound); @@ -903,12 +998,8 @@ analyze_child_block(PySTEntryObject *entry, PyObject *bound, PyObject *free, if (!analyze_block(entry, temp_bound, temp_free, temp_global)) goto error; - temp = PyNumber_InPlaceOr(child_free, temp_free); - if (!temp) - goto error; - Py_DECREF(temp); + *child_free = temp_free; Py_DECREF(temp_bound); - Py_DECREF(temp_free); Py_DECREF(temp_global); return 1; error: From 89882347b93e0fac4f5539c00069f4476ed84745 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Mon, 30 Jan 2023 16:02:28 -0800 Subject: [PATCH 02/35] simplify cell handling code slightly --- Python/compile.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index cef3906d508d64..709b26e05eacb8 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -5441,11 +5441,9 @@ push_inlined_comprehension_state(struct compiler *c, location loc, return ERROR; } } + ADDOP_NAME(c, loc, LOAD_FAST_OR_NULL, k, varnames); if (scope == CELL) { - ADDOP_NAME(c, loc, LOAD_FAST_OR_NULL, k, varnames); ADDOP_NAME(c, loc, MAKE_CELL, k, cellvars); - } else { - ADDOP_NAME(c, loc, LOAD_FAST_OR_NULL, k, varnames); } if (PyList_Append(state->pushed_locals, k)) { return ERROR; From 22c4a86a19aac5696e2d4a714ae99c53893469aa Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Mon, 30 Jan 2023 16:08:32 -0800 Subject: [PATCH 03/35] clarify comments --- Python/compile.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 709b26e05eacb8..6f909cd8b5f015 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -5403,7 +5403,7 @@ push_inlined_comprehension_state(struct compiler *c, location loc, // only values bound in the comprehension (DEF_LOCAL) need to be // handled; DEF_LOCAL | DEF_NONLOCAL (oddly) can occur in the case // of an assignment expression to a nonlocal in the comprehension, - // these don't need handling here + // these don't need handling here since they shouldn't be isolated if (symbol & DEF_LOCAL && ~symbol & DEF_NONLOCAL) { long scope = (symbol >> SCOPE_OFFSET) & SCOPE_MASK; PyObject *outv = PyDict_GetItemWithError(c->u->u_ste->ste_symbols, k); @@ -5413,16 +5413,19 @@ push_inlined_comprehension_state(struct compiler *c, location loc, assert(PyLong_Check(outv)); long outsc = (PyLong_AS_LONG(outv) >> SCOPE_OFFSET) & SCOPE_MASK; if (outsc == GLOBAL_IMPLICIT || outsc == GLOBAL_EXPLICIT) { + // if a name is global in the outer scope but local in the + // comprehension scope, we need to keep it global in outer + // scope but ensure the comprehension writes to the local, + // not the global; this is all the isolation we need. if (state->temp_symbols == NULL) { state->temp_symbols = PyDict_New(); if (state->temp_symbols == NULL) { return ERROR; } } - // if a name is global in the outer scope but local in the - // comprehension scope, we need to keep it global in outer - // scope but ensure the comprehension writes to the local, - // not the global + // 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)) { return ERROR; @@ -5441,6 +5444,9 @@ push_inlined_comprehension_state(struct compiler *c, location loc, return ERROR; } } + // in the case of a cell, this will actually push the cell + // itself to the stack, then we'll create a new one for the + // comprehension and restore the original one after ADDOP_NAME(c, loc, LOAD_FAST_OR_NULL, k, varnames); if (scope == CELL) { ADDOP_NAME(c, loc, MAKE_CELL, k, cellvars); @@ -5475,9 +5481,9 @@ pop_inlined_comprehension_state(struct compiler *c, location loc, if (k == NULL) { return ERROR; } - // preserve the built result of the comprehension; we could be - // cleverer about minimizing swaps but it doesn't matter because - // `apply_static_swaps` will eliminate all of these anyway + // preserve the list/dict/set result of the comprehension as TOS; we + // could be cleverer about minimizing swaps but it doesn't matter + // because `apply_static_swaps` will eliminate all of these anyway ADDOP_I(c, loc, SWAP, 2); ADDOP_NAME(c, loc, STORE_FAST, k, varnames); } From c1b54f03ded9e4b46aa3b64f904b268c034d6dd4 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Mon, 30 Jan 2023 16:39:45 -0800 Subject: [PATCH 04/35] enable inlining async comprehensions also --- Lib/test/test_compile.py | 6 +++--- .../2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst | 4 ++-- Python/symtable.c | 6 ++---- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index 05a5ed1fa9a637..573961e5653ad0 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -1363,7 +1363,7 @@ async def f(): compiled_code, _ = self.check_positions_against_ast(snippet) g = {} eval(compiled_code, g) - compiled_code = g['f'].__code__.co_consts[1] + compiled_code = g['f'].__code__ self.assertIsInstance(compiled_code, types.CodeType) self.assertOpcodeSourcePositionIs(compiled_code, 'LIST_APPEND', line=2, end_line=3, column=5, end_column=12, occurrence=1) @@ -1404,7 +1404,7 @@ async def f(): compiled_code, _ = self.check_positions_against_ast(snippet) g = {} eval(compiled_code, g) - compiled_code = g['f'].__code__.co_consts[1] + compiled_code = g['f'].__code__ self.assertIsInstance(compiled_code, types.CodeType) self.assertOpcodeSourcePositionIs(compiled_code, 'SET_ADD', line=2, end_line=3, column=5, end_column=12, occurrence=1) @@ -1445,7 +1445,7 @@ async def f(): compiled_code, _ = self.check_positions_against_ast(snippet) g = {} eval(compiled_code, g) - compiled_code = g['f'].__code__.co_consts[1] + compiled_code = g['f'].__code__ self.assertIsInstance(compiled_code, types.CodeType) self.assertOpcodeSourcePositionIs(compiled_code, 'MAP_ADD', line=2, end_line=3, column=5, end_column=11, occurrence=1) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst b/Misc/NEWS.d/next/Core and Builtins/2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst index 48fdca6cd438da..cf5985cc4bf703 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst @@ -1,2 +1,2 @@ -Inline (non-async) list, dict and set comprehensions to reduce performance -and bytecode size overhead. +Inline list, dict and set comprehensions to improve performance +and reduce bytecode size. diff --git a/Python/symtable.c b/Python/symtable.c index efbe8611204748..fb72d93e1ebc23 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -907,13 +907,11 @@ analyze_block(PySTEntryObject *ste, PyObject *bound, PyObject *free, assert(c && PySTEntry_Check(c)); entry = (PySTEntryObject*)c; - // we inline comprehensions if they are inside a function, are not a - // generator, and are not async + // we inline comprehensions if inside a function and not a generator int inline_comp = entry->ste_comprehension && ste->ste_type == FunctionBlock && - !entry->ste_generator && - !entry->ste_coroutine; + !entry->ste_generator; if (!analyze_child_block(entry, newbound, newfree, newglobal, &child_free)) From 43db9b891cd1803f3e27bd650d266123fdcb5c3e Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Mon, 30 Jan 2023 17:05:36 -0800 Subject: [PATCH 05/35] fix typo --- Python/symtable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/symtable.c b/Python/symtable.c index fb72d93e1ebc23..1dbdc9d6f1086c 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -618,7 +618,7 @@ inline_comprehension(PySTEntryObject *ste, PySTEntryObject *comp, } else { // free vars in comprehension that are locals in outer scope can // now simply be locals, unless they are free in comp children - if ((PyLong_AsLong(existing) && DEF_BOUND) && + if ((PyLong_AsLong(existing) & DEF_BOUND) && !is_free_in_children(comp, k)) { if (PySet_Discard(comp_free, k) < 0) { return 0; From aceb6c7431577ca41aec81552ddabd6b01ef5158 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 31 Jan 2023 13:35:56 -0800 Subject: [PATCH 06/35] fix outer-cell, inner-local case --- Lib/test/test_listcomps.py | 13 ++++++++++--- Python/compile.c | 25 +++++++++++++++---------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index 0fef1d55aec9e9..ee1c47c8fb82e3 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -176,15 +176,22 @@ Comprehensions' scopes don't interact with each other: - >>> def test_func(): - ... lst = list(range(3)) + >>> def test_func(lst): ... ret = [lambda: x for x in lst] ... inc = [x + 1 for x in lst] ... [x for x in inc] ... return ret - >>> test_func()[0]() + >>> test_func(range(3))[0]() 2 + >>> def test_func(lst): + ... x = -1 + ... funcs = [lambda: x for x in lst] + ... items = [x + 1 for x in lst] + ... return x + >>> test_func(range(3)) + -1 + """ diff --git a/Python/compile.c b/Python/compile.c index 6f63b3f6fce4e8..f19236a512809e 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -5396,10 +5396,10 @@ 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; DEF_LOCAL | DEF_NONLOCAL (oddly) 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 + // 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 if (symbol & DEF_LOCAL && ~symbol & DEF_NONLOCAL) { long scope = (symbol >> SCOPE_OFFSET) & SCOPE_MASK; PyObject *outv = PyDict_GetItemWithError(c->u->u_ste->ste_symbols, k); @@ -5408,11 +5408,15 @@ push_inlined_comprehension_state(struct compiler *c, location loc, } assert(PyLong_Check(outv)); long outsc = (PyLong_AS_LONG(outv) >> SCOPE_OFFSET) & SCOPE_MASK; - if (outsc == GLOBAL_IMPLICIT || outsc == GLOBAL_EXPLICIT) { - // if a name is global in the outer scope but local in the - // comprehension scope, we need to keep it global in outer - // scope but ensure the comprehension writes to the local, - // not the global; this is all the isolation we need. + int outer_global = (outsc == GLOBAL_IMPLICIT || outsc == GLOBAL_EXPLICIT); + if (outer_global || (outsc == CELL && scope == LOCAL)) { + // If a name is global in the outer scope but local in the + // comprehension scope, we need to keep it global in outer scope + // but ensure the comprehension writes to the local, not the + // global; this is all the isolation we need. If it's a cell in + // outer scope and a local inside the comprehension, we want the + // comprehension to treat it as a local, but we also need to + // save/restore the outer cell. if (state->temp_symbols == NULL) { state->temp_symbols = PyDict_New(); if (state->temp_symbols == NULL) { @@ -5430,7 +5434,8 @@ push_inlined_comprehension_state(struct compiler *c, location loc, return ERROR; } Py_DECREF(outv); - } else if (scope == LOCAL || scope == CELL) { + } + if (!outer_global && (scope == LOCAL || scope == CELL)) { // local names bound in comprehension must be isolated from // outer scope; push existing value (which may be NULL if // not defined) on stack From e57c35449601eef2fbd752425f76ae78ec0c72ef Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 31 Jan 2023 14:36:47 -0800 Subject: [PATCH 07/35] fix restoring NULL (unbound outer name) followed by load --- Include/internal/pycore_opcode.h | 3 ++- Include/opcode.h | 4 +++- Lib/opcode.py | 2 ++ Lib/test/test_listcomps.py | 12 ++++++++++++ Lib/test/test_tokenize.py | 2 ++ Python/compile.c | 13 +++++++++---- 6 files changed, 30 insertions(+), 6 deletions(-) diff --git a/Include/internal/pycore_opcode.h b/Include/internal/pycore_opcode.h index ea4c2ca259e04d..a54aea42afc1d1 100644 --- a/Include/internal/pycore_opcode.h +++ b/Include/internal/pycore_opcode.h @@ -228,7 +228,7 @@ const uint8_t _PyOpcode_Deopt[256] = { #endif // NEED_OPCODE_TABLES #ifdef Py_DEBUG -static const char *const _PyOpcode_OpName[263] = { +static const char *const _PyOpcode_OpName[264] = { [CACHE] = "CACHE", [POP_TOP] = "POP_TOP", [PUSH_NULL] = "PUSH_NULL", @@ -492,6 +492,7 @@ static const char *const _PyOpcode_OpName[263] = { [JUMP] = "JUMP", [JUMP_NO_INTERRUPT] = "JUMP_NO_INTERRUPT", [LOAD_METHOD] = "LOAD_METHOD", + [STORE_FAST_MAYBE_NULL] = "STORE_FAST_MAYBE_NULL", }; #endif diff --git a/Include/opcode.h b/Include/opcode.h index 03c9e48cf4264b..2dbefb5cdf6342 100644 --- a/Include/opcode.h +++ b/Include/opcode.h @@ -125,7 +125,8 @@ extern "C" { #define JUMP 260 #define JUMP_NO_INTERRUPT 261 #define LOAD_METHOD 262 -#define MAX_PSEUDO_OPCODE 262 +#define STORE_FAST_MAYBE_NULL 263 +#define MAX_PSEUDO_OPCODE 263 #define BINARY_OP_ADD_FLOAT 5 #define BINARY_OP_ADD_INT 6 #define BINARY_OP_ADD_UNICODE 7 @@ -193,6 +194,7 @@ extern "C" { || ((op) == JUMP) \ || ((op) == JUMP_NO_INTERRUPT) \ || ((op) == LOAD_METHOD) \ + || ((op) == STORE_FAST_MAYBE_NULL) \ ) #define HAS_CONST(op) (false\ diff --git a/Lib/opcode.py b/Lib/opcode.py index f04aab308fc981..1846d19739afe4 100644 --- a/Lib/opcode.py +++ b/Lib/opcode.py @@ -242,6 +242,8 @@ def pseudo_op(name, op, real_ops): pseudo_op('LOAD_METHOD', 262, ['LOAD_ATTR']) +pseudo_op('STORE_FAST_MAYBE_NULL', 263, ['STORE_FAST']) + MAX_PSEUDO_OPCODE = MIN_PSEUDO_OPCODE + len(_pseudo_ops) - 1 del def_op, name_op, jrel_op, jabs_op, pseudo_op diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index ee1c47c8fb82e3..32d880ad971a06 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -195,6 +195,18 @@ """ +class ListComprehensionTest(unittest.TestCase): + def test_unbound_local_after_comprehension(self): + def f(): + if False: + x = 0 + [x for x in [1]] + return x + + with self.assertRaises(UnboundLocalError): + f() + + __test__ = {'doctests' : doctests} def load_tests(loader, tests, pattern): diff --git a/Lib/test/test_tokenize.py b/Lib/test/test_tokenize.py index 63c2501cfe2338..35d7f8f5e69282 100644 --- a/Lib/test/test_tokenize.py +++ b/Lib/test/test_tokenize.py @@ -2538,6 +2538,8 @@ def test_continuation_lines_indentation(self): def get_tokens(string): return [(kind, string) for (kind, string, *_) in _generate_tokens_from_c_tokenizer(string)] + import dis; dis.dis(get_tokens) + code = dedent(""" def fib(n): \\ diff --git a/Python/compile.c b/Python/compile.c index f19236a512809e..1516a538d6590c 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -1232,6 +1232,7 @@ stack_effect(int opcode, int oparg, int jump) case LOAD_FAST_OR_NULL: return 1; case STORE_FAST: + case STORE_FAST_MAYBE_NULL: return -1; case DELETE_FAST: return 0; @@ -5486,7 +5487,7 @@ pop_inlined_comprehension_state(struct compiler *c, location loc, // could be cleverer about minimizing swaps but it doesn't matter // because `apply_static_swaps` will eliminate all of these anyway ADDOP_I(c, loc, SWAP, 2); - ADDOP_NAME(c, loc, STORE_FAST, k, varnames); + ADDOP_NAME(c, loc, STORE_FAST_MAYBE_NULL, k, varnames); } } return SUCCESS; @@ -7680,12 +7681,14 @@ push_cold_blocks_to_end(cfg_builder *g, int code_flags) { } static void -convert_exception_handlers_to_nops(basicblock *entryblock) { +convert_pseudo_ops(basicblock *entryblock) { for (basicblock *b = entryblock; b != NULL; b = b->b_next) { for (int i = 0; i < b->b_iused; i++) { struct instr *instr = &b->b_instr[i]; if (is_block_push(instr) || instr->i_opcode == POP_BLOCK) { INSTR_SET_OP0(instr, NOP); + } else if (instr->i_opcode == STORE_FAST_MAYBE_NULL) { + instr->i_opcode = STORE_FAST; } } } @@ -8969,7 +8972,7 @@ assemble(struct compiler *c, int addNone) } /* TO DO -- For 3.12, make sure that `maxdepth <= MAX_ALLOWED_STACK_USE` */ - convert_exception_handlers_to_nops(g->g_entryblock); + convert_pseudo_ops(g->g_entryblock); /* Order of basic blocks must have been determined by now */ if (normalize_jumps(g) < 0) { @@ -9236,7 +9239,9 @@ swaptimize(basicblock *block, int *ix) // - can't invoke arbitrary code (besides finalizers) // - only touch the TOS (and pop it when finished) #define SWAPPABLE(opcode) \ - ((opcode) == STORE_FAST || (opcode) == POP_TOP) + ((opcode) == STORE_FAST || \ + (opcode) == STORE_FAST_MAYBE_NULL || \ + (opcode) == POP_TOP) static int next_swappable_instruction(basicblock *block, int i, int lineno) From 795d854e575621fb91098b70ca38d5c044811d73 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 31 Jan 2023 15:05:33 -0800 Subject: [PATCH 08/35] emit 1 x SWAP N+1 instead of N x SWAP 2 --- Python/compile.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 1516a538d6590c..268b340267806b 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -5478,15 +5478,15 @@ pop_inlined_comprehension_state(struct compiler *c, location loc, } if (state.pushed_locals) { // pop names we pushed to stack earlier - for (Py_ssize_t i = PyList_GET_SIZE(state.pushed_locals); i > 0; --i) { - k = PyList_GetItem(state.pushed_locals, i - 1); + Py_ssize_t npops = PyList_GET_SIZE(state.pushed_locals); + // preserve the list/dict/set result of the comprehension as TOS + ADDOP_I(c, loc, SWAP, npops + 1); + for (Py_ssize_t i = npops; i > 0; --i) { + // i % npops: pop in order e.g. 0, 3, 2, 1: accounts for the swap + k = PyList_GetItem(state.pushed_locals, i % npops); if (k == NULL) { return ERROR; } - // preserve the list/dict/set result of the comprehension as TOS; we - // could be cleverer about minimizing swaps but it doesn't matter - // because `apply_static_swaps` will eliminate all of these anyway - ADDOP_I(c, loc, SWAP, 2); ADDOP_NAME(c, loc, STORE_FAST_MAYBE_NULL, k, varnames); } } From 686221a8c38c0a95d912aba4518cbcac333635fa Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 31 Jan 2023 15:08:16 -0800 Subject: [PATCH 09/35] remove stray dis.dis() call --- Lib/test/test_tokenize.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_tokenize.py b/Lib/test/test_tokenize.py index 35d7f8f5e69282..63c2501cfe2338 100644 --- a/Lib/test/test_tokenize.py +++ b/Lib/test/test_tokenize.py @@ -2538,8 +2538,6 @@ def test_continuation_lines_indentation(self): def get_tokens(string): return [(kind, string) for (kind, string, *_) in _generate_tokens_from_c_tokenizer(string)] - import dis; dis.dis(get_tokens) - code = dedent(""" def fib(n): \\ From ac99697d0a8a668311fcb5be546d9d8051a0f0bf Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 31 Jan 2023 15:11:16 -0800 Subject: [PATCH 10/35] fix compiler warning about Py_ssize_t -> int conversion --- Python/symtable.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/symtable.c b/Python/symtable.c index 1dbdc9d6f1086c..5d2726d607b1aa 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -808,8 +808,8 @@ analyze_block(PySTEntryObject *ste, PyObject *bound, PyObject *free, PyObject *name, *v, *local = NULL, *scopes = NULL, *newbound = NULL; PyObject *newglobal = NULL, *newfree = NULL; PyObject *temp; - int i, success = 0; - Py_ssize_t pos = 0; + int success = 0; + Py_ssize_t i, pos = 0; local = PySet_New(NULL); /* collect new names bound in block */ if (!local) From 46208568b2d5e5e2e6cd4b37baabe03b4905d8a4 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Wed, 1 Feb 2023 12:59:14 -0800 Subject: [PATCH 11/35] add a couple more tests --- Lib/test/test_listcomps.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index 32d880ad971a06..3c6a902c32bf1c 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -143,7 +143,7 @@ >>> test_func() [2, 2, 2, 2, 2] -A comprehension's iteration var, if in a cell, doesn't stomp on a previous value: +Some more tests for scoping edge cases: >>> def test_func(): ... y = 10 @@ -154,8 +154,6 @@ >>> test_func() (10, [4, 4, 4, 4, 4]) -A comprehension's iteration var doesn't shadow implicit globals for sibling scopes: - >>> g = -1 >>> def test_func(): ... def inner(): @@ -165,8 +163,6 @@ >>> test_func()() -1 -A modification to a closed-over variable is visible in the outer scope: - >>> def test_func(): ... x = -1 ... items = [(x:=y) for y in range(3)] @@ -174,8 +170,6 @@ >>> test_func() 2 -Comprehensions' scopes don't interact with each other: - >>> def test_func(lst): ... ret = [lambda: x for x in lst] ... inc = [x + 1 for x in lst] @@ -192,6 +186,11 @@ >>> test_func(range(3)) -1 + >>> def test_func(x): + ... return [x for x in x] + >>> test_func([1]) + [1] + """ @@ -206,6 +205,14 @@ def f(): with self.assertRaises(UnboundLocalError): f() + def test_unbound_local_inside_comprehension(self): + def f(): + l = [None] + return [1 for (l[0], l) in [[1, 2]]] + + with self.assertRaises(UnboundLocalError): + f() + __test__ = {'doctests' : doctests} From 8773653b4c078591d6c1ea28c1285b0922a775f6 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Wed, 1 Feb 2023 15:56:01 -0800 Subject: [PATCH 12/35] clear comp locals on entry, eval iter expr first --- Doc/library/dis.rst | 8 +- Include/internal/pycore_opcode.h | 4 +- Include/opcode.h | 2 +- Lib/opcode.py | 2 +- Python/bytecodes.c | 5 +- Python/compile.c | 148 +++++++++++++++++++------------ Python/generated_cases.c.h | 5 +- Python/opcode_metadata.h | 6 +- Python/opcode_targets.h | 2 +- 9 files changed, 106 insertions(+), 76 deletions(-) diff --git a/Doc/library/dis.rst b/Doc/library/dis.rst index e3ce47150bfa4d..2c9d868ebd872b 100644 --- a/Doc/library/dis.rst +++ b/Doc/library/dis.rst @@ -1213,13 +1213,11 @@ iterations of the loop. .. versionadded:: 3.12 -.. opcode:: LOAD_FAST_OR_NULL (var_num) +.. opcode:: LOAD_FAST_AND_CLEAR (var_num) - Pushes a reference to the local ``co_varnames[var_num]`` onto the stack, or + Pushes a reference to the local ``co_varnames[var_num]`` onto the stack (or pushes ``NULL`` onto the stack if the local variable has not been - initialized. This opcode has the same runtime effect as ``LOAD_FAST``; it - exists to maintain the invariant that ``LOAD_FAST`` will never load ``NULL`` - and may appear only where the variable is guaranteed to be initialized. + initialized) and sets ``co_varnames[var_num]`` to ``NULL``. .. versionadded:: 3.12 diff --git a/Include/internal/pycore_opcode.h b/Include/internal/pycore_opcode.h index a54aea42afc1d1..2a86c6b24d4b21 100644 --- a/Include/internal/pycore_opcode.h +++ b/Include/internal/pycore_opcode.h @@ -165,8 +165,8 @@ const uint8_t _PyOpcode_Deopt[256] = { [LOAD_CONST__LOAD_FAST] = LOAD_CONST, [LOAD_DEREF] = LOAD_DEREF, [LOAD_FAST] = LOAD_FAST, + [LOAD_FAST_AND_CLEAR] = LOAD_FAST_AND_CLEAR, [LOAD_FAST_CHECK] = LOAD_FAST_CHECK, - [LOAD_FAST_OR_NULL] = LOAD_FAST_OR_NULL, [LOAD_FAST__LOAD_CONST] = LOAD_FAST, [LOAD_FAST__LOAD_FAST] = LOAD_FAST, [LOAD_GLOBAL] = LOAD_GLOBAL, @@ -372,7 +372,7 @@ static const char *const _PyOpcode_OpName[264] = { [JUMP_BACKWARD] = "JUMP_BACKWARD", [COMPARE_AND_BRANCH] = "COMPARE_AND_BRANCH", [CALL_FUNCTION_EX] = "CALL_FUNCTION_EX", - [LOAD_FAST_OR_NULL] = "LOAD_FAST_OR_NULL", + [LOAD_FAST_AND_CLEAR] = "LOAD_FAST_AND_CLEAR", [EXTENDED_ARG] = "EXTENDED_ARG", [LIST_APPEND] = "LIST_APPEND", [SET_ADD] = "SET_ADD", diff --git a/Include/opcode.h b/Include/opcode.h index 2dbefb5cdf6342..9e0cc18c4345ea 100644 --- a/Include/opcode.h +++ b/Include/opcode.h @@ -97,7 +97,7 @@ extern "C" { #define JUMP_BACKWARD 140 #define COMPARE_AND_BRANCH 141 #define CALL_FUNCTION_EX 142 -#define LOAD_FAST_OR_NULL 143 +#define LOAD_FAST_AND_CLEAR 143 #define EXTENDED_ARG 144 #define LIST_APPEND 145 #define SET_ADD 146 diff --git a/Lib/opcode.py b/Lib/opcode.py index 1846d19739afe4..bc5cedef8ce197 100644 --- a/Lib/opcode.py +++ b/Lib/opcode.py @@ -196,7 +196,7 @@ def pseudo_op(name, op, real_ops): hascompare.append(141) def_op('CALL_FUNCTION_EX', 142) # Flags -def_op('LOAD_FAST_OR_NULL', 143) # Local variable number, may load NULL if undefined +def_op('LOAD_FAST_AND_CLEAR', 143) # Local variable number haslocal.append(143) def_op('EXTENDED_ARG', 144) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index c468e7cffbded8..4e8c6037909aa0 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -114,9 +114,10 @@ dummy_func( Py_INCREF(value); } - inst(LOAD_FAST_OR_NULL, (-- value)) { + inst(LOAD_FAST_AND_CLEAR, (-- value)) { value = GETLOCAL(oparg); - Py_XINCREF(value); + // do not use SETLOCAL here, it decrefs the old value + GETLOCAL(oparg) = NULL; } inst(LOAD_CONST, (-- value)) { diff --git a/Python/compile.c b/Python/compile.c index bc60931c07b9e6..5787155dbb7166 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -550,14 +550,14 @@ static int compiler_sync_comprehension_generator( asdl_comprehension_seq *generators, int gen_index, int depth, expr_ty elt, expr_ty val, int type, - int outermost_iter_is_param); + int iter_on_stack); static int compiler_async_comprehension_generator( struct compiler *c, location loc, asdl_comprehension_seq *generators, int gen_index, int depth, expr_ty elt, expr_ty val, int type, - int outermost_iter_is_param); + int iter_on_stack); static int compiler_pattern(struct compiler *, pattern_ty, pattern_context *); static int compiler_match(struct compiler *, stmt_ty); @@ -1229,7 +1229,7 @@ stack_effect(int opcode, int oparg, int jump) case LOAD_FAST: case LOAD_FAST_CHECK: - case LOAD_FAST_OR_NULL: + case LOAD_FAST_AND_CLEAR: return 1; case STORE_FAST: case STORE_FAST_MAYBE_NULL: @@ -5150,18 +5150,18 @@ compiler_comprehension_generator(struct compiler *c, location loc, asdl_comprehension_seq *generators, int gen_index, int depth, expr_ty elt, expr_ty val, int type, - int outermost_iter_is_param) + int iter_on_stack) { comprehension_ty gen; gen = (comprehension_ty)asdl_seq_GET(generators, gen_index); if (gen->is_async) { return compiler_async_comprehension_generator( c, loc, generators, gen_index, depth, elt, val, type, - outermost_iter_is_param); + iter_on_stack); } else { return compiler_sync_comprehension_generator( c, loc, generators, gen_index, depth, elt, val, type, - outermost_iter_is_param); + iter_on_stack); } } @@ -5170,7 +5170,7 @@ compiler_sync_comprehension_generator(struct compiler *c, location loc, asdl_comprehension_seq *generators, int gen_index, int depth, expr_ty elt, expr_ty val, int type, - int outermost_iter_is_param) + int iter_on_stack) { /* generate code for the iterator, then each of the ifs, and then write to the element */ @@ -5182,37 +5182,39 @@ compiler_sync_comprehension_generator(struct compiler *c, location loc, comprehension_ty gen = (comprehension_ty)asdl_seq_GET(generators, gen_index); - if (gen_index == 0 && outermost_iter_is_param) { - /* Receive outermost iter as an implicit argument */ - c->u->u_argcount = 1; - ADDOP_I(c, loc, LOAD_FAST, 0); - } - else { - /* Sub-iter - calculate on the fly */ - /* Fast path for the temporary variable assignment idiom: - for y in [f(x)] - */ - asdl_expr_seq *elts; - switch (gen->iter->kind) { - case List_kind: - elts = gen->iter->v.List.elts; - break; - case Tuple_kind: - elts = gen->iter->v.Tuple.elts; - break; - default: - elts = NULL; + if (!iter_on_stack) { + if (gen_index == 0) { + /* Receive outermost iter as an implicit argument */ + c->u->u_argcount = 1; + ADDOP_I(c, loc, LOAD_FAST, 0); } - if (asdl_seq_LEN(elts) == 1) { - expr_ty elt = asdl_seq_GET(elts, 0); - if (elt->kind != Starred_kind) { - VISIT(c, expr, elt); - start = NO_LABEL; + else { + /* Sub-iter - calculate on the fly */ + /* Fast path for the temporary variable assignment idiom: + for y in [f(x)] + */ + asdl_expr_seq *elts; + switch (gen->iter->kind) { + case List_kind: + elts = gen->iter->v.List.elts; + break; + case Tuple_kind: + elts = gen->iter->v.Tuple.elts; + break; + default: + elts = NULL; + } + if (asdl_seq_LEN(elts) == 1) { + expr_ty elt = asdl_seq_GET(elts, 0); + if (elt->kind != Starred_kind) { + VISIT(c, expr, elt); + start = NO_LABEL; + } + } + if (IS_LABEL(start)) { + VISIT(c, expr, gen->iter); + ADDOP(c, loc, GET_ITER); } - } - if (IS_LABEL(start)) { - VISIT(c, expr, gen->iter); - ADDOP(c, loc, GET_ITER); } } if (IS_LABEL(start)) { @@ -5287,7 +5289,7 @@ compiler_async_comprehension_generator(struct compiler *c, location loc, asdl_comprehension_seq *generators, int gen_index, int depth, expr_ty elt, expr_ty val, int type, - int outermost_iter_is_param) + int iter_on_stack) { NEW_JUMP_TARGET_LABEL(c, start); NEW_JUMP_TARGET_LABEL(c, except); @@ -5296,15 +5298,17 @@ compiler_async_comprehension_generator(struct compiler *c, location loc, comprehension_ty gen = (comprehension_ty)asdl_seq_GET(generators, gen_index); - if (gen_index == 0 && outermost_iter_is_param) { - /* Receive outermost iter as an implicit argument */ - c->u->u_argcount = 1; - ADDOP_I(c, loc, LOAD_FAST, 0); - } - else { - /* Sub-iter - calculate on the fly */ - VISIT(c, expr, gen->iter); - ADDOP(c, loc, GET_AITER); + if (!iter_on_stack) { + if (gen_index == 0) { + /* Receive outermost iter as an implicit argument */ + c->u->u_argcount = 1; + ADDOP_I(c, loc, LOAD_FAST, 0); + } + else { + /* Sub-iter - calculate on the fly */ + VISIT(c, expr, gen->iter); + ADDOP(c, loc, GET_AITER); + } } USE_LABEL(c, start); @@ -5449,7 +5453,7 @@ push_inlined_comprehension_state(struct compiler *c, location loc, // in the case of a cell, this will actually push the cell // itself to the stack, then we'll create a new one for the // comprehension and restore the original one after - ADDOP_NAME(c, loc, LOAD_FAST_OR_NULL, k, varnames); + ADDOP_NAME(c, loc, LOAD_FAST_AND_CLEAR, k, varnames); if (scope == CELL) { ADDOP_NAME(c, loc, MAKE_CELL, k, cellvars); } @@ -5459,6 +5463,14 @@ push_inlined_comprehension_state(struct compiler *c, location loc, } } } + if (state->pushed_locals) { + // Outermost iterable expression was already evaluated and is on the + // stack, we need to swap it back to TOS. This also rotates the order of + // `pushed_locals` on the stack, but this will be reversed when we swap + // out the comprehension result in pop_inlined_comprehension_state + ADDOP_I(c, loc, SWAP, PyList_GET_SIZE(state->pushed_locals) + 1); + } + return SUCCESS; } @@ -5479,11 +5491,13 @@ pop_inlined_comprehension_state(struct compiler *c, location loc, if (state.pushed_locals) { // pop names we pushed to stack earlier Py_ssize_t npops = PyList_GET_SIZE(state.pushed_locals); - // preserve the list/dict/set result of the comprehension as TOS + // Preserve the list/dict/set result of the comprehension as TOS. This + // reverses the SWAP we did in push_inlined_comprehension_state to get + // the outermost iterable to TOS, so we can still just iterate + // pushed_locals in simple reverse order ADDOP_I(c, loc, SWAP, npops + 1); - for (Py_ssize_t i = npops; i > 0; --i) { - // i % npops: pop in order e.g. 0, 3, 2, 1: accounts for the swap - k = PyList_GetItem(state.pushed_locals, i % npops); + for (Py_ssize_t i = npops - 1; i >= 0; --i) { + k = PyList_GetItem(state.pushed_locals, i); if (k == NULL) { return ERROR; } @@ -5493,6 +5507,19 @@ pop_inlined_comprehension_state(struct compiler *c, location loc, return SUCCESS; } +static inline int +compiler_comprehension_iter(struct compiler *c, location loc, + comprehension_ty comp) +{ + VISIT(c, expr, comp->iter); + if (comp->is_async) { + ADDOP(c, loc, GET_AITER); + } else { + ADDOP(c, loc, GET_ITER); + } + return SUCCESS; +} + static int compiler_comprehension(struct compiler *c, expr_ty e, int type, identifier name, asdl_comprehension_seq *generators, expr_ty elt, @@ -5516,6 +5543,9 @@ compiler_comprehension(struct compiler *c, expr_ty e, int type, outermost = (comprehension_ty) asdl_seq_GET(generators, 0); if (is_inlined) { + if (compiler_comprehension_iter(c, loc, outermost)) { + goto error; + } if (push_inlined_comprehension_state(c, loc, entry, &inline_state)) { goto error; } @@ -5557,10 +5587,13 @@ compiler_comprehension(struct compiler *c, expr_ty e, int type, } ADDOP_I(c, loc, op, 0); + if (is_inlined) { + ADDOP_I(c, loc, SWAP, 2); + } } if (compiler_comprehension_generator(c, loc, generators, 0, 0, - elt, val, type, !is_inlined) < 0) { + elt, val, type, is_inlined) < 0) { goto error_in_scope; } @@ -5595,13 +5628,8 @@ compiler_comprehension(struct compiler *c, expr_ty e, int type, } Py_DECREF(co); - VISIT(c, expr, outermost->iter); - - loc = LOC(e); - if (outermost->is_async) { - ADDOP(c, loc, GET_AITER); - } else { - ADDOP(c, loc, GET_ITER); + if (compiler_comprehension_iter(c, loc, outermost)) { + goto error; } ADDOP_I(c, loc, CALL, 0); @@ -8141,6 +8169,7 @@ scan_block_for_locals(basicblock *b, basicblock ***sp) uint64_t bit = (uint64_t)1 << instr->i_oparg; switch (instr->i_opcode) { case DELETE_FAST: + case LOAD_FAST_AND_CLEAR: unsafe_mask |= bit; break; case STORE_FAST: @@ -8194,6 +8223,7 @@ fast_scan_many_locals(basicblock *entryblock, int nlocals) assert(arg >= 0); switch (instr->i_opcode) { case DELETE_FAST: + case LOAD_FAST_AND_CLEAR: states[arg - 64] = blocknum - 1; break; case STORE_FAST: diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 59b3ef15d3ea0d..ba2f5a24754133 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -46,10 +46,11 @@ DISPATCH(); } - TARGET(LOAD_FAST_OR_NULL) { + TARGET(LOAD_FAST_AND_CLEAR) { PyObject *value; value = GETLOCAL(oparg); - Py_XINCREF(value); + // do not use SETLOCAL here, it decrefs the old value + GETLOCAL(oparg) = NULL; STACK_GROW(1); POKE(1, value); DISPATCH(); diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index 71021e75b08097..b0db2d565cb062 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -16,7 +16,7 @@ _PyOpcode_num_popped(int opcode, int oparg, bool jump) { return 0; case LOAD_FAST: return 0; - case LOAD_FAST_OR_NULL: + case LOAD_FAST_AND_CLEAR: return 0; case LOAD_CONST: return 0; @@ -364,7 +364,7 @@ _PyOpcode_num_pushed(int opcode, int oparg, bool jump) { return 1; case LOAD_FAST: return 1; - case LOAD_FAST_OR_NULL: + case LOAD_FAST_AND_CLEAR: return 1; case LOAD_CONST: return 1; @@ -711,7 +711,7 @@ struct opcode_metadata { [LOAD_CLOSURE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [LOAD_FAST_CHECK] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [LOAD_FAST] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, - [LOAD_FAST_OR_NULL] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, + [LOAD_FAST_AND_CLEAR] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [LOAD_CONST] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [STORE_FAST] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [LOAD_FAST__LOAD_FAST] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IBIB }, diff --git a/Python/opcode_targets.h b/Python/opcode_targets.h index e04df563c0df91..4252228ee5a159 100644 --- a/Python/opcode_targets.h +++ b/Python/opcode_targets.h @@ -142,7 +142,7 @@ static void *opcode_targets[256] = { &&TARGET_JUMP_BACKWARD, &&TARGET_COMPARE_AND_BRANCH, &&TARGET_CALL_FUNCTION_EX, - &&TARGET_LOAD_FAST_OR_NULL, + &&TARGET_LOAD_FAST_AND_CLEAR, &&TARGET_EXTENDED_ARG, &&TARGET_LIST_APPEND, &&TARGET_SET_ADD, From f0c051e94f27ea40ee0ceedd1c141e53a27c921d Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Thu, 9 Feb 2023 09:56:21 -0800 Subject: [PATCH 13/35] fix double decref in error case --- Python/compile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/compile.c b/Python/compile.c index 5d9be7fbd6d762..bdd113815c409a 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -5447,7 +5447,7 @@ compiler_comprehension(struct compiler *c, expr_ty e, int type, if (compiler_make_closure(c, loc, co, 0) < 0) { goto error; } - Py_DECREF(co); + Py_CLEAR(co); if (compiler_comprehension_iter(c, loc, outermost)) { goto error; From 142859a89d1656c8f3d8af09f95f7c73ec9ae475 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Thu, 9 Feb 2023 10:05:06 -0800 Subject: [PATCH 14/35] adjust to RETURN_CONST --- Lib/test/test_compile.py | 6 +++--- Lib/test/test_dis.py | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index 597606e89d51a7..41b4b1e0654d1b 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -1367,7 +1367,7 @@ async def f(): line=2, end_line=3, column=5, end_column=12, occurrence=1) self.assertOpcodeSourcePositionIs(compiled_code, 'JUMP_BACKWARD', line=2, end_line=3, column=5, end_column=12, occurrence=1) - self.assertOpcodeSourcePositionIs(compiled_code, 'RETURN_VALUE', + self.assertOpcodeSourcePositionIs(compiled_code, 'RETURN_CONST', line=2, end_line=7, column=4, end_column=36, occurrence=1) def test_multiline_set_comprehension(self): @@ -1408,7 +1408,7 @@ async def f(): line=2, end_line=3, column=5, end_column=12, occurrence=1) self.assertOpcodeSourcePositionIs(compiled_code, 'JUMP_BACKWARD', line=2, end_line=3, column=5, end_column=12, occurrence=1) - self.assertOpcodeSourcePositionIs(compiled_code, 'RETURN_VALUE', + self.assertOpcodeSourcePositionIs(compiled_code, 'RETURN_CONST', line=2, end_line=7, column=4, end_column=36, occurrence=1) def test_multiline_dict_comprehension(self): @@ -1449,7 +1449,7 @@ async def f(): line=2, end_line=3, column=5, end_column=11, occurrence=1) self.assertOpcodeSourcePositionIs(compiled_code, 'JUMP_BACKWARD', line=2, end_line=3, column=5, end_column=11, occurrence=1) - self.assertOpcodeSourcePositionIs(compiled_code, 'RETURN_VALUE', + self.assertOpcodeSourcePositionIs(compiled_code, 'RETURN_CONST', line=2, end_line=7, column=4, end_column=36, occurrence=1) def test_matchcase_sequence(self): diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index aef84774434599..408528f413a218 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -690,8 +690,7 @@ def foo(x): POP_TOP JUMP_BACKWARD 11 (to 10) >> END_FOR - LOAD_CONST 0 (None) - RETURN_VALUE + RETURN_CONST 0 (None) >> CALL_INTRINSIC_1 3 RERAISE 1 ExceptionTable: From 568a4707b0f9b5d77d83132704f37d8f2dcc6352 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Thu, 9 Feb 2023 21:19:54 -0800 Subject: [PATCH 15/35] fix up refcounting --- Python/compile.c | 5 +++++ Python/symtable.c | 1 + 2 files changed, 6 insertions(+) diff --git a/Python/compile.c b/Python/compile.c index bdd113815c409a..9fcb831758578e 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -5254,9 +5254,11 @@ push_inlined_comprehension_state(struct compiler *c, location loc, // comprehension Py_INCREF(outv); if (PyDict_SetItem(c->u->u_ste->ste_symbols, k, v)) { + Py_DECREF(outv); return ERROR; } if (PyDict_SetItem(state->temp_symbols, k, outv)) { + Py_DECREF(outv); return ERROR; } Py_DECREF(outv); @@ -5422,6 +5424,8 @@ compiler_comprehension(struct compiler *c, expr_ty e, int type, if (pop_inlined_comprehension_state(c, loc, inline_state)) { goto error; } + Py_XDECREF(inline_state.pushed_locals); + Py_XDECREF(inline_state.temp_symbols); return SUCCESS; } @@ -5468,6 +5472,7 @@ compiler_comprehension(struct compiler *c, expr_ty e, int type, } error: Py_XDECREF(co); + Py_XDECREF(entry); Py_XDECREF(inline_state.pushed_locals); Py_XDECREF(inline_state.temp_symbols); return ERROR; diff --git a/Python/symtable.c b/Python/symtable.c index 5d2726d607b1aa..3198856f043411 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -927,6 +927,7 @@ analyze_block(PySTEntryObject *ste, PyObject *bound, PyObject *free, Py_DECREF(child_free); if (!temp) goto error; + Py_DECREF(temp); /* Check if any children have free variables */ if (entry->ste_free || entry->ste_child_free) ste->ste_child_free = 1; From 17d5d840b5258add54e452a9bc5071c6650b7c79 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Thu, 9 Feb 2023 21:25:08 -0800 Subject: [PATCH 16/35] improve importlib comment --- Lib/importlib/_bootstrap_external.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index 0b94a8334ae500..fe71fd3f474cda 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -432,7 +432,7 @@ def _write_atomic(path, data, mode=0o666): # Python 3.12a5 3516 (Add COMPARE_AND_BRANCH instruction) # Python 3.12a5 3517 (Change YIELD_VALUE oparg to exception block depth) # Python 3.12a5 3518 (Add RETURN_CONST instruction) -# Python 3.12a5 3519 (Inline sync list/dict/set comprehensions in the calling function) +# Python 3.12a5 3519 (Inline list/dict/set comprehensions) # Python 3.13 will start with 3550 From b87d20983a5d1afa686a5a98f97fa374e1a984a4 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 10 Feb 2023 11:46:35 -0800 Subject: [PATCH 17/35] mark STORE_FAST_MAYBE_NULL as possibly NULLing a local Given the current usage of STORE_FAST_MAYBE_NULL, it isn't possible to construct a repro where this is needed, because if the local was NULL coming into the comprehension it will already be marked as unsafe coming out of the comprehension (since the body of the comprehension is always a loop which may be skipped entirely). But in case this opcode is used in other contexts in future, it should mark the local unsafe. --- Python/compile.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Python/compile.c b/Python/compile.c index 9fcb831758578e..5be61df91fc645 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -7996,6 +7996,7 @@ scan_block_for_locals(basicblock *b, basicblock ***sp) switch (instr->i_opcode) { case DELETE_FAST: case LOAD_FAST_AND_CLEAR: + case STORE_FAST_MAYBE_NULL: unsafe_mask |= bit; break; case STORE_FAST: @@ -8050,6 +8051,7 @@ fast_scan_many_locals(basicblock *entryblock, int nlocals) switch (instr->i_opcode) { case DELETE_FAST: case LOAD_FAST_AND_CLEAR: + case STORE_FAST_MAYBE_NULL: states[arg - 64] = blocknum - 1; break; case STORE_FAST: From 463c740b7199650fca15870b4ce583652970dd7f Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Mon, 27 Feb 2023 16:57:54 -0800 Subject: [PATCH 18/35] add test for NameError/UnboundLocalError --- Lib/test/test_listcomps.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index 3c6a902c32bf1c..3a6e62fe448724 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -205,6 +205,18 @@ def f(): with self.assertRaises(UnboundLocalError): f() + def test_nameerror(self): + def f(): + [x for x in [1]] + return x + + with self.assertRaises(NameError) as ctx: + f() + + # UnboundLocalError is a subtype of NameError; in this case we want to + # check that it is actually NameError + self.assertIs(type(ctx.exception), NameError) + def test_unbound_local_inside_comprehension(self): def f(): l = [None] From 67f50ba3766d7c52a9eceaff0e27ae6f9866304b Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 28 Feb 2023 13:32:32 -0800 Subject: [PATCH 19/35] fix case where iter var is free in outer scope --- Lib/test/test_listcomps.py | 10 ++++++++++ Python/compile.c | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index 3a6e62fe448724..e4d1c1900cd1f7 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -191,6 +191,16 @@ >>> test_func([1]) [1] + >>> def test_func(): + ... x = 1 + ... def g(): + ... [x for x in range(3)] + ... return x + ... g() + ... return x + >>> test_func() + 1 + """ diff --git a/Python/compile.c b/Python/compile.c index 545fd2f41972a5..16ff29ff33f173 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -5237,7 +5237,7 @@ push_inlined_comprehension_state(struct compiler *c, location loc, assert(PyLong_Check(outv)); long outsc = (PyLong_AS_LONG(outv) >> SCOPE_OFFSET) & SCOPE_MASK; int outer_global = (outsc == GLOBAL_IMPLICIT || outsc == GLOBAL_EXPLICIT); - if (outer_global || (outsc == CELL && scope == LOCAL)) { + if (outer_global || ((outsc == CELL || outsc == FREE) && scope == LOCAL)) { // If a name is global in the outer scope but local in the // comprehension scope, we need to keep it global in outer scope // but ensure the comprehension writes to the local, not the From 4109baab0351090bf3650cd56150a2e591a45d74 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Mon, 6 Mar 2023 17:48:33 -0800 Subject: [PATCH 20/35] add inlining of non-function-scope comprehensions --- Lib/test/test_compile.py | 12 ------------ Python/compile.c | 28 +++++++++++++++++++++++++--- Python/symtable.c | 2 -- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index 3de11001a18e9e..d1d791915bbbb4 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -1352,14 +1352,11 @@ def test_multiline_list_comprehension(self): and x != 50)] """) compiled_code, _ = self.check_positions_against_ast(snippet) - compiled_code = compiled_code.co_consts[0] self.assertIsInstance(compiled_code, types.CodeType) self.assertOpcodeSourcePositionIs(compiled_code, 'LIST_APPEND', line=1, end_line=2, column=1, end_column=8, occurrence=1) self.assertOpcodeSourcePositionIs(compiled_code, 'JUMP_BACKWARD', line=1, end_line=2, column=1, end_column=8, occurrence=1) - self.assertOpcodeSourcePositionIs(compiled_code, 'RETURN_VALUE', - line=1, end_line=6, column=0, end_column=32, occurrence=1) def test_multiline_async_list_comprehension(self): snippet = textwrap.dedent("""\ @@ -1393,14 +1390,11 @@ def test_multiline_set_comprehension(self): and x != 50)} """) compiled_code, _ = self.check_positions_against_ast(snippet) - compiled_code = compiled_code.co_consts[0] self.assertIsInstance(compiled_code, types.CodeType) self.assertOpcodeSourcePositionIs(compiled_code, 'SET_ADD', line=1, end_line=2, column=1, end_column=8, occurrence=1) self.assertOpcodeSourcePositionIs(compiled_code, 'JUMP_BACKWARD', line=1, end_line=2, column=1, end_column=8, occurrence=1) - self.assertOpcodeSourcePositionIs(compiled_code, 'RETURN_VALUE', - line=1, end_line=6, column=0, end_column=32, occurrence=1) def test_multiline_async_set_comprehension(self): snippet = textwrap.dedent("""\ @@ -1434,14 +1428,11 @@ def test_multiline_dict_comprehension(self): and x != 50)} """) compiled_code, _ = self.check_positions_against_ast(snippet) - compiled_code = compiled_code.co_consts[0] self.assertIsInstance(compiled_code, types.CodeType) self.assertOpcodeSourcePositionIs(compiled_code, 'MAP_ADD', line=1, end_line=2, column=1, end_column=7, occurrence=1) self.assertOpcodeSourcePositionIs(compiled_code, 'JUMP_BACKWARD', line=1, end_line=2, column=1, end_column=7, occurrence=1) - self.assertOpcodeSourcePositionIs(compiled_code, 'RETURN_VALUE', - line=1, end_line=6, column=0, end_column=32, occurrence=1) def test_multiline_async_dict_comprehension(self): snippet = textwrap.dedent("""\ @@ -1711,9 +1702,6 @@ def test_column_offset_deduplication(self): for source in [ "lambda: a", "(a for b in c)", - "[a for b in c]", - "{a for b in c}", - "{a: b for c in d}", ]: with self.subTest(source): code = compile(f"{source}, {source}", "", "eval") diff --git a/Python/compile.c b/Python/compile.c index 5882417bb31a2a..b5d12258cedcfc 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -439,6 +439,9 @@ struct compiler_unit { PyObject *u_cellvars; /* cell variables */ PyObject *u_freevars; /* free variables */ + // A set of names that should use fast-locals, even if not in a function */ + PyObject *u_fastlocals; + PyObject *u_private; /* for private name mangling */ Py_ssize_t u_argcount; /* number of arguments for block */ @@ -3975,7 +3978,9 @@ compiler_nameop(struct compiler *c, location loc, optype = OP_DEREF; break; case LOCAL: - if (c->u->u_ste->ste_type == FunctionBlock) + if (c->u->u_ste->ste_type == FunctionBlock || + (c->u->u_fastlocals && + PySet_Contains(c->u->u_fastlocals, mangled))) optype = OP_FAST; break; case GLOBAL_IMPLICIT: @@ -5154,6 +5159,16 @@ push_inlined_comprehension_state(struct compiler *c, location loc, // them from the outer scope as needed PyObject *k, *v; Py_ssize_t pos = 0; + assert(c->u->u_fastlocals == NULL); + if (c->u->u_ste->ste_type != FunctionBlock) { + // comprehension in non-function scope; for isolation, we'll need to + // override names bound in the comprehension to use fast locals, even + // though nothing else in this frame will + c->u->u_fastlocals = PySet_New(0); + if (c->u->u_fastlocals == NULL) { + return ERROR; + } + } while (PyDict_Next(entry->ste_symbols, &pos, &k, &v)) { assert(PyLong_Check(v)); long symbol = PyLong_AS_LONG(v); @@ -5162,6 +5177,12 @@ push_inlined_comprehension_state(struct compiler *c, location loc, // 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 (c->u->u_fastlocals) { + // non-function scope: override this name to use fast locals, + // and that's all we need + PySet_Add(c->u->u_fastlocals, k); + continue; + } long scope = (symbol >> SCOPE_OFFSET) & SCOPE_MASK; PyObject *outv = PyDict_GetItemWithError(c->u->u_ste->ste_symbols, k); if (outv == NULL) { @@ -5262,6 +5283,9 @@ pop_inlined_comprehension_state(struct compiler *c, location loc, ADDOP_NAME(c, loc, STORE_FAST_MAYBE_NULL, k, varnames); } } + if (c->u->u_fastlocals) { + Py_CLEAR(c->u->u_fastlocals); + } return SUCCESS; } @@ -5294,8 +5318,6 @@ compiler_comprehension(struct compiler *c, expr_ty e, int type, } int is_inlined = entry->ste_comp_inlined; int is_async_generator = entry->ste_coroutine; - // we only inline comprehensions in functions - assert(!is_inlined || c->u->u_ste->ste_type == FunctionBlock); location loc = LOC(e); diff --git a/Python/symtable.c b/Python/symtable.c index c25311a8659e34..df1c0cb24f5144 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -909,7 +909,6 @@ analyze_block(PySTEntryObject *ste, PyObject *bound, PyObject *free, // we inline comprehensions if inside a function and not a generator int inline_comp = entry->ste_comprehension && - ste->ste_type == FunctionBlock && !entry->ste_generator; if (!analyze_child_block(entry, newbound, newfree, newglobal, @@ -2306,4 +2305,3 @@ _Py_Mangle(PyObject *privateobj, PyObject *ident) assert(_PyUnicode_CheckConsistency(result, 1)); return result; } - From d8802a1d71e40364b89d213dab35a86671d7d6a8 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 7 Mar 2023 10:22:32 -0800 Subject: [PATCH 21/35] simplify scope handling --- Python/compile.c | 19 +++++++------------ Python/symtable.c | 25 +++++++------------------ 2 files changed, 14 insertions(+), 30 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index b5d12258cedcfc..7257668c03639e 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -5162,8 +5162,8 @@ push_inlined_comprehension_state(struct compiler *c, location loc, assert(c->u->u_fastlocals == NULL); if (c->u->u_ste->ste_type != FunctionBlock) { // comprehension in non-function scope; for isolation, we'll need to - // override names bound in the comprehension to use fast locals, even - // though nothing else in this frame will + // temporarily override names bound in the comprehension to use fast + // locals, even though nothing else in this frame will c->u->u_fastlocals = PySet_New(0); if (c->u->u_fastlocals == NULL) { return ERROR; @@ -5190,15 +5190,10 @@ push_inlined_comprehension_state(struct compiler *c, location loc, } assert(PyLong_Check(outv)); long outsc = (PyLong_AS_LONG(outv) >> SCOPE_OFFSET) & SCOPE_MASK; - int outer_global = (outsc == GLOBAL_IMPLICIT || outsc == GLOBAL_EXPLICIT); - if (outer_global || ((outsc == CELL || outsc == FREE) && scope == LOCAL)) { - // If a name is global in the outer scope but local in the - // comprehension scope, we need to keep it global in outer scope - // but ensure the comprehension writes to the local, not the - // global; this is all the isolation we need. If it's a cell in - // outer scope and a local inside the comprehension, we want the - // comprehension to treat it as a local, but we also need to - // save/restore the outer cell. + if (scope != outsc) { + // 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 (state->temp_symbols == NULL) { state->temp_symbols = PyDict_New(); if (state->temp_symbols == NULL) { @@ -5219,7 +5214,7 @@ push_inlined_comprehension_state(struct compiler *c, location loc, } Py_DECREF(outv); } - if (!outer_global && (scope == LOCAL || scope == CELL)) { + if (outsc == LOCAL || outsc == CELL || outsc == FREE) { // local names bound in comprehension must be isolated from // outer scope; push existing value (which may be NULL if // not defined) on stack diff --git a/Python/symtable.c b/Python/symtable.c index df1c0cb24f5144..33b7f335cfe3bc 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -605,23 +605,12 @@ inline_comprehension(PySTEntryObject *ste, PySTEntryObject *comp, } SET_SCOPE(scopes, k, scope); } else { - // name already exists in scope - PyObject *v_existing_scope = PyDict_GetItemWithError(scopes, k); - if (v_existing_scope == NULL) { - return 0; - } - long existing_scope = PyLong_AsLong(v_existing_scope); - // if name in comprehension was a cell, promote to cell - if (scope == CELL && existing_scope != CELL) { - SET_SCOPE(scopes, k, CELL); - } else { - // free vars in comprehension that are locals in outer scope can - // now simply be locals, unless they are free in comp children - if ((PyLong_AsLong(existing) & DEF_BOUND) && - !is_free_in_children(comp, k)) { - if (PySet_Discard(comp_free, k) < 0) { - return 0; - } + // free vars in comprehension that are locals in outer scope can + // now simply be locals, unless they are free in comp children + if ((PyLong_AsLong(existing) & DEF_BOUND) && + !is_free_in_children(comp, k)) { + if (PySet_Discard(comp_free, k) < 0) { + return 0; } } } @@ -906,7 +895,7 @@ analyze_block(PySTEntryObject *ste, PyObject *bound, PyObject *free, assert(c && PySTEntry_Check(c)); entry = (PySTEntryObject*)c; - // we inline comprehensions if inside a function and not a generator + // we inline all non-generator-expression comprehensions int inline_comp = entry->ste_comprehension && !entry->ste_generator; From b6a025ba83269a20c38cafb6ecd62846dba9f0fc Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Wed, 8 Mar 2023 12:00:11 -0800 Subject: [PATCH 22/35] add tests for comprehensions in class scope --- Lib/test/test_listcomps.py | 34 ++++++++++++++++++++++++++++++++-- Python/compile.c | 5 +---- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index e4d1c1900cd1f7..f227e9519af345 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -115,7 +115,7 @@ >>> [x() for x in items] [2, 2, 2, 2, 2] -We also repeat each of the above scoping tests inside a function +We also repeat each of the above scoping tests inside a function: >>> def test_func(): ... items = [(lambda i=i: i) for i in range(5)] @@ -143,7 +143,37 @@ >>> test_func() [2, 2, 2, 2, 2] -Some more tests for scoping edge cases: +And in class scope: + + >>> class C: + ... items = [(lambda i=i: i) for i in range(5)] + ... ret = [x() for x in items] + >>> C.ret + [0, 1, 2, 3, 4] + + >>> class C: + ... items = [(lambda: i) for i in range(5)] + ... ret = [x() for x in items] + >>> C.ret + [4, 4, 4, 4, 4] + + >>> class C: + ... items = [(lambda: i) for i in range(5)] + ... i = 20 + ... ret = [x() for x in items] + >>> C.ret + [4, 4, 4, 4, 4] + >>> C.i + 20 + + >>> class C: + ... items = [(lambda: y) for i in range(5)] + ... y = 2 + ... ret = [x() for x in items] + >>> C.ret + [2, 2, 2, 2, 2] + +Some more tests for scoping edge cases, each in func/module/class scope: >>> def test_func(): ... y = 10 diff --git a/Python/compile.c b/Python/compile.c index 4bd6d74e284253..bc608b7f2922ae 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -2670,7 +2670,6 @@ compiler_class(struct compiler *c, stmt_ty s) } else { /* No methods referenced __class__, so just return None */ - assert(PyDict_GET_SIZE(c->u->u_cellvars) == 0); ADDOP_LOAD_CONST(c, NO_LOCATION, Py_None); } ADDOP_IN_SCOPE(c, NO_LOCATION, RETURN_VALUE); @@ -5318,10 +5317,8 @@ push_inlined_comprehension_state(struct compiler *c, location loc, // need handling here since they shouldn't be isolated if (symbol & DEF_LOCAL && ~symbol & DEF_NONLOCAL) { if (c->u->u_fastlocals) { - // non-function scope: override this name to use fast locals, - // and that's all we need + // non-function scope: override this name to use fast locals PySet_Add(c->u->u_fastlocals, k); - continue; } long scope = (symbol >> SCOPE_OFFSET) & SCOPE_MASK; PyObject *outv = PyDict_GetItemWithError(c->u->u_ste->ste_symbols, k); From 90b34dea96d6304adb8a1abe2af001908086e63e Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Wed, 8 Mar 2023 13:22:13 -0800 Subject: [PATCH 23/35] run all listcomp scope tests in module, class, and func scope --- Lib/test/test_listcomps.py | 278 ++++++++++++++++++------------------- 1 file changed, 134 insertions(+), 144 deletions(-) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index f227e9519af345..a986fc285d8e54 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -1,4 +1,5 @@ import doctest +import textwrap import unittest @@ -87,154 +88,143 @@ >>> [None for i in range(10)] [None, None, None, None, None, None, None, None, None, None] -########### Tests for various scoping corner cases ############ - -Return lambdas that use the iteration variable as a default argument - - >>> items = [(lambda i=i: i) for i in range(5)] - >>> [x() for x in items] - [0, 1, 2, 3, 4] - -Same again, only this time as a closure variable - - >>> items = [(lambda: i) for i in range(5)] - >>> [x() for x in items] - [4, 4, 4, 4, 4] - -Another way to test that the iteration variable is local to the list comp - - >>> items = [(lambda: i) for i in range(5)] - >>> i = 20 - >>> [x() for x in items] - [4, 4, 4, 4, 4] - -And confirm that a closure can jump over the list comp scope - - >>> items = [(lambda: y) for i in range(5)] - >>> y = 2 - >>> [x() for x in items] - [2, 2, 2, 2, 2] - -We also repeat each of the above scoping tests inside a function: - - >>> def test_func(): - ... items = [(lambda i=i: i) for i in range(5)] - ... return [x() for x in items] - >>> test_func() - [0, 1, 2, 3, 4] - - >>> def test_func(): - ... items = [(lambda: i) for i in range(5)] - ... return [x() for x in items] - >>> test_func() - [4, 4, 4, 4, 4] - - >>> def test_func(): - ... items = [(lambda: i) for i in range(5)] - ... i = 20 - ... return [x() for x in items] - >>> test_func() - [4, 4, 4, 4, 4] - - >>> def test_func(): - ... items = [(lambda: y) for i in range(5)] - ... y = 2 - ... return [x() for x in items] - >>> test_func() - [2, 2, 2, 2, 2] - -And in class scope: - - >>> class C: - ... items = [(lambda i=i: i) for i in range(5)] - ... ret = [x() for x in items] - >>> C.ret - [0, 1, 2, 3, 4] - - >>> class C: - ... items = [(lambda: i) for i in range(5)] - ... ret = [x() for x in items] - >>> C.ret - [4, 4, 4, 4, 4] - - >>> class C: - ... items = [(lambda: i) for i in range(5)] - ... i = 20 - ... ret = [x() for x in items] - >>> C.ret - [4, 4, 4, 4, 4] - >>> C.i - 20 - - >>> class C: - ... items = [(lambda: y) for i in range(5)] - ... y = 2 - ... ret = [x() for x in items] - >>> C.ret - [2, 2, 2, 2, 2] - -Some more tests for scoping edge cases, each in func/module/class scope: - - >>> def test_func(): - ... y = 10 - ... items = [(lambda: y) for y in range(5)] - ... x = y - ... y = 20 - ... return x, [z() for z in items] - >>> test_func() - (10, [4, 4, 4, 4, 4]) - - >>> g = -1 - >>> def test_func(): - ... def inner(): - ... return g - ... [g for g in range(5)] - ... return inner - >>> test_func()() - -1 - - >>> def test_func(): - ... x = -1 - ... items = [(x:=y) for y in range(3)] - ... return x - >>> test_func() - 2 - - >>> def test_func(lst): - ... ret = [lambda: x for x in lst] - ... inc = [x + 1 for x in lst] - ... [x for x in inc] - ... return ret - >>> test_func(range(3))[0]() - 2 - - >>> def test_func(lst): - ... x = -1 - ... funcs = [lambda: x for x in lst] - ... items = [x + 1 for x in lst] - ... return x - >>> test_func(range(3)) - -1 - - >>> def test_func(x): - ... return [x for x in x] - >>> test_func([1]) - [1] - - >>> def test_func(): - ... x = 1 - ... def g(): - ... [x for x in range(3)] - ... return x - ... g() - ... return x - >>> test_func() - 1 - """ class ListComprehensionTest(unittest.TestCase): + def _check_in_scopes(self, code, outputs, ns=None, scopes=None): + code = textwrap.dedent(code) + scopes = scopes or ["module", "class", "function"] + for scope in scopes: + with self.subTest(scope=scope): + if scope == "class": + newcode = textwrap.dedent(""" + class C: + {code} + """).format(code=textwrap.indent(code, " ")) + def get_output(moddict, name): + return getattr(moddict["C"], name) + elif scope == "function": + newcode = textwrap.dedent(""" + def f(): + {code} + return locals() + """).format(code=textwrap.indent(code, " ")) + def get_output(moddict, name): + return moddict["f"]()[name] + else: + newcode = code + def get_output(moddict, name): + return moddict[name] + ns = ns or {} + exec(newcode, ns) + for k, v in outputs.items(): + self.assertEqual(get_output(ns, k), v) + + def test_lambdas_with_iteration_var_as_default(self): + code = """ + items = [(lambda i=i: i) for i in range(5)] + y = [x() for x in items] + """ + outputs = {"y": [0, 1, 2, 3, 4]} + self._check_in_scopes(code, outputs) + + def test_lambdas_with_free_var(self): + code = """ + items = [(lambda: i) for i in range(5)] + y = [x() for x in items] + """ + outputs = {"y": [4, 4, 4, 4, 4]} + self._check_in_scopes(code, outputs) + + def test_inner_cell_shadows_outer(self): + code = """ + items = [(lambda: i) for i in range(5)] + i = 20 + y = [x() for x in items] + """ + outputs = {"y": [4, 4, 4, 4, 4]} + self._check_in_scopes(code, outputs) + + def test_closure_can_jump_over_comp_scope(self): + code = """ + items = [(lambda: y) for i in range(5)] + y = 2 + z = [x() for x in items] + """ + outputs = {"z": [2, 2, 2, 2, 2]} + self._check_in_scopes(code, outputs) + + def test_inner_cell_shadows_outer_redefined(self): + code = """ + y = 10 + items = [(lambda: y) for y in range(5)] + x = y + y = 20 + out = [z() for z in items] + """ + outputs = {"x": 10, "out": [4, 4, 4, 4, 4]} + self._check_in_scopes(code, outputs) + + def test_shadows_outer_cell(self): + code = """ + def inner(): + return g + [g for g in range(5)] + x = inner() + """ + outputs = {"x": -1} + self._check_in_scopes(code, outputs, ns={"g": -1}) + + def test_assignment_expression(self): + code = """ + x = -1 + items = [(x:=y) for y in range(3)] + """ + outputs = {"x": 2} + # assignment expression in comprehension is disallowed in class scope + self._check_in_scopes(code, outputs, scopes=["module", "function"]) + + def test_free_var_in_comp_child(self): + code = """ + lst = range(3) + funcs = [lambda: x for x in lst] + inc = [x + 1 for x in lst] + [x for x in inc] + x = funcs[0]() + """ + outputs = {"x": 2} + self._check_in_scopes(code, outputs) + + def test_shadow_with_free_and_local(self): + code = """ + lst = range(3) + x = -1 + funcs = [lambda: x for x in lst] + items = [x + 1 for x in lst] + """ + outputs = {"x": -1} + self._check_in_scopes(code, outputs) + + def test_shadow_comp_iterable_name(self): + code = """ + x = [1] + y = [x for x in x] + """ + outputs = {"x": [1]} + self._check_in_scopes(code, outputs) + + def test_nested_free(self): + code = """ + x = 1 + def g(): + [x for x in range(3)] + return x + g() + """ + outputs = {"x": 1} + self._check_in_scopes(code, outputs) + def test_unbound_local_after_comprehension(self): def f(): if False: From b52046b8dd73f9d64d1f721af7b2f12155dba64c Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 14 Mar 2023 14:16:59 -0700 Subject: [PATCH 24/35] handle frame locals materialization in class/module scope --- Include/internal/pycore_code.h | 1 + Lib/test/test_listcomps.py | 21 ++++++++++++---- Objects/frameobject.c | 4 +++ Python/compile.c | 45 +++++++++++++++++++--------------- 4 files changed, 46 insertions(+), 25 deletions(-) diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h index 10f1e320a12ff4..6a55c7a9d9f51f 100644 --- a/Include/internal/pycore_code.h +++ b/Include/internal/pycore_code.h @@ -125,6 +125,7 @@ struct callable_cache { // Note that these all fit within a byte, as do combinations. // Later, we will use the smaller numbers to differentiate the different // kinds of locals (e.g. pos-only arg, varkwargs, local-only). +#define CO_FAST_HIDDEN 0x10 #define CO_FAST_LOCAL 0x20 #define CO_FAST_CELL 0x40 #define CO_FAST_FREE 0x80 diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index a986fc285d8e54..5ae22c6a48ea15 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -99,19 +99,20 @@ def _check_in_scopes(self, code, outputs, ns=None, scopes=None): with self.subTest(scope=scope): if scope == "class": newcode = textwrap.dedent(""" - class C: + class _C: {code} """).format(code=textwrap.indent(code, " ")) def get_output(moddict, name): - return getattr(moddict["C"], name) + return getattr(moddict["_C"], name) elif scope == "function": newcode = textwrap.dedent(""" - def f(): + def _f(): {code} return locals() + _out = _f() """).format(code=textwrap.indent(code, " ")) def get_output(moddict, name): - return moddict["f"]()[name] + return moddict["_out"][name] else: newcode = code def get_output(moddict, name): @@ -143,7 +144,7 @@ def test_inner_cell_shadows_outer(self): i = 20 y = [x() for x in items] """ - outputs = {"y": [4, 4, 4, 4, 4]} + outputs = {"y": [4, 4, 4, 4, 4], "i": 20} self._check_in_scopes(code, outputs) def test_closure_can_jump_over_comp_scope(self): @@ -225,6 +226,16 @@ def g(): outputs = {"x": 1} self._check_in_scopes(code, outputs) + def test_introspecting_frame_locals(self): + code = """ + import sys + [i for i in range(2)] + i = 20 + sys._getframe().f_locals + """ + outputs = {"i": 20} + self._check_in_scopes(code, outputs) + def test_unbound_local_after_comprehension(self): def f(): if False: diff --git a/Objects/frameobject.c b/Objects/frameobject.c index 133c991bf701c4..b5051790bcdd11 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -1208,6 +1208,10 @@ _PyFrame_FastToLocalsWithError(_PyInterpreterFrame *frame) } PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i); + _PyLocals_Kind kind = _PyLocals_GetKind(co->co_localspluskinds, i); + if (kind & CO_FAST_HIDDEN) { + continue; + } if (value == NULL) { if (PyObject_DelItem(locals, name) != 0) { if (PyErr_ExceptionMatches(PyExc_KeyError)) { diff --git a/Python/compile.c b/Python/compile.c index bc608b7f2922ae..9b4810228dde1a 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -630,9 +630,9 @@ struct compiler_unit { PyObject *u_cellvars; /* cell variables */ PyObject *u_freevars; /* free variables */ - // A set of names that should use fast-locals, even if not in a function */ - PyObject *u_fastlocals; - + PyObject *u_fasthidden; /* dict; keys are names that are fast-locals only + temporarily within an inlined comprehension. When + value is True, treat as fast-local. */ PyObject *u_private; /* for private name mangling */ Py_ssize_t u_argcount; /* number of arguments for block */ @@ -1000,6 +1000,7 @@ compiler_unit_free(struct compiler_unit *u) Py_CLEAR(u->u_varnames); Py_CLEAR(u->u_freevars); Py_CLEAR(u->u_cellvars); + Py_CLEAR(u->u_fasthidden); Py_CLEAR(u->u_private); PyObject_Free(u); } @@ -1671,6 +1672,12 @@ compiler_enter_scope(struct compiler *c, identifier name, return ERROR; } + u->u_fasthidden = PyDict_New(); + if (!u->u_fasthidden) { + compiler_unit_free(u); + return ERROR; + } + u->u_nfblocks = 0; u->u_firstlineno = lineno; u->u_consts = PyDict_New(); @@ -4118,8 +4125,7 @@ compiler_nameop(struct compiler *c, location loc, break; case LOCAL: if (c->u->u_ste->ste_type == FunctionBlock || - (c->u->u_fastlocals && - PySet_Contains(c->u->u_fastlocals, mangled))) + (PyDict_GetItem(c->u->u_fasthidden, mangled) == Py_True)) optype = OP_FAST; break; case GLOBAL_IMPLICIT: @@ -5298,16 +5304,6 @@ push_inlined_comprehension_state(struct compiler *c, location loc, // them from the outer scope as needed PyObject *k, *v; Py_ssize_t pos = 0; - assert(c->u->u_fastlocals == NULL); - if (c->u->u_ste->ste_type != FunctionBlock) { - // comprehension in non-function scope; for isolation, we'll need to - // temporarily override names bound in the comprehension to use fast - // locals, even though nothing else in this frame will - c->u->u_fastlocals = PySet_New(0); - if (c->u->u_fastlocals == NULL) { - return ERROR; - } - } while (PyDict_Next(entry->ste_symbols, &pos, &k, &v)) { assert(PyLong_Check(v)); long symbol = PyLong_AS_LONG(v); @@ -5316,9 +5312,9 @@ push_inlined_comprehension_state(struct compiler *c, location loc, // 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 (c->u->u_fastlocals) { + if (c->u->u_ste->ste_type != FunctionBlock) { // non-function scope: override this name to use fast locals - PySet_Add(c->u->u_fastlocals, k); + PyDict_SetItem(c->u->u_fasthidden, k, Py_True); } long scope = (symbol >> SCOPE_OFFSET) & SCOPE_MASK; PyObject *outv = PyDict_GetItemWithError(c->u->u_ste->ste_symbols, k); @@ -5392,7 +5388,6 @@ pop_inlined_comprehension_state(struct compiler *c, location loc, PyObject *k, *v; Py_ssize_t pos = 0; if (state.temp_symbols) { - // restore scope for globals that we temporarily set as locals while (PyDict_Next(state.temp_symbols, &pos, &k, &v)) { if (PyDict_SetItem(c->u->u_ste->ste_symbols, k, v)) { return ERROR; @@ -5415,8 +5410,15 @@ pop_inlined_comprehension_state(struct compiler *c, location loc, ADDOP_NAME(c, loc, STORE_FAST_MAYBE_NULL, k, varnames); } } - if (c->u->u_fastlocals) { - Py_CLEAR(c->u->u_fastlocals); + pos = 0; + while (PyDict_Next(c->u->u_fasthidden, &pos, &k, &v)) { + if (v == Py_True) { + // we set to False instead of clearing, so we can track which names + // were temporarily fast-locals and should use CO_FAST_HIDDEN + if (PyDict_SetItem(c->u->u_fasthidden, k, Py_False)) { + return ERROR; + } + } } return SUCCESS; } @@ -8333,6 +8335,9 @@ compute_localsplus_info(struct compiler *c, int nlocalsplus, assert(offset < nlocalsplus); // For now we do not distinguish arg kinds. _PyLocals_Kind kind = CO_FAST_LOCAL; + if (PyDict_Contains(c->u->u_fasthidden, k)) { + kind |= CO_FAST_HIDDEN; + } if (PyDict_GetItem(c->u->u_cellvars, k) != NULL) { kind |= CO_FAST_CELL; } From 51a12944d45567b5c807c2c1ca78db867e01eb1a Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 2 May 2023 14:32:37 -0700 Subject: [PATCH 25/35] update comment --- Python/flowgraph.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index fbaceef70edb3a..4cdd55b529e534 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -1643,7 +1643,8 @@ fast_scan_many_locals(basicblock *entryblock, int nlocals) Py_ssize_t blocknum = 0; // state[i - 64] == blocknum if local i is guaranteed to // be initialized, i.e., if it has had a previous LOAD_FAST or - // STORE_FAST within that basicblock (not followed by DELETE_FAST). + // STORE_FAST within that basicblock (not followed by + // DELETE_FAST/LOAD_FAST_AND_CLEAR/STORE_FAST_MAYBE_NULL). for (basicblock *b = entryblock; b != NULL; b = b->b_next) { blocknum++; for (int i = 0; i < b->b_iused; i++) { From 43722b4aadcdddde1b09cba8161dffdd78f8e5fa Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 5 May 2023 10:33:09 -0700 Subject: [PATCH 26/35] review comments --- Doc/whatsnew/3.12.rst | 24 ++++++++++ Lib/test/test_listcomps.py | 48 ++++++++++++------- ...3-01-30-15-40-29.gh-issue-97933.nUlp3r.rst | 2 +- Python/compile.c | 10 ++-- 4 files changed, 63 insertions(+), 21 deletions(-) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 4f952e2a37ef2f..dd93907f643efe 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -153,6 +153,30 @@ New Features In Python 3.14, the default will switch to ``'data'``. (Contributed by Petr Viktorin in :pep:`706`.) +.. _whatsnew312-pep709: + +PEP 709: Comprehension inlining +------------------------------- + +Dictionary, list, and set comprehensions are now inlined, rather than creating a +new single-use function object for each execution of the comprehension. This +speeds up execution of a comprehension by up to 2x. + +Comprehension iteration variables remain isolated; they don't overwrite a +variable of the same name in the outer scope, nor are they visible after the +comprehension. This isolation is now maintained via stack/locals manipulation, +not via separate function scope. + +Inlining does result in a few visible behavior changes: + +* There is no longer a separate frame for the comprehension in tracebacks, + and tracing/profiling no longer shows the comprehension as a function call. +* Calling :func:`locals` inside a comprehension now includes variables + from outside the comprehension, and no longer includes the synthetic `.0` + variable for the comprehension "argument". + +Contributed by Carl Meyer and Vladimir Matveev in :pep:`709`. + PEP 688: Making the buffer protocol accessible in Python -------------------------------------------------------- diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index 5ae22c6a48ea15..831f31f054cbcf 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -92,7 +92,7 @@ class ListComprehensionTest(unittest.TestCase): - def _check_in_scopes(self, code, outputs, ns=None, scopes=None): + def _check_in_scopes(self, code, outputs=None, ns=None, scopes=None, raises=()): code = textwrap.dedent(code) scopes = scopes or ["module", "class", "function"] for scope in scopes: @@ -118,9 +118,14 @@ def get_output(moddict, name): def get_output(moddict, name): return moddict[name] ns = ns or {} - exec(newcode, ns) - for k, v in outputs.items(): - self.assertEqual(get_output(ns, k), v) + try: + exec(newcode, ns) + except raises as e: + # We care about e.g. NameError vs UnboundLocalError + self.assertIs(type(e), raises) + else: + for k, v in (outputs or {}).items(): + self.assertEqual(get_output(ns, k), v) def test_lambdas_with_iteration_var_as_default(self): code = """ @@ -236,6 +241,29 @@ def test_introspecting_frame_locals(self): outputs = {"i": 20} self._check_in_scopes(code, outputs) + def test_nested(self): + code = """ + l = [1, 2, 3] + y = [x*2 for x in [x for x in l]] + """ + outputs = {"y": [2, 4, 6]} + self._check_in_scopes(code, outputs) + + def test_nameerror(self): + code = """ + [x for x in [1]] + x + """ + + self._check_in_scopes(code, raises=NameError) + + def test_dunder_name(self): + code = """ + y = [__x for __x in [1]] + """ + outputs = {"y": [1]} + self._check_in_scopes(code, outputs) + def test_unbound_local_after_comprehension(self): def f(): if False: @@ -246,18 +274,6 @@ def f(): with self.assertRaises(UnboundLocalError): f() - def test_nameerror(self): - def f(): - [x for x in [1]] - return x - - with self.assertRaises(NameError) as ctx: - f() - - # UnboundLocalError is a subtype of NameError; in this case we want to - # check that it is actually NameError - self.assertIs(type(ctx.exception), NameError) - def test_unbound_local_inside_comprehension(self): def f(): l = [None] diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst b/Misc/NEWS.d/next/Core and Builtins/2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst index cf5985cc4bf703..2eec05cb3ace5c 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst @@ -1,2 +1,2 @@ -Inline list, dict and set comprehensions to improve performance +:pep:`709`: inline list, dict and set comprehensions to improve performance and reduce bytecode size. diff --git a/Python/compile.c b/Python/compile.c index 5e8f5da5b4ad68..a15c61b1080965 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -5011,7 +5011,9 @@ push_inlined_comprehension_state(struct compiler *c, location loc, if (symbol & DEF_LOCAL && ~symbol & DEF_NONLOCAL) { if (c->u->u_ste->ste_type != FunctionBlock) { // non-function scope: override this name to use fast locals - PyDict_SetItem(c->u->u_metadata.u_fasthidden, k, Py_True); + if (PyDict_SetItem(c->u->u_metadata.u_fasthidden, k, Py_True) < 0) { + return ERROR; + } } long scope = (symbol >> SCOPE_OFFSET) & SCOPE_MASK; PyObject *outv = PyDict_GetItemWithError(c->u->u_ste->ste_symbols, k); @@ -5034,11 +5036,11 @@ push_inlined_comprehension_state(struct compiler *c, location loc, // 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)) { + 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)) { + if (PyDict_SetItem(state->temp_symbols, k, outv) < 0) { Py_DECREF(outv); return ERROR; } @@ -5061,7 +5063,7 @@ push_inlined_comprehension_state(struct compiler *c, location loc, if (scope == CELL) { ADDOP_NAME(c, loc, MAKE_CELL, k, cellvars); } - if (PyList_Append(state->pushed_locals, k)) { + if (PyList_Append(state->pushed_locals, k) < 0) { return ERROR; } } From bf9e1f1c4a63b800b7af5fe3d5a75cf110c6cb8e Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 5 May 2023 10:38:49 -0700 Subject: [PATCH 27/35] fix single backticks --- Doc/whatsnew/3.12.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index dd93907f643efe..e33e47d868ca82 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -172,7 +172,7 @@ Inlining does result in a few visible behavior changes: * There is no longer a separate frame for the comprehension in tracebacks, and tracing/profiling no longer shows the comprehension as a function call. * Calling :func:`locals` inside a comprehension now includes variables - from outside the comprehension, and no longer includes the synthetic `.0` + from outside the comprehension, and no longer includes the synthetic ``.0`` variable for the comprehension "argument". Contributed by Carl Meyer and Vladimir Matveev in :pep:`709`. From ca636a5635b4455d2c838d22c28ecaa33bebf83f Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 5 May 2023 10:46:30 -0700 Subject: [PATCH 28/35] better nested test --- Lib/test/test_listcomps.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index 831f31f054cbcf..598e225efc22aa 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -243,10 +243,10 @@ def test_introspecting_frame_locals(self): def test_nested(self): code = """ - l = [1, 2, 3] - y = [x*2 for x in [x for x in l]] + l = [2, 3] + y = [[x ** 2 for x in range(x)] for x in l] """ - outputs = {"y": [2, 4, 6]} + outputs = {"y": [[0, 1], [0, 1, 4]]} self._check_in_scopes(code, outputs) def test_nameerror(self): From 46c7a4f4eb61ad4882e1ede2e3ba6e1d6a43c68a Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 5 May 2023 11:35:29 -0700 Subject: [PATCH 29/35] fix u_fasthidden in nested case --- Lib/test/test_listcomps.py | 17 +++++++++++++++++ Python/compile.c | 32 +++++++++++++++++++++++++------- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index 598e225efc22aa..dde2424e8eedc3 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -249,6 +249,23 @@ def test_nested(self): outputs = {"y": [[0, 1], [0, 1, 4]]} self._check_in_scopes(code, outputs) + def test_nested_2(self): + code = """ + l = [1, 2, 3] + x = 3 + 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) + + def test_nested_3(self): + code = """ + l = [(1, 2), (3, 4), (5, 6)] + y = [x for (x, [x ** x for x in range(x)][x - 1]) in l] + """ + outputs = {"y": [1, 3, 5]} + self._check_in_scopes(code, outputs) + def test_nameerror(self): code = """ [x for x in [1]] diff --git a/Python/compile.c b/Python/compile.c index a15c61b1080965..a5e2db89a3003c 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -4990,6 +4990,7 @@ compiler_async_comprehension_generator(struct compiler *c, location loc, typedef struct { PyObject *pushed_locals; PyObject *temp_symbols; + PyObject *fast_hidden; } inlined_comprehension_state; static int @@ -5011,8 +5012,20 @@ push_inlined_comprehension_state(struct compiler *c, location loc, if (symbol & DEF_LOCAL && ~symbol & DEF_NONLOCAL) { if (c->u->u_ste->ste_type != FunctionBlock) { // non-function scope: override this name to use fast locals - if (PyDict_SetItem(c->u->u_metadata.u_fasthidden, k, Py_True) < 0) { - return ERROR; + PyObject *orig = PyDict_GetItem(c->u->u_metadata.u_fasthidden, k); + if (orig != Py_True) { + if (PyDict_SetItem(c->u->u_metadata.u_fasthidden, k, Py_True) < 0) { + return ERROR; + } + if (state->fast_hidden == NULL) { + state->fast_hidden = PySet_New(NULL); + if (state->fast_hidden == NULL) { + return ERROR; + } + } + if (PySet_Add(state->fast_hidden, k) < 0) { + return ERROR; + } } } long scope = (symbol >> SCOPE_OFFSET) & SCOPE_MASK; @@ -5092,6 +5105,7 @@ pop_inlined_comprehension_state(struct compiler *c, location loc, return ERROR; } } + Py_CLEAR(state.temp_symbols); } if (state.pushed_locals) { // pop names we pushed to stack earlier @@ -5108,16 +5122,21 @@ pop_inlined_comprehension_state(struct compiler *c, location loc, } ADDOP_NAME(c, loc, STORE_FAST_MAYBE_NULL, k, varnames); } + Py_CLEAR(state.pushed_locals); } - pos = 0; - while (PyDict_Next(c->u->u_metadata.u_fasthidden, &pos, &k, &v)) { - if (v == Py_True) { + if (state.fast_hidden) { + while (PySet_Size(state.fast_hidden) > 0) { + PyObject *k = PySet_Pop(state.fast_hidden); + if (k == NULL) { + return ERROR; + } // we set to False instead of clearing, so we can track which names // were temporarily fast-locals and should use CO_FAST_HIDDEN if (PyDict_SetItem(c->u->u_metadata.u_fasthidden, k, Py_False)) { return ERROR; } } + Py_CLEAR(state.fast_hidden); } return SUCCESS; } @@ -5214,8 +5233,6 @@ compiler_comprehension(struct compiler *c, expr_ty e, int type, if (pop_inlined_comprehension_state(c, loc, inline_state)) { goto error; } - Py_XDECREF(inline_state.pushed_locals); - Py_XDECREF(inline_state.temp_symbols); return SUCCESS; } @@ -5265,6 +5282,7 @@ compiler_comprehension(struct compiler *c, expr_ty e, int type, Py_XDECREF(entry); Py_XDECREF(inline_state.pushed_locals); Py_XDECREF(inline_state.temp_symbols); + Py_XDECREF(inline_state.fast_hidden); return ERROR; } From fb9f89ef6a7c8a497c9d23302285cfab5fb38ade Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 5 May 2023 14:59:56 -0700 Subject: [PATCH 30/35] fix refleak --- Python/compile.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Python/compile.c b/Python/compile.c index a5e2db89a3003c..85131bd1664b0d 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -5133,8 +5133,10 @@ pop_inlined_comprehension_state(struct compiler *c, location loc, // we set to False instead of clearing, so we can track which names // were temporarily fast-locals and should use CO_FAST_HIDDEN if (PyDict_SetItem(c->u->u_metadata.u_fasthidden, k, Py_False)) { + Py_DECREF(k); return ERROR; } + Py_DECREF(k); } Py_CLEAR(state.fast_hidden); } From 5914d7790984be4bdd192a711b87ee809984a3d9 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 5 May 2023 16:28:11 -0700 Subject: [PATCH 31/35] remove assumption that class scopes can't have cellvars --- Lib/test/test_listcomps.py | 11 +++++++++++ Python/compile.c | 5 +---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index dde2424e8eedc3..92fed98dd0004a 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -143,6 +143,17 @@ def test_lambdas_with_free_var(self): outputs = {"y": [4, 4, 4, 4, 4]} self._check_in_scopes(code, outputs) + def test_class_scope_free_var_with_class_cell(self): + class C: + def method(self): + super() + return __class__ + items = [(lambda: i) for i in range(5)] + y = [x() for x in items] + + self.assertEqual(C.y, [4, 4, 4, 4, 4]) + self.assertIs(C().method(), C) + def test_inner_cell_shadows_outer(self): code = """ items = [(lambda: i) for i in range(5)] diff --git a/Python/compile.c b/Python/compile.c index 85131bd1664b0d..112f741543901e 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -1245,9 +1245,7 @@ compiler_enter_scope(struct compiler *c, identifier name, /* Cook up an implicit __class__ cell. */ int res; assert(u->u_scope_type == COMPILER_SCOPE_CLASS); - assert(PyDict_GET_SIZE(u->u_metadata.u_cellvars) == 0); - res = PyDict_SetItem(u->u_metadata.u_cellvars, &_Py_ID(__class__), - _PyLong_GetZero()); + res = dict_add_o(u->u_metadata.u_cellvars, &_Py_ID(__class__)); if (res < 0) { compiler_unit_free(u); return ERROR; @@ -2245,7 +2243,6 @@ compiler_class(struct compiler *c, stmt_ty s) compiler_exit_scope(c); return ERROR; } - assert(i == 0); ADDOP_I(c, NO_LOCATION, LOAD_CLOSURE, i); ADDOP_I(c, NO_LOCATION, COPY, 1); if (compiler_nameop(c, NO_LOCATION, &_Py_ID(__classcell__), Store) < 0) { From ffae4e6783f35ebc50a56a5b3487b0ed8bd530a7 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Mon, 8 May 2023 23:19:32 -0600 Subject: [PATCH 32/35] Apply suggestions from code review Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> --- Python/compile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 112f741543901e..a9fdb19d962096 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -1243,7 +1243,7 @@ compiler_enter_scope(struct compiler *c, identifier name, } if (u->u_ste->ste_needs_class_closure) { /* Cook up an implicit __class__ cell. */ - int res; + Py_ssize_t res; assert(u->u_scope_type == COMPILER_SCOPE_CLASS); res = dict_add_o(u->u_metadata.u_cellvars, &_Py_ID(__class__)); if (res < 0) { @@ -5006,7 +5006,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)) { if (c->u->u_ste->ste_type != FunctionBlock) { // non-function scope: override this name to use fast locals PyObject *orig = PyDict_GetItem(c->u->u_metadata.u_fasthidden, k); From a8425a669953febb4e5278ab44464ed69f0f234a Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Mon, 8 May 2023 22:24:36 -0700 Subject: [PATCH 33/35] review comments --- Python/symtable.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/symtable.c b/Python/symtable.c index 33b7f335cfe3bc..b3398215ad01d9 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -560,7 +560,7 @@ analyze_name(PySTEntryObject *ste, PyObject *scopes, PyObject *name, long flags, } static int -is_free_in_children(PySTEntryObject *entry, PyObject *key) { +is_free_in_any_child(PySTEntryObject *entry, PyObject *key) { for (Py_ssize_t i = 0; i < PyList_GET_SIZE(entry->ste_children); i++) { PySTEntryObject *child_ste = (PySTEntryObject *)PyList_GET_ITEM( entry->ste_children, i); @@ -608,7 +608,7 @@ 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 ((PyLong_AsLong(existing) & DEF_BOUND) && - !is_free_in_children(comp, k)) { + !is_free_in_any_child(comp, k)) { if (PySet_Discard(comp_free, k) < 0) { return 0; } From 656e46bf7443f0a6a5dd0b68e8b8bdbdcf5dcbab Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 9 May 2023 07:08:11 -0600 Subject: [PATCH 34/35] Apply suggestions from code review Co-authored-by: Erlend E. Aasland --- Python/flowgraph.c | 3 ++- Python/symtable.c | 13 +++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 4cdd55b529e534..7f790b79d2844f 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -1989,7 +1989,8 @@ _PyCfg_ConvertPseudoOps(basicblock *entryblock) cfg_instr *instr = &b->b_instr[i]; if (is_block_push(instr) || instr->i_opcode == POP_BLOCK) { INSTR_SET_OP0(instr, NOP); - } else if (instr->i_opcode == STORE_FAST_MAYBE_NULL) { + } + else if (instr->i_opcode == STORE_FAST_MAYBE_NULL) { instr->i_opcode = STORE_FAST; } } diff --git a/Python/symtable.c b/Python/symtable.c index b3398215ad01d9..6e74d764245a57 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -560,7 +560,8 @@ analyze_name(PySTEntryObject *ste, PyObject *scopes, PyObject *name, long flags, } static int -is_free_in_any_child(PySTEntryObject *entry, PyObject *key) { +is_free_in_any_child(PySTEntryObject *entry, PyObject *key) +{ for (Py_ssize_t i = 0; i < PyList_GET_SIZE(entry->ste_children); i++) { PySTEntryObject *child_ste = (PySTEntryObject *)PyList_GET_ITEM( entry->ste_children, i); @@ -604,7 +605,8 @@ inline_comprehension(PySTEntryObject *ste, PySTEntryObject *comp, return 0; } SET_SCOPE(scopes, k, scope); - } else { + } + else { // free vars in comprehension that are locals in outer scope can // now simply be locals, unless they are free in comp children if ((PyLong_AsLong(existing) & DEF_BOUND) && @@ -902,7 +904,9 @@ analyze_block(PySTEntryObject *ste, PyObject *bound, PyObject *free, if (!analyze_child_block(entry, newbound, newfree, newglobal, &child_free)) + { goto error; + } if (inline_comp) { if (!inline_comprehension(ste, entry, scopes, child_free)) { Py_DECREF(child_free); @@ -927,8 +931,9 @@ analyze_block(PySTEntryObject *ste, PyObject *bound, PyObject *free, assert(c && PySTEntry_Check(c)); entry = (PySTEntryObject*)c; if (entry->ste_comp_inlined && - PyList_SetSlice(ste->ste_children, i, i + 1, - entry->ste_children) < 0) { + PyList_SetSlice(ste->ste_children, i, i + 1, + entry->ste_children) < 0) + { goto error; } } From 1402e7ae316dcf46db48c79ebe4f9ec711f30277 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 9 May 2023 07:09:32 -0600 Subject: [PATCH 35/35] Apply suggestions from code review Co-authored-by: Erlend E. Aasland --- Python/compile.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 60cc1f5d67ba87..e03aff0988cb0b 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -5147,7 +5147,8 @@ compiler_comprehension_iter(struct compiler *c, location loc, VISIT(c, expr, comp->iter); if (comp->is_async) { ADDOP(c, loc, GET_AITER); - } else { + } + else { ADDOP(c, loc, GET_ITER); } return SUCCESS; @@ -5180,7 +5181,8 @@ compiler_comprehension(struct compiler *c, expr_ty e, int type, if (push_inlined_comprehension_state(c, loc, entry, &inline_state)) { goto error; } - } else { + } + else { if (compiler_enter_scope(c, name, COMPILER_SCOPE_COMPREHENSION, (void *)e, e->lineno) < 0) {