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

Improve coverage results on certain derived traits that show two different counters #79626

Closed
richkadel opened this issue Dec 2, 2020 · 2 comments
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage)

Comments

@richkadel
Copy link
Contributor

Related to MIR pass InstrumentCoverage (tracking issue #79121)

The derived traits get coverage, which is great, but some of the traits appear to get two coverage execution counts at different positions:

   4|      2|#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
                      ^0            ^0      ^0 ^0  ^1       ^0 ^0^0
```text

`PartialEq`, `PartialOrd`, and `Ord` (and possibly `Eq`, if the trait name was longer than 2
characters) have counts at their first and last characters.

Why is this? Why does `PartialOrd` have two values (1 and 0)? This must mean we are checking distinct coverages, so maybe we don't want to eliminate one of them. Should we merge them? If merged, do we lose some information?
@richkadel
Copy link
Contributor Author

It's possible this issue is fixed.

Note that the sample code with coverage snippet in this issue's description came from:

4| 2|#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
^0 ^0 ^0 ^0 ^1 ^0 ^0^0

Which was the coverage result from this test:

// This test confirms an earlier problem was resolved, supporting the MIR graph generated by the

The coverage tests were temporarily disabled on or around Feb 4, 2021 by PR #81688.

At some point between that PR and the failed CI merge attempt on Feb 10 (#81983 (comment)), some change was made to the Rust compiler that resolved the inconsistent coverage counts described in this issue.

(I assume this was a side-effect of another change made for unrelated reasons.)

I learned of the problem when I tried to merge PR #81734, which reactivated the coverage tests.

Since I don't know what PR/commit introduced this change in behavior, I'm not certain if that change fixes all similar issues where derived traits were showing two different counts. Unless someone finds other examples of that problem, I think we can wait for a short period of time, and then close this issue if no other examples are found.

cc: @tmandry @wesleywiser @pnkfelix

@Enselic
Copy link
Member

Enselic commented Dec 17, 2023

Triage: We have now waited a short time (and more) so let's close.

@Enselic Enselic closed this as completed Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage)
Projects
None yet
Development

No branches or pull requests

3 participants