Skip to content

Commit 416779a

Browse files
committed
Make sure that line number is correct after a return, as defined by PEP 626.
1 parent 7301979 commit 416779a

File tree

3 files changed

+160
-11
lines changed

3 files changed

+160
-11
lines changed

Lib/test/test_compile.py

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ def test_leading_newlines(self):
156156
s256 = "".join(["\n"] * 256 + ["spam"])
157157
co = compile(s256, 'fn', 'exec')
158158
self.assertEqual(co.co_firstlineno, 1)
159-
self.assertEqual(list(co.co_lines()), [(0, 4, 257), (4, 8, None)])
159+
self.assertEqual(list(co.co_lines()), [(0, 8, 257)])
160160

161161
def test_literals_with_leading_zeroes(self):
162162
for arg in ["077787", "0xj", "0x.", "0e", "090000000000000",
@@ -775,6 +775,39 @@ def or_false(x):
775775
self.assertIn('LOAD_', opcodes[0].opname)
776776
self.assertEqual('RETURN_VALUE', opcodes[1].opname)
777777

778+
def test_lineno_after_implicit_return(self):
779+
TRUE = True
780+
# Don't use constant True or False, as compiler will remove test
781+
def if1(x):
782+
x()
783+
if TRUE:
784+
pass
785+
def if2(x):
786+
x()
787+
if TRUE:
788+
pass
789+
else:
790+
pass
791+
def if3(x):
792+
x()
793+
if TRUE:
794+
pass
795+
else:
796+
return None
797+
def if4(x):
798+
x()
799+
if not TRUE:
800+
pass
801+
funcs = [ if1, if2, if3, if4]
802+
lastlines = [ 3, 3, 3, 2]
803+
frame = None
804+
def save_caller_frame():
805+
nonlocal frame
806+
frame = sys._getframe(1)
807+
for func, lastline in zip(funcs, lastlines, strict=True):
808+
with self.subTest(func=func):
809+
func(save_caller_frame)
810+
self.assertEqual(frame.f_lineno-frame.f_code.co_firstlineno, lastline)
778811

779812
def test_big_dict_literal(self):
780813
# The compiler has a flushing point in "compiler_dict" that calls compiles

Lib/test/test_dis.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,13 +295,15 @@ def bug1333982(x=[]):
295295
52 STORE_FAST 0 (e)
296296
54 DELETE_FAST 0 (e)
297297
56 RERAISE
298-
>> 58 RERAISE
298+
299+
%3d >> 58 RERAISE
299300
""" % (TRACEBACK_CODE.co_firstlineno + 1,
300301
TRACEBACK_CODE.co_firstlineno + 2,
301302
TRACEBACK_CODE.co_firstlineno + 5,
302303
TRACEBACK_CODE.co_firstlineno + 3,
303304
TRACEBACK_CODE.co_firstlineno + 4,
304-
TRACEBACK_CODE.co_firstlineno + 5)
305+
TRACEBACK_CODE.co_firstlineno + 5,
306+
TRACEBACK_CODE.co_firstlineno + 3)
305307

306308
def _fstring(a, b, c, d):
307309
return f'{a} {b:4} {c!r} {d!r:4}'

Python/compile.c

Lines changed: 122 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,28 @@ compiler_use_next_block(struct compiler *c, basicblock *block)
812812
return block;
813813
}
814814

815+
static basicblock *
816+
compiler_copy_block(struct compiler *c, basicblock *block)
817+
{
818+
/* Cannot copy a block if it has a fallthrough, since
819+
* a block can only have one fallthrough predecessor.
820+
*/
821+
assert(block->b_nofallthrough);
822+
basicblock *result = compiler_next_block(c);
823+
if (result == NULL) {
824+
return NULL;
825+
}
826+
for(int i = 0; i < block->b_iused; i++) {
827+
int n = compiler_next_instr(result);
828+
if (n < 0) {
829+
return NULL;
830+
}
831+
result->b_instr[n] = block->b_instr[i];
832+
}
833+
result->b_exit = block->b_exit;
834+
return result;
835+
}
836+
815837
/* Returns the offset of the next instruction in the current block's
816838
b_instr array. Resizes the b_instr as necessary.
817839
Returns -1 on failure.
@@ -5929,9 +5951,16 @@ dump_basicblock(const basicblock *b)
59295951
}
59305952
#endif
59315953

5954+
5955+
static int
5956+
normalize_basic_block(basicblock *bb) ;
5957+
59325958
static int
59335959
optimize_cfg(struct assembler *a, PyObject *consts);
59345960

5961+
static int
5962+
ensure_exits_have_lineno(struct compiler *c);
5963+
59355964
static PyCodeObject *
59365965
assemble(struct compiler *c, int addNone)
59375966
{
@@ -5952,6 +5981,16 @@ assemble(struct compiler *c, int addNone)
59525981
ADDOP(c, RETURN_VALUE);
59535982
}
59545983

5984+
for (b = c->u->u_blocks; b != NULL; b = b->b_list) {
5985+
if (normalize_basic_block(b)) {
5986+
goto error;
5987+
}
5988+
}
5989+
5990+
if (ensure_exits_have_lineno(c)) {
5991+
goto error;
5992+
}
5993+
59555994
nblocks = 0;
59565995
entryblock = NULL;
59575996
for (b = c->u->u_blocks; b != NULL; b = b->b_list) {
@@ -5966,6 +6005,7 @@ assemble(struct compiler *c, int addNone)
59666005
else
59676006
c->u->u_firstlineno = 1;
59686007
}
6008+
59696009
if (!assemble_init(&a, nblocks, c->u->u_firstlineno))
59706010
goto error;
59716011
a.a_entry = entryblock;
@@ -6338,7 +6378,6 @@ clean_basic_block(basicblock *bb) {
63386378
bb->b_iused = dest;
63396379
}
63406380

6341-
63426381
static int
63436382
normalize_basic_block(basicblock *bb) {
63446383
/* Mark blocks as exit and/or nofallthrough.
@@ -6367,7 +6406,6 @@ normalize_basic_block(basicblock *bb) {
63676406
return 0;
63686407
}
63696408

6370-
63716409
static int
63726410
mark_reachable(struct assembler *a) {
63736411
basicblock **stack, **sp;
@@ -6398,8 +6436,27 @@ mark_reachable(struct assembler *a) {
63986436
return 0;
63996437
}
64006438

6439+
/* If an instruction has no line number, but it's predecessor in the BB does,
6440+
* then copy the line number. This reduces the size of the line number table,
6441+
* but has no impact on the generated line number events.
6442+
*/
6443+
static void
6444+
minimize_lineno_table(struct assembler *a) {
6445+
for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) {
6446+
int prev_lineno = -1;
6447+
for (int i = 0; i < b->b_iused; i++) {
6448+
if (b->b_instr[i].i_lineno < 0) {
6449+
b->b_instr[i].i_lineno = prev_lineno;
6450+
}
6451+
else {
6452+
prev_lineno = b->b_instr[i].i_lineno;
6453+
}
6454+
}
6455+
6456+
}
6457+
}
64016458

6402-
/* Perform basic peephole optimizations on a control flow graph.
6459+
/* Perform optimizations on a control flow graph.
64036460
The consts object should still be in list form to allow new constants
64046461
to be appended.
64056462
@@ -6411,11 +6468,6 @@ mark_reachable(struct assembler *a) {
64116468
static int
64126469
optimize_cfg(struct assembler *a, PyObject *consts)
64136470
{
6414-
for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) {
6415-
if (normalize_basic_block(b)) {
6416-
return -1;
6417-
}
6418-
}
64196471
for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) {
64206472
if (optimize_basic_block(b, consts)) {
64216473
return -1;
@@ -6432,9 +6484,71 @@ optimize_cfg(struct assembler *a, PyObject *consts)
64326484
b->b_iused = 0;
64336485
}
64346486
}
6487+
minimize_lineno_table(a);
64356488
return 0;
64366489
}
64376490

6491+
static int
6492+
is_exit_without_lineno(basicblock *b) {
6493+
if (!b->b_exit) {
6494+
return 0;
6495+
}
6496+
for (int i = 0; i < b->b_iused; i++) {
6497+
if (b->b_instr[i].i_lineno >= 0) {
6498+
return 0;
6499+
}
6500+
}
6501+
return 1;
6502+
}
6503+
6504+
/* PEP 626 mandates that the f_lineno of a frame is correct
6505+
* after a frame terminates. It would be prohibitively expensive
6506+
* to continuously update the f_lineno field at runtime,
6507+
* so we make sure that all exiting instruction (raises and returns)
6508+
* have a valid line number, allowing us to compute f_lineno lazily.
6509+
* We can do this by duplicating the exit blocks without line number
6510+
* so that none have more than one predecessor. We can then safely
6511+
* copy the line number from the sole predecessor block.
6512+
*/
6513+
static int
6514+
ensure_exits_have_lineno(struct compiler *c)
6515+
{
6516+
/* Copy all exit blocks without line number that are targets of a jump.
6517+
*/
6518+
for (basicblock *b = c->u->u_blocks; b != NULL; b = b->b_list) {
6519+
if (b->b_iused > 0 && is_jump(&b->b_instr[b->b_iused-1])) {
6520+
switch(b->b_instr[b->b_iused-1].i_opcode) {
6521+
/* Note: Only actual jumps, not exception handlers */
6522+
case SETUP_ASYNC_WITH:
6523+
case SETUP_WITH:
6524+
case SETUP_FINALLY:
6525+
continue;
6526+
}
6527+
basicblock *target = b->b_instr[b->b_iused-1].i_target;
6528+
if (is_exit_without_lineno(target)) {
6529+
basicblock *new_target = compiler_copy_block(c, target);
6530+
if (new_target == NULL) {
6531+
return -1;
6532+
}
6533+
new_target->b_instr[0].i_lineno = b->b_instr[b->b_iused-1].i_lineno;
6534+
b->b_instr[b->b_iused-1].i_target = new_target;
6535+
}
6536+
}
6537+
}
6538+
/* Any remaining reachable exit blocks without line number can only be reached by
6539+
* fall through, and thus can only have a single predecessor */
6540+
for (basicblock *b = c->u->u_blocks; b != NULL; b = b->b_list) {
6541+
if (!b->b_nofallthrough && b->b_next && b->b_iused > 0) {
6542+
if (is_exit_without_lineno(b->b_next)) {
6543+
assert(b->b_next->b_iused > 0);
6544+
b->b_next->b_instr[0].i_lineno = b->b_instr[b->b_iused-1].i_lineno;
6545+
}
6546+
}
6547+
}
6548+
return 0;
6549+
}
6550+
6551+
64386552
/* Retained for API compatibility.
64396553
* Optimization is now done in optimize_cfg */
64406554

0 commit comments

Comments
 (0)