rust
rust copied to clipboard
rust-lang
coverage: Initial support for branch coverage instrumentation
(This is a review-ready version of the changes that were drafted in #118305.)
This PR adds support for branch coverage instrumentation, gated behind the unstable flag value -Zunstable-options -Cinstrument-coverage=branch.
During THIR-to-MIR lowering (MIR building), if branch coverage is enabled, we collect additional information about branch conditions and their corresponding then/else blocks. We inject special marker statements into those blocks, so that the InstrumentCoverage MIR pass can reliably identify them even after the initially-built MIR has been simplified and renumbered.
The rest of the changes are mostly just plumbing needed to gather up the information that was collected during MIR building, and include it in the coverage metadata that we embed in the final binary.
Note that llvm-cov show doesn't print branch coverage information in its source views by default; that needs to be explicitly enabled with --show-branches=count or similar.
The current implementation doesn't have any support for instrumenting if let or let-chains. I think it's still useful without that, and adding it would be non-trivial, so I'm happy to leave that for future work.
I would prefer to have #122226 land before this, because that PR changes the unstable flag that is used to enable branch coverage instrumentation. But that isn't strictly necessary, and the adjustments required would be small.
r? @nnethercote
rustbot has assigned @nnethercote. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Some changes occurred to MIR optimizations
cc @rust-lang/wg-mir-opt
Some changes occurred in match lowering
cc @Nadrieril
The changes that are most important in terms of reviewer attention are the alterations to non-coverage-specific code in rustc_mir_build, since they have the most potential to step on other people's toes.
(Whereas the coverage-specific modules are mostly self-contained, so there's less impact on the rest of the compiler.)
Made some style improvements to coverageinfo.rs, since I wasn't happy with Inversions as a name (diff).
I skimmed through this and it looks reasonable and is well-commented, and I like that you added the test first so the changes to it are clear later on. But I know very little about MIR and so am not a good reviewer for this. According to triagebot.toml the MIR-iest reviewer we have is...
r? @oli-obk
#122226 has been approved, so once that lands I'll rebase this and make the necessary adjustments.
I've rebased and updated the tests for #122226, ~~but I still need to update the “placeholder” docs for -Zcoverage-options=branch~~.
EDIT: Updates should be complete now.
