August 01, 2015
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@puremagic.com
          Reporter: issues.dlang@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.

--