From ae7c59dc25cdf21e193a52ee06f236f9d535fb8d Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 20 Oct 2022 18:44:30 +0100 Subject: [PATCH] gh-98461: Fix location of RETURN_VALUE in async generator bytecode. compiler_jump_if no longer needs a pointer to the loc --- Lib/test/test_compile.py | 2 +- Python/compile.c | 99 +++++++++++++++++----------------------- 2 files changed, 44 insertions(+), 57 deletions(-) diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index a59d692876b69a..27f91dbcd63aa5 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -1283,7 +1283,7 @@ def test_multiline_async_generator_expression(self): self.assertOpcodeSourcePositionIs(compiled_code, 'YIELD_VALUE', line=1, end_line=2, column=1, end_column=8, occurrence=2) self.assertOpcodeSourcePositionIs(compiled_code, 'RETURN_VALUE', - line=6, end_line=6, column=23, end_column=30, occurrence=1) + line=1, end_line=6, column=0, end_column=32, occurrence=1) def test_multiline_list_comprehension(self): snippet = """\ diff --git a/Python/compile.c b/Python/compile.c index 70f38d4d0cdac8..6dd7879904c168 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -2864,13 +2864,14 @@ static int compiler_addcompare(struct compiler *c, location loc, static int -compiler_jump_if(struct compiler *c, location *ploc, +compiler_jump_if(struct compiler *c, location loc, expr_ty e, jump_target_label next, int cond) { switch (e->kind) { case UnaryOp_kind: - if (e->v.UnaryOp.op == Not) - return compiler_jump_if(c, ploc, e->v.UnaryOp.operand, next, !cond); + if (e->v.UnaryOp.op == Not) { + return compiler_jump_if(c, loc, e->v.UnaryOp.operand, next, !cond); + } /* fallback to general implementation */ break; case BoolOp_kind: { @@ -2884,11 +2885,13 @@ compiler_jump_if(struct compiler *c, location *ploc, next2 = new_next2; } for (i = 0; i < n; ++i) { - if (!compiler_jump_if(c, ploc, (expr_ty)asdl_seq_GET(s, i), next2, cond2)) + if (!compiler_jump_if(c, loc, (expr_ty)asdl_seq_GET(s, i), next2, cond2)) { return 0; + } } - if (!compiler_jump_if(c, ploc, (expr_ty)asdl_seq_GET(s, n), next, cond)) + if (!compiler_jump_if(c, loc, (expr_ty)asdl_seq_GET(s, n), next, cond)) { return 0; + } if (!SAME_LABEL(next2, next)) { USE_LABEL(c, next2); } @@ -2897,45 +2900,46 @@ compiler_jump_if(struct compiler *c, location *ploc, case IfExp_kind: { NEW_JUMP_TARGET_LABEL(c, end); NEW_JUMP_TARGET_LABEL(c, next2); - if (!compiler_jump_if(c, ploc, e->v.IfExp.test, next2, 0)) + if (!compiler_jump_if(c, loc, e->v.IfExp.test, next2, 0)) { return 0; - if (!compiler_jump_if(c, ploc, e->v.IfExp.body, next, cond)) + } + if (!compiler_jump_if(c, loc, e->v.IfExp.body, next, cond)) { return 0; + } ADDOP_JUMP(c, NO_LOCATION, JUMP, end); USE_LABEL(c, next2); - if (!compiler_jump_if(c, ploc, e->v.IfExp.orelse, next, cond)) + if (!compiler_jump_if(c, loc, e->v.IfExp.orelse, next, cond)) { return 0; + } USE_LABEL(c, end); return 1; } case Compare_kind: { - SET_LOC(c, e); - *ploc = LOC(e); - Py_ssize_t i, n = asdl_seq_LEN(e->v.Compare.ops) - 1; + Py_ssize_t n = asdl_seq_LEN(e->v.Compare.ops) - 1; if (n > 0) { if (!check_compare(c, e)) { return 0; } NEW_JUMP_TARGET_LABEL(c, cleanup); VISIT(c, expr, e->v.Compare.left); - for (i = 0; i < n; i++) { + for (Py_ssize_t i = 0; i < n; i++) { VISIT(c, expr, (expr_ty)asdl_seq_GET(e->v.Compare.comparators, i)); - ADDOP_I(c, *ploc, SWAP, 2); - ADDOP_I(c, *ploc, COPY, 2); - ADDOP_COMPARE(c, *ploc, asdl_seq_GET(e->v.Compare.ops, i)); - ADDOP_JUMP(c, *ploc, POP_JUMP_IF_FALSE, cleanup); + ADDOP_I(c, LOC(e), SWAP, 2); + ADDOP_I(c, LOC(e), COPY, 2); + ADDOP_COMPARE(c, LOC(e), asdl_seq_GET(e->v.Compare.ops, i)); + ADDOP_JUMP(c, LOC(e), POP_JUMP_IF_FALSE, cleanup); } VISIT(c, expr, (expr_ty)asdl_seq_GET(e->v.Compare.comparators, n)); - ADDOP_COMPARE(c, *ploc, asdl_seq_GET(e->v.Compare.ops, n)); - ADDOP_JUMP(c, *ploc, cond ? POP_JUMP_IF_TRUE : POP_JUMP_IF_FALSE, next); + ADDOP_COMPARE(c, LOC(e), asdl_seq_GET(e->v.Compare.ops, n)); + ADDOP_JUMP(c, LOC(e), cond ? POP_JUMP_IF_TRUE : POP_JUMP_IF_FALSE, next); NEW_JUMP_TARGET_LABEL(c, end); ADDOP_JUMP(c, NO_LOCATION, JUMP, end); USE_LABEL(c, cleanup); - ADDOP(c, *ploc, POP_TOP); + ADDOP(c, LOC(e), POP_TOP); if (!cond) { ADDOP_JUMP(c, NO_LOCATION, JUMP, next); } @@ -2964,8 +2968,7 @@ compiler_ifexp(struct compiler *c, expr_ty e) NEW_JUMP_TARGET_LABEL(c, end); NEW_JUMP_TARGET_LABEL(c, next); - location loc = LOC(e); - if (!compiler_jump_if(c, &loc, e->v.IfExp.test, next, 0)) { + if (!compiler_jump_if(c, LOC(e), e->v.IfExp.test, next, 0)) { return 0; } VISIT(c, expr, e->v.IfExp.body); @@ -3050,8 +3053,7 @@ compiler_if(struct compiler *c, stmt_ty s) else { next = end; } - location loc = LOC(s); - if (!compiler_jump_if(c, &loc, s->v.If.test, next, 0)) { + if (!compiler_jump_if(c, LOC(s), s->v.If.test, next, 0)) { return 0; } VISIT_SEQ(c, stmt, s->v.If.body); @@ -3158,25 +3160,22 @@ compiler_async_for(struct compiler *c, stmt_ty s) static int compiler_while(struct compiler *c, stmt_ty s) { - location loc = LOC(s); NEW_JUMP_TARGET_LABEL(c, loop); NEW_JUMP_TARGET_LABEL(c, body); NEW_JUMP_TARGET_LABEL(c, end); NEW_JUMP_TARGET_LABEL(c, anchor); USE_LABEL(c, loop); - if (!compiler_push_fblock(c, loc, WHILE_LOOP, loop, end, NULL)) { + if (!compiler_push_fblock(c, LOC(s), WHILE_LOOP, loop, end, NULL)) { return 0; } - if (!compiler_jump_if(c, &loc, s->v.While.test, anchor, 0)) { + if (!compiler_jump_if(c, LOC(s), s->v.While.test, anchor, 0)) { return 0; } USE_LABEL(c, body); VISIT_SEQ(c, stmt, s->v.While.body); - SET_LOC(c, s); - loc = LOC(s); - if (!compiler_jump_if(c, &loc, s->v.While.test, body, 1)) { + if (!compiler_jump_if(c, LOC(s), s->v.While.test, body, 1)) { return 0; } @@ -3977,8 +3976,7 @@ compiler_assert(struct compiler *c, stmt_ty s) return 1; } NEW_JUMP_TARGET_LABEL(c, end); - location loc = LOC(s); - if (!compiler_jump_if(c, &loc, s->v.Assert.test, end, 1)) { + if (!compiler_jump_if(c, LOC(s), s->v.Assert.test, end, 1)) { return 0; } ADDOP(c, LOC(s), LOAD_ASSERTION_ERROR); @@ -4008,18 +4006,13 @@ compiler_stmt_expr(struct compiler *c, location loc, expr_ty value) } VISIT(c, expr, value); - /* Mark POP_TOP as artificial */ - UNSET_LOC(c); - ADDOP(c, NO_LOCATION, POP_TOP); + ADDOP(c, NO_LOCATION, POP_TOP); /* artificial */ return 1; } static int compiler_visit_stmt(struct compiler *c, stmt_ty s) { - Py_ssize_t i, n; - /* Always assign a lineno to the next instruction for a stmt. */ - SET_LOC(c, s); switch (s->kind) { case FunctionDef_kind: @@ -4033,12 +4026,11 @@ compiler_visit_stmt(struct compiler *c, stmt_ty s) break; case Assign_kind: { - n = asdl_seq_LEN(s->v.Assign.targets); + Py_ssize_t n = asdl_seq_LEN(s->v.Assign.targets); VISIT(c, expr, s->v.Assign.value); - location loc = LOC(s); - for (i = 0; i < n; i++) { + for (Py_ssize_t i = 0; i < n; i++) { if (i < n - 1) { - ADDOP_I(c, loc, COPY, 1); + ADDOP_I(c, LOC(s), COPY, 1); } VISIT(c, expr, (expr_ty)asdl_seq_GET(s->v.Assign.targets, i)); @@ -4059,7 +4051,7 @@ compiler_visit_stmt(struct compiler *c, stmt_ty s) return compiler_match(c, s); case Raise_kind: { - n = 0; + Py_ssize_t n = 0; if (s->v.Raise.exc) { VISIT(c, expr, s->v.Raise.exc); n++; @@ -4068,8 +4060,7 @@ compiler_visit_stmt(struct compiler *c, stmt_ty s) n++; } } - location loc = LOC(s); - ADDOP_I(c, loc, RAISE_VARARGS, (int)n); + ADDOP_I(c, LOC(s), RAISE_VARARGS, (int)n); break; } case Try_kind: @@ -4087,24 +4078,20 @@ compiler_visit_stmt(struct compiler *c, stmt_ty s) break; case Expr_kind: { - location loc = LOC(s); - return compiler_stmt_expr(c, loc, s->v.Expr.value); + return compiler_stmt_expr(c, LOC(s), s->v.Expr.value); } case Pass_kind: { - location loc = LOC(s); - ADDOP(c, loc, NOP); + ADDOP(c, LOC(s), NOP); break; } case Break_kind: { - location loc = LOC(s); - return compiler_break(c, loc); + return compiler_break(c, LOC(s)); } case Continue_kind: { - location loc = LOC(s); - return compiler_continue(c, loc); + return compiler_continue(c, LOC(s)); } case With_kind: return compiler_with(c, s, 0); @@ -5266,7 +5253,7 @@ compiler_sync_comprehension_generator(struct compiler *c, location loc, Py_ssize_t n = asdl_seq_LEN(gen->ifs); for (Py_ssize_t i = 0; i < n; i++) { expr_ty e = (expr_ty)asdl_seq_GET(gen->ifs, i); - if (!compiler_jump_if(c, &loc, e, if_cleanup, 0)) { + if (!compiler_jump_if(c, loc, e, if_cleanup, 0)) { return 0; } } @@ -5365,7 +5352,7 @@ compiler_async_comprehension_generator(struct compiler *c, location loc, Py_ssize_t n = asdl_seq_LEN(gen->ifs); for (Py_ssize_t i = 0; i < n; i++) { expr_ty e = (expr_ty)asdl_seq_GET(gen->ifs, i); - if (!compiler_jump_if(c, &loc, e, if_cleanup, 0)) { + if (!compiler_jump_if(c, loc, e, if_cleanup, 0)) { return 0; } } @@ -7100,7 +7087,7 @@ compiler_match_inner(struct compiler *c, stmt_ty s, pattern_context *pc) // NOTE: Returning macros are safe again. if (m->guard) { RETURN_IF_FALSE(ensure_fail_pop(c, pc, 0)); - RETURN_IF_FALSE(compiler_jump_if(c, &loc, m->guard, pc->fail_pop[0], 0)); + RETURN_IF_FALSE(compiler_jump_if(c, loc, m->guard, pc->fail_pop[0], 0)); } // Success! Pop the subject off, we're done with it: if (i != cases - has_default - 1) { @@ -7129,7 +7116,7 @@ compiler_match_inner(struct compiler *c, stmt_ty s, pattern_context *pc) ADDOP(c, loc, NOP); } if (m->guard) { - RETURN_IF_FALSE(compiler_jump_if(c, &loc, m->guard, end, 0)); + RETURN_IF_FALSE(compiler_jump_if(c, loc, m->guard, end, 0)); } VISIT_SEQ(c, stmt, m->body); }