]> granicus.if.org Git - python/commitdiff
Further SET_LINENO reomval fixes. See comments in patch #587933.
authorMichael W. Hudson <mwh@python.net>
Fri, 30 Aug 2002 13:09:51 +0000 (13:09 +0000)
committerMichael W. Hudson <mwh@python.net>
Fri, 30 Aug 2002 13:09:51 +0000 (13:09 +0000)
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...

Doc/lib/libdis.tex
Doc/whatsnew/whatsnew23.tex
Include/opcode.h
Lib/dis.py
Lib/test/test_trace.py [new file with mode: 0644]
Python/ceval.c
Python/compile.c

index 567c0eed945aca0f4ed77ddb093f6a85c382cacb..7bdd239aeb07a25e2d13281d84825773103fc74d 100644 (file)
@@ -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}
index a971791fb8c18ea80f62ec61bf38289e0cbb0c8b..ff16de1e279352691bfb979fa777b62da7b529cb 100644 (file)
@@ -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}
 
 
index 28d0ae43b169a00f8901e3530545af781af24b65..2f3dd04ba479182be3fc71445f6c422c3b2d1458 100644 (file)
@@ -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
index 4257729c2f69c387d3ead9bb87840781e906f916..30053bf7794f73e9ea2631ab76fe93800bdaccb9 100644 (file)
@@ -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 (file)
index 0000000..ee847b6
--- /dev/null
@@ -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()
index d4be04e4e45b957de464a547a0cf58bada91dd4b..8bd945ed05b42774daef53d78f6707f525d8af1c 100644 (file)
@@ -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.
-
-          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
+          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.
 
-          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) {
index 0109fe539c8e6a4f3eda4c6b8caa1407a0366945..26c56f46bd387732822b2f8b5fada0206704c7a1 100644 (file)
@@ -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() */