fix code coverage bug in tail position and else

This was due to lowering keeping the same location info for the inserted
`return` or `goto` statement, even though the last seen location might not have
executed.

Author: JeffBezanson

base/compiler/ssair/inlining.jl

@@ -1818,7 +1818,9 @@ end
 
 function ssa_substitute!(insert_node!::Inserter, subst_inst::Instruction, @nospecialize(val),
                          ssa_substitute::SSASubstitute)
-    subst_inst[:line] += ssa_substitute.linetable_offset
+    if subst_inst[:line] != 0
+        subst_inst[:line] += ssa_substitute.linetable_offset
+    end
     return ssa_substitute_op!(insert_node!, subst_inst, val, ssa_substitute)
 end
 

base/compiler/ssair/passes.jl

@@ -1519,8 +1519,12 @@ function try_inline_finalizer!(ir::IRCode, argexprs::Vector{Any}, idx::Int,
             ssa_rename[ssa.id]
         end
         stmt′ = ssa_substitute_op!(InsertBefore(ir, SSAValue(idx)), inst, stmt′, ssa_substitute)
+        newline = inst[:line]
+        if newline != 0
+            newline += ssa_substitute.linetable_offset
+        end
         ssa_rename[idx′] = insert_node!(ir, idx,
-            NewInstruction(inst; stmt=stmt′, line=inst[:line]+ssa_substitute.linetable_offset),
+            NewInstruction(inst; stmt=stmt′, line=newline),
             attach_after)
     end
 

src/julia-syntax.scm

@@ -4380,8 +4380,9 @@ f(x) = yt(x)
               (begin (emit `(= ,tmp ,x)) tmp)
               x)))
       (define (actually-return x)
-        (let* ((x (if rett
-                      (compile (convert-for-type-decl (emit- x) rett #t lam) '() #t #f)
+        (let* ((x (begin0 (emit- x) (emit '(line #f))))
+               (x (if rett
+                      (compile (convert-for-type-decl x rett #t lam) '() #t #f)
                       x))
                (x (emit- x)))
           (let ((pexc (pop-exc-expr catch-token-stack '())))
@@ -4662,7 +4663,8 @@ f(x) = yt(x)
                (let ((v1 (compile (caddr e) break-labels value tail)))
                  (if val (emit-assignment val v1))
                  (if (and (not tail) (or (length> e 3) val))
-                     (emit end-jump))
+                     (begin (emit `(line #f))
+                            (emit end-jump)))
                  (let ((elselabel (make&mark-label)))
                    (for-each (lambda (test)
                                (set-car! (cddr test) elselabel))
@@ -5046,20 +5048,26 @@ f(x) = yt(x)
           (let ((e (car stmts)))
             (cond ((atom? e) (emit e))
                   ((eq? (car e) 'line)
-                   (if (and (= current-line 0) (length= e 2) (pair? linetable))
-                       ;; (line n) after push_loc just updates the line for the new file
-                       (begin (set-lineno! (car linetable) (cadr e))
-                              (set! current-line (cadr e)))
-                       (begin
-                         (set! current-line (cadr e))
-                         (if (pair? (cddr e))
-                             (set! current-file (caddr e)))
-                         (set! linetable (cons (if (null? locstack)
-                                                   (make-lineinfo name current-file current-line)
-                                                   (make-lineinfo name current-file current-line (caar locstack)))
-                                               linetable))
-                         (set! linetablelen (+ linetablelen 1))
-                         (set! current-loc linetablelen))))
+                   (cond ((and (length= e 2) (not (cadr e)))
+                          ;; (line #f) marks that we are entering a generated statement
+                          ;; that should not be counted as belonging to the previous marked location,
+                          ;; for example `return` after a not-executed `if` arm in tail position.
+                          (set! current-loc 0))
+                         ((and (= current-line 0) (length= e 2) (pair? linetable))
+                          ;; (line n) after push_loc just updates the line for the new file
+                          (begin (set-lineno! (car linetable) (cadr e))
+                                 (set! current-line (cadr e))))
+                         (else
+                          (begin
+                            (set! current-line (cadr e))
+                            (if (pair? (cddr e))
+                                (set! current-file (caddr e)))
+                            (set! linetable (cons (if (null? locstack)
+                                                      (make-lineinfo name current-file current-line)
+                                                      (make-lineinfo name current-file current-line (caar locstack)))
+                                                  linetable))
+                            (set! linetablelen (+ linetablelen 1))
+                            (set! current-loc linetablelen)))))
                   ((and (length> e 2) (eq? (car e) 'meta) (eq? (cadr e) 'push_loc))
                    (set! locstack (cons (list current-loc current-line current-file) locstack))
                    (set! current-file (caddr e))

test/cmdlineargs.jl

@@ -545,6 +545,69 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
         got = read(covfile, String)
         @test isempty(got)
         rm(covfile)
+
+        function coverage_info_for(src::String)
+            mktemp(dir) do srcfile, io
+                write(io, src); close(io)
+                outfile = tempname(dir, cleanup=false)*".info"
+                run(`$exename --code-coverage=$outfile $srcfile`)
+                result = read(outfile, String)
+                rm(outfile, force=true)
+                result
+            end
+        end
+        @test contains(coverage_info_for("""
+            function cov_bug(x, p)
+                if p > 2
+                    print("")  # runs
+                end
+                if Base.compilerbarrier(:const, false)
+                    println("Does not run")
+                end
+            end
+            function do_test()
+                cov_bug(5, 3)
+            end
+            do_test()
+            """), """
+            DA:1,1
+            DA:2,1
+            DA:3,1
+            DA:5,1
+            DA:6,0
+            DA:9,1
+            DA:10,1
+            LH:6
+            LF:7
+            """)
+        @test contains(coverage_info_for("""
+            function cov_bug()
+                if Base.compilerbarrier(:const, true)
+                    if Base.compilerbarrier(:const, true)
+                        if Base.compilerbarrier(:const, false)
+                            println("Does not run")
+                        end
+                    else
+                        print("Does not run either")
+                    end
+                else
+                    print("")
+                end
+                return nothing
+            end
+            cov_bug()
+            """), """
+            DA:1,1
+            DA:2,1
+            DA:3,1
+            DA:4,1
+            DA:5,0
+            DA:8,0
+            DA:11,0
+            DA:13,1
+            LH:5
+            LF:8
+            """)
     end
 
     # --track-allocation