coverage: Initial support for branch coverage instrumentation by Zalathar · Pull Request #122322 · rust-lang/rust
rustbot
added
S-waiting-on-review
labels
Mar 11, 2024This was referenced
Mar 11, 2024matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
Mar 13, 2024…hercote coverage: Remove or migrate all unstable values of `-Cinstrument-coverage` (This PR was substantially overhauled from its original version, which migrated all of the existing unstable values intact.) This PR takes the three nightly-only values that are currently accepted by `-Cinstrument-coverage`, completely removes two of them (`except-unused-functions` and `except-unused-generics`), and migrates the third (`branch`) over to a newly-introduced unstable flag `-Zcoverage-options`. I have a few motivations for wanting to do this: - It's unclear whether anyone actually uses the `except-unused-*` values, so this serves as an opportunity to either remove them, or prompt existing users to object to their removal. - After rust-lang#117199, the stable values of `-Cinstrument-coverage` treat it as a boolean-valued flag, so having nightly-only extra values feels out-of-place. - Nightly-only values also require extra ad-hoc code to make sure they aren't accidentally exposed to stable users. - The new system allows multiple different settings to be toggled independently, which isn't possible in the current single-value system. - The new system makes it easier to introduce new behaviour behind an unstable toggle, and then gather nightly-user feedback before possibly making it the default behaviour for all users. - The new system also gives us a convenient place to put relatively-narrow options that won't ever be the default, but that nightly users might still want access to. - It's likely that we will eventually want to give stable users more fine-grained control over coverage instrumentation. The new flag serves as a prototype of what that stable UI might eventually look like. The `branch` option is a placeholder that currently does nothing. It will be used by rust-lang#122322 to opt into branch coverage instrumentation. --- I see `-Zcoverage-options` as something that will exist more-or-less indefinitely, though individual sub-options might come and go as appropriate. I think there will always be some demand for nightly-only toggles, so I don't see `-Zcoverage-options` itself ever being stable, though we might eventually stabilize something similar to it.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
Mar 13, 2024…hercote coverage: Remove or migrate all unstable values of `-Cinstrument-coverage` (This PR was substantially overhauled from its original version, which migrated all of the existing unstable values intact.) This PR takes the three nightly-only values that are currently accepted by `-Cinstrument-coverage`, completely removes two of them (`except-unused-functions` and `except-unused-generics`), and migrates the third (`branch`) over to a newly-introduced unstable flag `-Zcoverage-options`. I have a few motivations for wanting to do this: - It's unclear whether anyone actually uses the `except-unused-*` values, so this serves as an opportunity to either remove them, or prompt existing users to object to their removal. - After rust-lang#117199, the stable values of `-Cinstrument-coverage` treat it as a boolean-valued flag, so having nightly-only extra values feels out-of-place. - Nightly-only values also require extra ad-hoc code to make sure they aren't accidentally exposed to stable users. - The new system allows multiple different settings to be toggled independently, which isn't possible in the current single-value system. - The new system makes it easier to introduce new behaviour behind an unstable toggle, and then gather nightly-user feedback before possibly making it the default behaviour for all users. - The new system also gives us a convenient place to put relatively-narrow options that won't ever be the default, but that nightly users might still want access to. - It's likely that we will eventually want to give stable users more fine-grained control over coverage instrumentation. The new flag serves as a prototype of what that stable UI might eventually look like. The `branch` option is a placeholder that currently does nothing. It will be used by rust-lang#122322 to opt into branch coverage instrumentation. --- I see `-Zcoverage-options` as something that will exist more-or-less indefinitely, though individual sub-options might come and go as appropriate. I think there will always be some demand for nightly-only toggles, so I don't see `-Zcoverage-options` itself ever being stable, though we might eventually stabilize something similar to it.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Mar 13, 2024Rollup merge of rust-lang#122226 - Zalathar:zcoverage-options, r=nnethercote coverage: Remove or migrate all unstable values of `-Cinstrument-coverage` (This PR was substantially overhauled from its original version, which migrated all of the existing unstable values intact.) This PR takes the three nightly-only values that are currently accepted by `-Cinstrument-coverage`, completely removes two of them (`except-unused-functions` and `except-unused-generics`), and migrates the third (`branch`) over to a newly-introduced unstable flag `-Zcoverage-options`. I have a few motivations for wanting to do this: - It's unclear whether anyone actually uses the `except-unused-*` values, so this serves as an opportunity to either remove them, or prompt existing users to object to their removal. - After rust-lang#117199, the stable values of `-Cinstrument-coverage` treat it as a boolean-valued flag, so having nightly-only extra values feels out-of-place. - Nightly-only values also require extra ad-hoc code to make sure they aren't accidentally exposed to stable users. - The new system allows multiple different settings to be toggled independently, which isn't possible in the current single-value system. - The new system makes it easier to introduce new behaviour behind an unstable toggle, and then gather nightly-user feedback before possibly making it the default behaviour for all users. - The new system also gives us a convenient place to put relatively-narrow options that won't ever be the default, but that nightly users might still want access to. - It's likely that we will eventually want to give stable users more fine-grained control over coverage instrumentation. The new flag serves as a prototype of what that stable UI might eventually look like. The `branch` option is a placeholder that currently does nothing. It will be used by rust-lang#122322 to opt into branch coverage instrumentation. --- I see `-Zcoverage-options` as something that will exist more-or-less indefinitely, though individual sub-options might come and go as appropriate. I think there will always be some demand for nightly-only toggles, so I don't see `-Zcoverage-options` itself ever being stable, though we might eventually stabilize something similar to it.
This will allow MIR building to check whether a function is eligible for coverage instrumentation, and avoid collecting branch coverage info if it is not.
bors
added
S-waiting-on-bors
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.labels
Mar 14, 2024bors added a commit to rust-lang-ci/rust that referenced this pull request
Mar 14, 2024…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#117118 ([AIX] Remove AixLinker's debuginfo() implementation) - rust-lang#121650 (change std::process to drop supplementary groups based on CAP_SETGID) - rust-lang#121764 (Make incremental sessions identity no longer depend on the crate names provided by source code) - rust-lang#122212 (Copy byval argument to alloca if alignment is insufficient) - rust-lang#122322 (coverage: Initial support for branch coverage instrumentation) - rust-lang#122373 (Fix the conflict problem between the diagnostics fixes of lint `unnecessary_qualification` and `unused_imports`) - rust-lang#122479 (Implement `Duration::as_millis_{f64,f32}`) - rust-lang#122487 (Rename `StmtKind::Local` variant into `StmtKind::Let`) - rust-lang#122498 (Update version of cc crate) - rust-lang#122503 (Make `SubdiagMessageOp` well-formed) r? `@ghost` `@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request
Mar 15, 2024…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#117118 ([AIX] Remove AixLinker's debuginfo() implementation) - rust-lang#121650 (change std::process to drop supplementary groups based on CAP_SETGID) - rust-lang#121764 (Make incremental sessions identity no longer depend on the crate names provided by source code) - rust-lang#122212 (Copy byval argument to alloca if alignment is insufficient) - rust-lang#122322 (coverage: Initial support for branch coverage instrumentation) - rust-lang#122373 (Fix the conflict problem between the diagnostics fixes of lint `unnecessary_qualification` and `unused_imports`) - rust-lang#122479 (Implement `Duration::as_millis_{f64,f32}`) - rust-lang#122487 (Rename `StmtKind::Local` variant into `StmtKind::Let`) - rust-lang#122498 (Update version of cc crate) - rust-lang#122503 (Make `SubdiagMessageOp` well-formed) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Mar 15, 2024Rollup merge of rust-lang#122322 - Zalathar:branch, r=oli-obk coverage: Initial support for branch coverage instrumentation (This is a review-ready version of the changes that were drafted in rust-lang#118305.) This PR adds support for branch coverage instrumentation, gated behind the unstable flag value `-Zcoverage-options=branch`. (Coverage instrumentation must also be enabled with `-Cinstrument-coverage`.) 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters