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/Doc/lib/libdis.tex b/Doc/lib/libdis.tex
index 567c0ee..7bdd239 100644
--- a/Doc/lib/libdis.tex
+++ b/Doc/lib/libdis.tex
@@ -27,7 +27,8 @@
3 LOAD_FAST 0 (alist)
6 CALL_FUNCTION 1
9 RETURN_VALUE
- 10 RETURN_NONE
+ 10 LOAD_CONST 0 (None)
+ 13 RETURN_VALUE
\end{verbatim}
(The ``2'' is a line number).
@@ -401,14 +402,6 @@
Returns with TOS to the caller of the function.
\end{opcodedesc}
-\begin{opcodedesc}{RETURN_NONE}{}
-Returns \constant{None} to the caller of the function. This opcode is
-generated as the last opcode of every function and only then, for
-reasons to do with tracing support. See the comments in the function
-\cfunction{maybe_call_line_trace} in \file{Python/ceval.c} for the
-gory details. \versionadded{2.3}.
-\end{opcodedesc}
-
\begin{opcodedesc}{YIELD_VALUE}{}
Pops \code{TOS} and yields it from a generator.
\end{opcodedesc}
diff --git a/Doc/whatsnew/whatsnew23.tex b/Doc/whatsnew/whatsnew23.tex
index a971791..ff16de1 100644
--- a/Doc/whatsnew/whatsnew23.tex
+++ b/Doc/whatsnew/whatsnew23.tex
@@ -1225,11 +1225,6 @@
This will have the added effect of making the code work as desired
under ``python -O'' in earlier versions of Python.
-To make tracing work as expected, it was found necessary to add a new
-opcode, \cdata{RETURN_NONE}, to the VM. If you want to know why, read
-the comments in the function \cfunction{maybe_call_line_trace} in
-\file{Python/ceval.c}.
-
\end{itemize}
diff --git a/Include/opcode.h b/Include/opcode.h
index 28d0ae4..2f3dd04 100644
--- a/Include/opcode.h
+++ b/Include/opcode.h
@@ -71,9 +71,6 @@
#define INPLACE_OR 79
#define BREAK_LOOP 80
-#define RETURN_NONE 81 /* *only* for function epilogues
- -- see comments in
- ceval.c:maybe_call_line_trace for why */
#define LOAD_LOCALS 82
#define RETURN_VALUE 83
#define IMPORT_STAR 84
diff --git a/Lib/dis.py b/Lib/dis.py
index 4257729..30053bf 100644
--- a/Lib/dis.py
+++ b/Lib/dis.py
@@ -254,7 +254,6 @@
def_op('INPLACE_OR', 79)
def_op('BREAK_LOOP', 80)
-def_op('RETURN_NONE', 81)
def_op('LOAD_LOCALS', 82)
def_op('RETURN_VALUE', 83)
def_op('IMPORT_STAR', 84)
diff --git a/Lib/test/test_trace.py b/Lib/test/test_trace.py
new file mode 100644
index 0000000..ee847b6
--- /dev/null
+++ b/Lib/test/test_trace.py
@@ -0,0 +1,110 @@
+# Testing the line trace facility.
+
+from test import test_support
+import unittest
+import sys
+import difflib
+
+# A very basic example. If this fails, we're in deep trouble.
+def basic():
+ return 1
+
+basic.events = [(0, 'call'),
+ (1, 'line'),
+ (1, 'return')]
+
+# Armin Rigo's failing example:
+def arigo_example():
+ x = 1
+ del x
+ while 0:
+ pass
+ x = 1
+
+arigo_example.events = [(0, 'call'),
+ (1, 'line'),
+ (2, 'line'),
+ (3, 'line'),
+ (5, 'line'),
+ (5, 'return')]
+
+# check that lines consisting of just one instruction get traced:
+def one_instr_line():
+ x = 1
+ del x
+ x = 1
+
+one_instr_line.events = [(0, 'call'),
+ (1, 'line'),
+ (2, 'line'),
+ (3, 'line'),
+ (3, 'return')]
+
+def no_pop_tops(): # 0
+ x = 1 # 1
+ for a in range(2): # 2
+ if a: # 3
+ x = 1 # 4
+ else: # 5
+ x = 1 # 6
+
+no_pop_tops.events = [(0, 'call'),
+ (1, 'line'),
+ (2, 'line'),
+ (3, 'line'),
+ (6, 'line'),
+ (2, 'line'),
+ (3, 'line'),
+ (4, 'line'),
+ (2, 'line'),
+ (6, 'return')]
+
+def no_pop_blocks():
+ while 0:
+ bla
+ x = 1
+
+no_pop_blocks.events = [(0, 'call'),
+ (1, 'line'),
+ (3, 'line'),
+ (3, 'return')]
+
+class Tracer:
+ def __init__(self):
+ self.events = []
+ def trace(self, frame, event, arg):
+ self.events.append((frame.f_lineno, event))
+ return self.trace
+
+class TraceTestCase(unittest.TestCase):
+ def run_test(self, func):
+ tracer = Tracer()
+ sys.settrace(tracer.trace)
+ func()
+ sys.settrace(None)
+ fl = func.func_code.co_firstlineno
+ events = [(l - fl, e) for (l, e) in tracer.events]
+ if events != func.events:
+ self.fail(
+ "events did not match expectation:\n" +
+ "\n".join(difflib.ndiff(map(str, func.events),
+ map(str, events))))
+
+ def test_1_basic(self):
+ self.run_test(basic)
+ def test_2_arigo(self):
+ self.run_test(arigo_example)
+ def test_3_one_instr(self):
+ self.run_test(one_instr_line)
+ def test_4_no_pop_blocks(self):
+ self.run_test(no_pop_blocks)
+ def test_5_no_pop_tops(self):
+ self.run_test(no_pop_tops)
+
+
+
+def test_main():
+ test_support.run_unittest(TraceTestCase)
+
+if __name__ == "__main__":
+ test_main()
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() */