Fix issue 19163: don't count declarations in coverage analysis

[FeepingCreature]

Consider this code:

module test;

void main()
{
    if (false)
    {
        static if (2 == 2)
        {
        }
        else
        {
        }
        alias type = int;
        enum k = 2;
        enum array = [1, 2];
        static foreach (i; array)
        {
        }
    }
}

When run with dmd -cov -run test.d, the alias, enum and static foreach are given coverage counts of 0, despite the fact that they contain no executable code.

This PR fixes that, by only counting coverage for expressions that actually do work at runtime.

[dlang-bot]

Thanks for your pull request and interest in making D better, @FeepingCreature! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	19163	minor	static/tuple foreach counted incorrectly in coverage analysis

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "stable + dmd#8997"

[RazvanN7]

Is there any way we can test this?

[FeepingCreature]

From looking at the test suite, not easily. Would have to make a script to check the generated .lst file after the rest has run, and I have basically no idea how to set that up.

[thewilsonator]

This is an enhancement, please target master.

[FeepingCreature]

It's not a new feature or new functionality, it's a correction of an error in existing functionality.

Lines of code that have no runtime behavior should not have coverage.

[wilzbach]

From looking at the test suite, not easily. Would have to make a script to check the generated .lst file after the rest has run, and I have basically no idea how to set that up.

Have a look at the existing tests. You basically start with a shell script, invoke the compiler there, then compare the two files with diff. There are a few environment variables that can by used to set e.g. the output directory to the one of the testsuite.

[thewilsonator]

dlang/dub is red. Is this expected on stable?

[FeepingCreature]

I'm reasonably sure it's spurious.

[thewilsonator]

Hmm, I hesitant to pull with such a critical piece of infrastructure failing, I restarted it once already and it failed again.

[FeepingCreature]

I'm not sure how I could logically be responsible for "Package dub not found for registry at https://code.dlang.org/".

Try restarting another PR. You should get this error everywhere.

[thewilsonator]

Maybe it is a stable thing? I'll recheck tomorrow morning.

[PetarKirov]

The dub interactive remove test has been fixed, but the latest tag (1.12.0) doesn't contain the fix. Compare:
https://github.com/dlang/dub/blob/v1.12.0/test/interactive-remove.sh#L10 with:
https://github.com/dlang/dub/blob/b4b8bf9e7fd5a820ecc79cb8bc8ed18d9f97daae/test/interactive-remove.sh#L10

[PetarKirov]

Auto-merge toggled on

[PetarKirov]

It looks like the github->bugzilla issue auto-close integration has not yet kicked in. If the issue is not auto-closed in several hours be sure to close it manually and provide a link to this PR.

Actually, I think it waits for the commit to be merged to master.

[FeepingCreature]

yay

[wilzbach]

Actually, I think it waits for the commit to be merged to master.

Yup.