[Issue 14857] New: -cov should ignore invariants when compiling with -release (or maybe even always)

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=14857

          Issue ID: 14857
           Summary: -cov should ignore invariants when compiling with
                    -release (or maybe even always)
           Product: D
           Version: D2
          Hardware: All
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P1
         Component: dmd
          Assignee: nobody at puremagic.com
          Reporter: issues.dlang at jmdavisProg.com

Okay. This code

void main()
{
}

class C
{
    this(int i)
    {
        _i = i;
    }

    int foo(int j)
    in
    {
        assert(j > 5);
    }
    out(ret)
    {
        assert(ret == _i + j);
    }
    body
    {
        return _i + j;
    }

private:

    invariant()
    {
        assert(_i > 0 && _i < 43);
    }

    int _i = 7;
}

unittest
{
    auto c = new C(42);
    c.foo(19);
}

results in this coverage report with _not_ compiled with -release:

       |void main()
       |{
       |}
       |
       |class C
       |{
      2|    this(int i)
       |    {
      1|        _i = i;
       |    }
       |
       |    int foo(int j)
       |    in
      2|    {
      1|        assert(j > 5);
       |    }
       |    out(ret)
      2|    {
      1|        assert(ret == _i + j);
       |    }
       |    body
       |    {
      2|        return _i + j;
       |    }
       |
       |private:
       |
       |    invariant()
       |    {
      6|        assert(_i > 0 && _i < 43);
       |    }
       |
       |    int _i = 7;
       |}
       |
       |unittest
       |{
      1|    auto c = new C(42);
      1|    c.foo(19);
       |}
q.d is 100% covered

It has 100% code coverage. However, if the same program is compiled with
-release, you end up with

       |void main()
       |{
       |}
       |
       |class C
       |{
      1|    this(int i)
       |    {
      1|        _i = i;
       |    }
       |
       |    int foo(int j)
       |    in
       |    {
       |        assert(j > 5);
       |    }
       |    out(ret)
       |    {
       |        assert(ret == _i + j);
       |    }
       |    body
       |    {
      1|        return _i + j;
       |    }
       |
       |private:
       |
       |    invariant()
       |    {
0000000|        assert(_i > 0 && _i < 43);
       |    }
       |
       |    int _i = 7;
       |}
       |
       |unittest
       |{
      1|    auto c = new C(42);
      1|    c.foo(19);
       |}
q.d is 83% covered

So, the code coverage is now at 83% instead of 100% just because it was
compiled with -release, and the invariant wasn't run. But since it isn't
_supposed_ to run, it really makes no sense to include it in the code coverage
totals. And arguably, the invariants and are purely for testing code and
shouldn't even count towards the totals when not compiling with -release,
though I'm not sure that that really matters other than the fact that it skews
the percentages, because properly covered code would have run all of the
invariants anyway and thus not have stopped anyone from reaching 100% code
coverage (unlike with -release).

Now, curiously, the in and out blocks do not show totals next to them when
compiling with -release. So, presumably, this was already fixed for in/out
blocks. But invariants still end up with numbers next to them in -release, and
the code coverage does not reach 100%. -cov really should treat invariants the
same as it does in/out blocks, since they're pretty much the same thing.

But aside from consistency between invariants and in/out blocks, the big
problem is that with the current behavior, when compiling with -release, any
code with invariants cannot reach 100% code coverage, even though the code in
question isn't even supposed to run. So, this prevents code that is actually
100% covered from being marked as such, and so tools won't be able to verify
full coverage, and anyone who wants to verify it will have to do so by hand.

Because this is specific to -release, it's not as big a deal as it could be,
but because -release code doesn't necessarily match non-release code (due to
version blocks and the like), I think that it's still important - particularly
when you consider that Phobos runs its unit tests in both debug and release.
And since the release tests run second, the coverage files actually list the
coverage from -release, and we end up seeing lower coverage when we look at
those files due to this behavior with invariants.

--