Further SET_LINENO reomval fixes. See comments in patch #587933.
Use a slightly different strategy to determine when not to call the line
trace function. This removes the need for the RETURN_NONE opcode, so
that's gone again. Update docs and comments to match.
Thanks to Neal and Armin!
Also add a test suite. This should have come with the original patch...
diff --git a/Python/ceval.c b/Python/ceval.c
index d4be04e..8bd945e 100644
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -1515,12 +1515,6 @@
why = WHY_RETURN;
break;
- case RETURN_NONE:
- retval = Py_None;
- Py_INCREF(retval);
- why = WHY_RETURN;
- break;
-
case YIELD_VALUE:
retval = POP();
f->f_stacktop = stack_pointer;
@@ -2880,9 +2874,8 @@
This is all fairly simple. Digging the information out of
co_lnotab takes some work, but is conceptually clear.
- Somewhat harder to explain is why we don't call the line
- trace function when executing a POP_TOP or RETURN_NONE
- opcodes. An example probably serves best.
+ Somewhat harder to explain is why we don't *always* call the
+ line trace function when the above test fails.
Consider this code:
@@ -2907,7 +2900,8 @@
5 16 LOAD_CONST 2 (2)
19 PRINT_ITEM
20 PRINT_NEWLINE
- >> 21 RETURN_NONE
+ >> 21 LOAD_CONST 0 (None)
+ 24 RETURN_VALUE
If a is false, execution will jump to instruction at offset
15 and the co_lnotab will claim that execution has moved to
@@ -2915,56 +2909,48 @@
associate the POP_TOP with line 4, but that doesn't make
sense in all cases (I think).
- On the other hand, if a is true, execution will jump from
- instruction offset 12 to offset 21. Then the co_lnotab would
- imply that execution has moved to line 5, which is again
- misleading.
+ What we do is only call the line trace function if the co_lnotab
+ indicates we have jumped to the *start* of a line, i.e. if the
+ current instruction offset matches the offset given for the
+ start of a line by the co_lnotab.
- This is why it is important that RETURN_NONE is *only* used
- for the "falling off the end of the function" form of
- returning None -- using it for code like
-
- 1: def f():
- 2: return
-
- would, once again, lead to misleading tracing behaviour.
-
- It is also worth mentioning that getting tracing behaviour
- right is the *entire* motivation for adding the RETURN_NONE
- opcode.
+ This also takes care of the situation where a is true.
+ Execution will jump from instruction offset 12 to offset 21.
+ Then the co_lnotab would imply that execution has moved to line
+ 5, which is again misleading.
*/
- if (opcode != POP_TOP && opcode != RETURN_NONE &&
- (frame->f_lasti < *instr_lb || frame->f_lasti > *instr_ub)) {
+ if ((frame->f_lasti < *instr_lb || frame->f_lasti >= *instr_ub)) {
PyCodeObject* co = frame->f_code;
int size, addr;
unsigned char* p;
- call_trace(func, obj, frame, PyTrace_LINE, Py_None);
+ size = PyString_GET_SIZE(co->co_lnotab) / 2;
+ p = (unsigned char*)PyString_AS_STRING(co->co_lnotab);
- size = PyString_Size(co->co_lnotab) / 2;
- p = (unsigned char*)PyString_AsString(co->co_lnotab);
+ addr = 0;
/* possible optimization: if f->f_lasti == instr_ub
(likely to be a common case) then we already know
instr_lb -- if we stored the matching value of p
somwhere we could skip the first while loop. */
- addr = 0;
-
/* see comments in compile.c for the description of
co_lnotab. A point to remember: increments to p
should come in pairs -- although we don't care about
the line increments here, treating them as byte
increments gets confusing, to say the least. */
- while (size >= 0) {
+ while (size > 0) {
if (addr + *p > frame->f_lasti)
break;
addr += *p++;
p++;
--size;
}
+ if (addr == frame->f_lasti)
+ call_trace(func, obj, frame,
+ PyTrace_LINE, Py_None);
*instr_lb = addr;
if (size > 0) {
while (--size >= 0) {
diff --git a/Python/compile.c b/Python/compile.c
index 0109fe5..26c56f4 100644
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -4014,7 +4014,10 @@
c->c_infunction = 1;
com_node(c, CHILD(n, 4));
c->c_infunction = 0;
- com_addbyte(c, RETURN_NONE);
+ com_addoparg(c, LOAD_CONST, com_addconst(c, Py_None));
+ com_push(c, 1);
+ com_addbyte(c, RETURN_VALUE);
+ com_pop(c, 1);
}
static void
@@ -4081,13 +4084,19 @@
n = CHILD(n, 0);
if (TYPE(n) != NEWLINE)
com_node(c, n);
- com_addbyte(c, RETURN_NONE);
+ com_addoparg(c, LOAD_CONST, com_addconst(c, Py_None));
+ com_push(c, 1);
+ com_addbyte(c, RETURN_VALUE);
+ com_pop(c, 1);
c->c_interactive--;
break;
case file_input: /* A whole file, or built-in function exec() */
com_file_input(c, n);
- com_addbyte(c, RETURN_NONE);
+ com_addoparg(c, LOAD_CONST, com_addconst(c, Py_None));
+ com_push(c, 1);
+ com_addbyte(c, RETURN_VALUE);
+ com_pop(c, 1);
break;
case eval_input: /* Built-in function input() */