From eae14769ab2c9a1a6dbd8e587f462652aefd3ad4 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Wed, 6 Mar 2019 18:03:27 +0100 Subject: [PATCH] Improve coverage heuristic in function_body_lines The purpose of function_body_lines is to find functions which are "dead" (= never executed by test suite), and mark them as code, because Julia's built-in coverage code does not do that; if we were to rely on it, such functions would be marked as non-code, and thus they do not show up in the overall code coverage percentage. But our current heuristic is too aggressive in so far as that Julia also sometimes marks code lines as "is not code" in the middle of functions that are being executed (most likely due to inlining and other optimizations). If we mark those lines as code, they will show up as uncovered, with no way for the package author to change this. With this commit, we refine our heuristic: we only apply it to functions for which Julia recorded no coverage at all, as before; but if even just one line in the function body has coverage information, we do not touch the code lines in it at all. Some care must be taken to make sure this works right for nested functions. --- src/Coverage.jl | 2 +- src/parser.jl | 28 ++++++++++++++++++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/Coverage.jl b/src/Coverage.jl index 3f0c611..b2ac3f3 100644 --- a/src/Coverage.jl +++ b/src/Coverage.jl @@ -175,7 +175,7 @@ module Coverage linestart = minimum(searchsorted(linepos, pos - 1)) ast, pos = Meta.parse(content, pos) isa(ast, Expr) || continue - flines = function_body_lines(ast) + flines = function_body_lines(ast, coverage, linestart - 1) if !isempty(flines) flines .+= linestart-1 for l in flines diff --git a/src/parser.jl b/src/parser.jl index 2d4def0..d888ad3 100644 --- a/src/parser.jl +++ b/src/parser.jl @@ -10,16 +10,19 @@ isfuncexpr(ex::Expr) = ex.head == :function || (ex.head == :(=) && typeof(ex.args[1]) == Expr && ex.args[1].head == :call) isfuncexpr(arg) = false -function_body_lines(ast) = function_body_lines!(Int[], ast, false) -function_body_lines!(flines, arg, infunction) = flines -function function_body_lines!(flines, node::LineNumberNode, infunction) +function_body_lines(ast, coverage, lineoffset) = function_body_lines!(Int[], ast, coverage, lineoffset, false) + +function_body_lines!(flines, arg, coverage, lineoffset, infunction) = flines + +function function_body_lines!(flines, node::LineNumberNode, coverage, lineoffset, infunction) line = node.line if infunction push!(flines, line) end flines end -function function_body_lines!(flines, ast::Expr, infunction) + +function function_body_lines!(flines, ast::Expr, coverage::Vector{CovCount}, lineoffset, infunction) if ast.head == :line line = ast.args[1] if infunction @@ -50,12 +53,25 @@ function function_body_lines!(flines, ast::Expr, infunction) # and we don't want those lines to be identified as runnable code. # In this context, ast.args[1] is the function signature and # ast.args[2] is the method body + # + # now compute all lines in the body of this function, but for now, + # track them separately from flines + flines_new = Int[] for arg in ast.args[2].args - flines = function_body_lines!(flines, arg, true) + function_body_lines!(flines_new, arg, coverage, lineoffset, true) + end + + # if any of the lines in the body of the function already has + # coverage, we assume that the function was executed, generally + # speaking, and so we should *not* apply our heuristic (which is mean + # to mark functions which were never executed as code, which the + # coverage data returned by Julia normally does not) + if all(l -> coverage[l+lineoffset] === nothing, flines_new) + append!(flines, flines_new) end else for arg in args - flines = function_body_lines!(flines, arg, infunction) + function_body_lines!(flines, arg, coverage, lineoffset, infunction) end end