Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implicit return lines reported active by coverage but not tracked in julia 1.10.2 #53557

Open
exaexa opened this issue Mar 2, 2024 · 7 comments
Labels
code coverage testsystem The unit testing framework and Test stdlib

Comments

@exaexa
Copy link
Contributor

exaexa commented Mar 2, 2024

Hello all,

I found that "implicit return" lines that contain only a variables are counted into lines that "should be covered" by tests but the coverage testing doesn't work on them.

For reproducers, I have a small package that only has this single source file:

module covtest

greet() = print("Hello World!")

function testfun(n)
    [ begin
        x = i
        x = x * x
        x = x + x
        x = x / x
        x
    end for i=1:n]
end

function testfun2(n)
    a = n
    a *= n
    a += n
    a /= n
    a
end

end # module covtest

and this runtests.jl file:

import covtest: testfun, testfun2
testfun(10)
testfun2(10)

The coverage produced by running ]test --coverage covtest is as follows:

        - module covtest
        - 
        - greet() = print("Hello World!")
        - 
        1 function testfun(n)
        1     [ begin
       10         x = i
       10         x = x * x
       10         x = x + x
       10         x = x / x
        0         x
        -     end for i=1:n]
        - end
        - 
        1 function testfun2(n)
        1     a = n
        1     a *= n
        1     a += n
        1     a /= n
        0     a
        - end
        - 
        - end # module covtest

This gets worked around by either adding explicit returns, or having a function called in the loops on the end (the explicit return can't be used there) -- e.g. as follows:

        - module covtest
        - 
        - greet() = print("Hello World!")
        - 
        1 function testfun(n)
        1     [ begin
       10         x = i
       10         x = x * x
       10         x = x + x
       10         x = x / x
       10         identity(x)
        -     end for i=1:n]
        - end
        - 
        1 function testfun2(n)
        1     a = n
        1     a *= n
        1     a += n
        1     a /= n
        1     return a
        - end
        - 
        - end # module covtest

For the functions I assume the return there is adviseable, but for the implicit values of begin/end blocks this actually seems like a regression to me. I originally thought this is somehow triggered by switch to JuliaSyntax (I could not reproduce this on <1.10), but the same problem happens with JULIA_USE_FLISP_PARSER=1 so maybe not. I'd expect that this could be caused by some part of codegen optimistically assuming that "this won't generate any code!!" and just dropping the whole line, but I wasn't able to find any commit/change/sourcecode to blame there, so it might also be something entirely different.

In case this is now the desired behavior, is there docs somewhere? (I just checked the release notes for 1.10 to be triple sure :D )

Thanks for any help/opinions on this :)

Best!
-mk

@exaexa
Copy link
Contributor Author

exaexa commented Mar 2, 2024

(note: I forgot to remove the greet() function but it looks labeled with a - instead of the expectable 0, as it is untested code. Is that right?)

@KristofferC
Copy link
Member

What was the latest version where this worke? 1.10.1, 1.9?

@exaexa
Copy link
Contributor Author

exaexa commented Mar 2, 2024

Looks like all was OK on 1.9.3 and 1.9.4 (e.g. here on master: https://github.com/COBREXA/JSONFBCModels.jl/actions/runs/7223225523/job/19681839328 ). In this specific case I found on this merge request: COBREXA/JSONFBCModels.jl#8 and this pipeline: https://github.com/COBREXA/JSONFBCModels.jl/actions/runs/7875684795/job/22202840125 which is 1.10.2.

I can try to bisect later, but I didn't have time to do much stuff lately so please don't count on me there 😅

@exaexa
Copy link
Contributor Author

exaexa commented Mar 2, 2024

(note: I forgot to remove the greet() function but it looks labeled with a - instead of the expectable 0, as it is untested code. Is that right?)

adding the report from 1.9.3 just for completeness:

        - module covtest
        - 
        - greet() = print("Hello World!")
        - 
        1 function testfun(n)
        1     [ begin
       10         x = i
       10         x = x * x
       10         x = x + x
       10         x = x / x
       10         x
        -     end for i=1:n]
        - end
        - 
        1 function testfun2(n)
        1     a = n
        1     a *= n
        1     a += n
        1     a /= n
        1     a
        - end
        - 
        - end # module covtest

...anyway so the greet() not being reported at all is probably not a regression but I still wouldn't say it's optimal.

@sjkelly
Copy link
Contributor

sjkelly commented Mar 4, 2024

Likely related to: #53354
which was introduced between 1.10.1 and 1.10.2
cc @JeffBezanson

@LilithHafner
Copy link
Member

I just hit this today.

@inkydragon inkydragon added the testsystem The unit testing framework and Test stdlib label Apr 8, 2024
@exaexa
Copy link
Contributor Author

exaexa commented May 12, 2024

In other news, I'm getting several similar cases with functions that are easy to inline, such as:

Base.:+(a::Int, b::Whatevs) = b+a
Base.:+(a::Whatevs, b::Int)= combine_whatevs_with_int(a,b)

The first line never gets coverage; even calls to 0 + Whatevs() generate coverage ticks for the second line.

I somehow assume there's an over-zealous optimization somewhere in the way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code coverage testsystem The unit testing framework and Test stdlib
Projects
None yet
Development

No branches or pull requests

6 participants