fix code coverage bug in tail position and else (JuliaLang#53354)

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.

Also fixes inliner handling of the sentinel `0` value for code
locations.

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

@@ -4371,7 +4371,7 @@ f(x) = yt(x)
  (if (eq? (cdr s) dest-tokens)
  (cons (car s) l)
  (loop (cdr s) (cons (car s) l))))))
- (define (emit-return x)
+ (define (emit-return tail x)
  (define (emit- x)
  (let* ((tmp (if ((if (null? catch-token-stack) valid-ir-return? simple-atom?) x)
  #f
@@ -4380,8 +4380,12 @@ 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)
+ ;; if we are adding an implicit return then mark it as having no location
+ (if (not (eq? tail 'explicit))
+ (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 '())))
@@ -4531,7 +4535,7 @@ f(x) = yt(x)
  (eq? (car e) 'globalref))
  (underscore-symbol? (cadr e)))))
  (error (string "all-underscore identifiers are write-only and their values cannot be used in expressions" (format-loc current-loc))))
- (cond (tail (emit-return e1))
+ (cond (tail (emit-return tail e1))
  (value e1)
  ((symbol? e1) (emit e1) #f) ;; keep symbols for undefined-var checking
  ((and (pair? e1) (eq? (car e1) 'outerref)) (emit e1) #f) ;; keep globals for undefined-var checking
@@ -4577,7 +4581,7 @@ f(x) = yt(x)
  (else
  (compile-args (cdr e) break-labels))))
  (callex (cons (car e) args)))
- (cond (tail (emit-return callex))
+ (cond (tail (emit-return tail callex))
  (value callex)
  (else (emit callex)))))
  ((=)
@@ -4594,7 +4598,7 @@ f(x) = yt(x)
  (if (not (eq? rr rhs))
  (emit `(= ,rr ,rhs)))
  (emit `(= ,lhs ,rr))
- (if tail (emit-return rr))
+ (if tail (emit-return tail rr))
  rr)
  (emit-assignment lhs rhs))))))
  ((block)
@@ -4647,7 +4651,7 @@ f(x) = yt(x)
  (if file-diff (set! filename last-fname))
  v)))
  ((return)
- (compile (cadr e) break-labels #t #t)
+ (compile (cadr e) break-labels #t 'explicit)
  #f)
  ((unnecessary)
  ;; `unnecessary` marks expressions generated by lowering that
@@ -4662,7 +4666,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))
@@ -4674,7 +4679,7 @@ f(x) = yt(x)
  (if (not tail)
  (set-car! (cdr end-jump) (make&mark-label))
  (if (length= e 3)
- (emit-return v2)))
+ (emit-return tail v2)))
  val))))
  ((_while)
  (let* ((endl (make-label))
@@ -4716,7 +4721,7 @@ f(x) = yt(x)
  (emit `(label ,m))
  (put! label-map (cadr e) (make&mark-label)))
  (if tail
- (emit-return '(null))
+ (emit-return tail '(null))
  (if value (error "misplaced label")))))
  ((symbolicgoto)
  (let* ((m (get label-map (cadr e) #f))
@@ -4762,7 +4767,7 @@ f(x) = yt(x)
  (begin (if els
  (begin (if (and (not val) v1) (emit v1))
  (emit `(leave ,handler-token)))
- (if v1 (emit-return v1)))
+ (if v1 (emit-return tail v1)))
  (if (not finally) (set! endl #f)))
  (begin (emit `(leave ,handler-token))
  (emit `(goto ,(or els endl)))))
@@ -4794,7 +4799,7 @@ f(x) = yt(x)
  (emit `(= ,tmp (call (core ===) ,finally ,(caar actions))))
  (emit `(gotoifnot ,tmp ,skip))))
  (let ((ac (cdar actions)))
- (cond ((eq? (car ac) 'return) (emit-return (cadr ac)))
+ (cond ((eq? (car ac) 'return) (emit-return tail (cadr ac)))
  ((eq? (car ac) 'break) (emit-break (cadr ac)))
  (else ;; assumed to be a rethrow
  (emit ac))))
@@ -4833,8 +4838,8 @@ f(x) = yt(x)
  (set! global-const-error current-loc))
  (emit e))))
  ((atomic) (error "misplaced atomic declaration"))
- ((isdefined) (if tail (emit-return e) e))
- ((boundscheck) (if tail (emit-return e) e))
+ ((isdefined) (if tail (emit-return tail e) e))
+ ((boundscheck) (if tail (emit-return tail e) e))
 
  ((method)
  (if (not (null? (cadr lam)))
@@ -4855,20 +4860,20 @@ f(x) = yt(x)
  l))))
  (emit `(method ,(or (cadr e) '(false)) ,sig ,lam))
  (if value (compile '(null) break-labels value tail)))
- (cond (tail (emit-return e))
+ (cond (tail (emit-return tail e))
  (value e)
  (else (emit e)))))
  ((lambda)
  (let ((temp (linearize e)))
- (cond (tail (emit-return temp))
+ (cond (tail (emit-return tail temp))
  (value temp)
  (else (emit temp)))))
 
  ;; top level expressions
  ((thunk module)
  (check-top-level e)
  (emit e)
- (if tail (emit-return '(null)))
+ (if tail (emit-return tail '(null)))
  '(null))
  ((toplevel-only)
  (check-top-level (cdr e))
@@ -4878,7 +4883,7 @@ f(x) = yt(x)
  (check-top-level e)
  (let ((val (make-ssavalue)))
  (emit `(= ,val ,e))
- (if tail (emit-return val))
+ (if tail (emit-return tail val))
  val))
 
  ;; other top level expressions
@@ -4887,7 +4892,7 @@ f(x) = yt(x)
  (emit e)
  (let ((have-ret? (and (pair? code) (pair? (car code)) (eq? (caar code) 'return))))
  (if (and tail (not have-ret?))
- (emit-return '(null))))
+ (emit-return tail '(null))))
  '(null))
 
  ((gc_preserve_begin)
@@ -4911,7 +4916,7 @@ f(x) = yt(x)
  (else
  (emit e)))
  (if (and tail (not have-ret?))
- (emit-return '(null)))
+ (emit-return tail '(null)))
  '(null)))
 
  ;; unsupported assignment operators
@@ -5027,6 +5032,7 @@ f(x) = yt(x)
  (labltable (table))
  (ssavtable (table))
  (current-loc 0)
+ (nowhere #f)
  (current-file file)
  (current-line line)
  (locstack '())
@@ -5040,26 +5046,33 @@ f(x) = yt(x)
  (set! current-loc 1)))
  (set! code (cons e code))
  (set! i (+ i 1))
- (set! locs (cons current-loc locs)))
+ (set! locs (cons (if nowhere 0 current-loc) locs))
+ (set! nowhere #f))
  (let loop ((stmts (cdr body)))
  (if (pair? stmts)
  (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! nowhere #t))
+ ((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

test/show.jl

@@ -2106,7 +2106,7 @@ let src = code_typed(my_fun28173, (Int,), debuginfo=:source)[1][1]
  io = IOBuffer()
  Base.IRShow.show_ir(io, ir, Base.IRShow.default_config(ir; verbose_linetable=true))
  seekstart(io)
- @test count(contains(r"@ a{80}:\d+ within `my_fun28173"), eachline(io)) == 11
+ @test count(contains(r"@ a{80}:\d+ within `my_fun28173"), eachline(io)) == 10
 
  # Test that a bad :invoke doesn't cause an error during printing
  Core.Compiler.insert_node!(ir, 1, Core.Compiler.NewInstruction(Expr(:invoke, nothing, sin), Any), false)

test/syntax.jl

@@ -713,7 +713,7 @@ m1_exprs = get_expr_list(Meta.lower(@__MODULE__, quote @m1 end))
 let low3 = Meta.lower(@__MODULE__, quote @m3 end)
  m3_exprs = get_expr_list(low3)
  ci = low3.args[1]::Core.CodeInfo
- @test ci.codelocs in ([4, 4, 2], [4, 2])
+ @test ci.codelocs in ([4, 4, 0], [4, 0])
  @test is_return_ssavalue(m3_exprs[end])
 end