Fix branch coverage for yield statements. #308 #324

Turns out the "call" and "return" trace events are really "start frame" and
"end frame".  They happen not only when functions are entered and left, but
when generators yield and resume.  We aren't interested in arcs into and out
of yield statements, so the trace functions look more closely to see what's
really happening, and record an arc in human-friendly terms.

Thanks for Mickie Betz for pushing on this bug, although her code is no
longer here. :(

4 files changed, 60 insertions, 21 deletions

diff --git a/coverage/parser.py b/coverage/parser.py
index cb0fc1fc..fc751eb2 100644
--- a/coverage/parser.py
+++ b/coverage/parser.py
@@ -308,7 +308,7 @@ OPS_CODE_END = _opcode_set('RETURN_VALUE')
 # Opcodes that unconditionally end the code chunk.
 OPS_CHUNK_END = _opcode_set(
     'JUMP_ABSOLUTE', 'JUMP_FORWARD', 'RETURN_VALUE', 'RAISE_VARARGS',
-    'BREAK_LOOP', 'CONTINUE_LOOP', 'YIELD_VALUE',
+    'BREAK_LOOP', 'CONTINUE_LOOP',
     )
 
 # Opcodes that unconditionally begin a new code chunk.  By starting new chunks
@@ -430,10 +430,9 @@ class ByteParser(object):
         Returns a list of `Chunk` objects.
 
         """
-        # The list of chunks so far, and the one we're working on.  We always
-        # start with an entrance to the code object.
-        chunk = Chunk(0, -1, True)
-        chunks = [chunk]
+        # The list of chunks so far, and the one we're working on.
+        chunks = []
+        chunk = None
 
         # A dict mapping byte offsets of line starts to the line numbers.
         bytes_lines_map = dict(self._bytes_lines())
@@ -482,6 +481,10 @@ class ByteParser(object):
                 if chunk:
                     chunk.exits.add(bc.offset)
                 chunk = Chunk(bc.offset, chunk_lineno, first_chunk)
+                if not chunks:
+                    # The very first chunk of a code object is always an
+                    # entrance.
+                    chunk.entrance = True
                 chunks.append(chunk)
 
             # Look at the opcode.
@@ -571,12 +574,15 @@ class ByteParser(object):
         """
         chunks = self._split_into_chunks()
 
-        # A map from byte offsets to chunks jumped into.
+        # A map from byte offsets to the chunk starting at that offset.
         byte_chunks = dict((c.byte, c) for c in chunks)
 
         # Traverse from the first chunk in each line, and yield arcs where
         # the trace function will be invoked.
         for chunk in chunks:
+            if chunk.entrance:
+                yield (-1, chunk.line)
+
             if not chunk.first:
                 continue
 
@@ -647,6 +653,8 @@ class Chunk(object):
 
     .. _basic block: http://en.wikipedia.org/wiki/Basic_block
 
+    `byte` is the offset to the bytecode starting this chunk.
+
     `line` is the source line number containing this chunk.
 
     `first` is true if this is the first chunk in the source line.
@@ -654,19 +662,24 @@ class Chunk(object):
     An exit < 0 means the chunk can leave the code (return).  The exit is
     the negative of the starting line number of the code block.
 
+    The `entrance` attribute is a boolean indicating whether the code object
+    can be entered at this chunk.
+
     """
     def __init__(self, byte, line, first):
         self.byte = byte
         self.line = line
         self.first = first
         self.length = 0
+        self.entrance = False
         self.exits = set()
 
     def __repr__(self):
-        if self.first:
-            bang = "!"
-        else:
-            bang = ""
-        return "<%d+%d @%d%s %r>" % (
-            self.byte, self.length, self.line, bang, list(self.exits)
+        return "<%d+%d @%d%s%s %r>" % (
+            self.byte,
+            self.length,
+            self.line,
+            "!" if self.first else "",
+            "v" if self.entrance else "",
+            list(self.exits),
             )
diff --git a/coverage/pytracer.py b/coverage/pytracer.py
index 0eafbef0..3f03aaf7 100644
--- a/coverage/pytracer.py
+++ b/coverage/pytracer.py
@@ -1,7 +1,15 @@
 """Raw data collector for Coverage."""
 
+import dis
 import sys
 
+from coverage import env
+
+# We need the YIELD_VALUE opcode below, in a comparison-friendly form.
+YIELD_VALUE = dis.opmap['YIELD_VALUE']
+if env.PY2:
+    YIELD_VALUE = chr(YIELD_VALUE)
+
 
 class PyTracer(object):
     """Python implementation of the raw data tracer."""
@@ -79,9 +87,11 @@ class PyTracer(object):
                 if tracename not in self.data:
                     self.data[tracename] = {}
                 self.cur_file_dict = self.data[tracename]
-            # Set the last_line to -1 because the next arc will be entering a
-            # code block, indicated by (-1, n).
-            self.last_line = -1
+            # The call event is really a "start frame" event, and happens for
+            # function calls and re-entering generators.  The f_lasti field is
+            # -1 for calls, and a real offset for generators.  Use -1 as the
+            # line number for calls, and the real line number for generators.
+            self.last_line = -1 if (frame.f_lasti < 0) else frame.f_lineno
         elif event == 'line':
             # Record an executed line.
             if self.cur_file_dict is not None:
@@ -93,8 +103,12 @@ class PyTracer(object):
                 self.last_line = lineno
         elif event == 'return':
             if self.arcs and self.cur_file_dict:
-                first = frame.f_code.co_firstlineno
-                self.cur_file_dict[(self.last_line, -first)] = None
+                # Record an arc leaving the function, but beware that a
+                # "return" event might just mean yielding from a generator.
+                bytecode = frame.f_code.co_code[frame.f_lasti]
+                if bytecode != YIELD_VALUE:
+                    first = frame.f_code.co_firstlineno
+                    self.cur_file_dict[(self.last_line, -first)] = None
             # Leaving this function, pop the filename stack.
             self.cur_file_dict, self.last_line = self.data_stack.pop()
         elif event == 'exception':
diff --git a/coverage/results.py b/coverage/results.py
index 0b27971f..def3a075 100644
--- a/coverage/results.py
+++ b/coverage/results.py
@@ -101,10 +101,13 @@ class Analysis(object):
         # Exclude arcs here which connect a line to itself.  They can occur
         # in executed data in some cases.  This is where they can cause
         # trouble, and here is where it's the least burden to remove them.
+        # Also, generators can somehow cause arcs from "enter" to "exit", so
+        # make sure we have at least one positive value.
         unpredicted = (
             e for e in executed
                 if e not in possible
                     and e[0] != e[1]
+                    and (e[0] > 0 or e[1] > 0)
         )
         return sorted(unpredicted)
 
diff --git a/coverage/tracer.c b/coverage/tracer.c
index 43ecd188..d532dcce 100644
--- a/coverage/tracer.c
+++ b/coverage/tracer.c
@@ -3,6 +3,7 @@
 #include "Python.h"
 #include "structmember.h"
 #include "frameobject.h"
+#include "opcode.h"
 
 /* Compile-time debugging helpers */
 #undef WHAT_LOG         /* Define to log the WHAT params in the trace function. */
@@ -596,7 +597,11 @@ CTracer_handle_call(CTracer *self, PyFrameObject *frame)
         SHOWLOG(self->pdata_stack->depth, frame->f_lineno, filename, "skipped");
     }
 
-    self->cur_entry.last_line = -1;
+    /* A call event is really a "start frame" event, and can happen for
+     * re-entering a generator also.  f_lasti is -1 for a true call, and a
+     * real byte offset for a generator re-entry.
+     */
+    self->cur_entry.last_line = (frame->f_lasti < 0) ? -1 : frame->f_lineno;
 
     ret = RET_OK;
 
@@ -685,9 +690,13 @@ CTracer_handle_return(CTracer *self, PyFrameObject *frame)
     }
     if (self->pdata_stack->depth >= 0) {
         if (self->tracing_arcs && self->cur_entry.file_data) {
-            int first = frame->f_code->co_firstlineno;
-            if (CTracer_record_pair(self, self->cur_entry.last_line, -first) < 0) {
-                goto error;
+            /* Need to distinguish between RETURN_VALUE and YIELD_VALUE. */
+            int bytecode = MyText_AS_STRING(frame->f_code->co_code)[frame->f_lasti];
+            if (bytecode != YIELD_VALUE) {
+                int first = frame->f_code->co_firstlineno;
+                if (CTracer_record_pair(self, self->cur_entry.last_line, -first) < 0) {
+                    goto error;
+                }
             }
         }