diff --git a/Doc/lib/libdis.tex b/Doc/lib/libdis.tex index 567c0eed945..7bdd239aeb0 100644 --- a/Doc/lib/libdis.tex +++ b/Doc/lib/libdis.tex @@ -27,7 +27,8 @@ the following command can be used to get the disassembly of 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 @@ is evaluated, the locals are passed to the class definition. 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 a971791fb8c..ff16de1e279 100644 --- a/Doc/whatsnew/whatsnew23.tex +++ b/Doc/whatsnew/whatsnew23.tex @@ -1225,11 +1225,6 @@ should instead call \code{PyCode_Addr2Line(f->f_code, f->f_lasti)}. 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 28d0ae43b16..2f3dd04ba47 100644 --- a/Include/opcode.h +++ b/Include/opcode.h @@ -71,9 +71,6 @@ extern "C" { #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 4257729c2f6..30053bf7794 100644 --- a/Lib/dis.py +++ b/Lib/dis.py @@ -254,7 +254,6 @@ def_op('INPLACE_XOR', 78) 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 00000000000..ee847b66bfc --- /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 d4be04e4e45..8bd945ed05b 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1515,12 +1515,6 @@ eval_frame(PyFrameObject *f) 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 @@ maybe_call_line_trace(int opcode, Py_tracefunc func, PyObject *obj, 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 @@ maybe_call_line_trace(int opcode, Py_tracefunc func, PyObject *obj, 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 @@ maybe_call_line_trace(int opcode, Py_tracefunc func, PyObject *obj, 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 0109fe539c8..26c56f46bd3 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -4014,7 +4014,10 @@ compile_funcdef(struct compiling *c, node *n) 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 @@ compile_node(struct compiling *c, node *n) 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() */