Skip to content

Commit

Permalink
Auto merge of rust-lang#124507 - Zalathar:coverage-level, r=compiler-…
Browse files Browse the repository at this point in the history
…errors

coverage: Replace boolean options with a `CoverageLevel` enum

After rust-lang#123409, and some discussion at rust-lang#79649 (comment) and rust-lang#124120, it became clear to me that we should have a unified concept of “coverage level”, instead of having several separate boolean flags that aren't actually independent.

This PR therefore introduces a `CoverageLevel` enum, to replace the existing boolean flags for `branch` and `mcdc`.

The `no-branch` value (for `-Zcoverage-options`) has been renamed to `block`, instructing the compiler to only instrument for block coverage, with no branch coverage or MD/DC instrumentation.

`@rustbot` label +A-code-coverage
cc `@ZhuUx` `@Lambdaris` `@RenjiSann`
  • Loading branch information
bors committed Apr 30, 2024
2 parents 74a8df6 + f926337 commit f9dca46
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 39 deletions.
14 changes: 7 additions & 7 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ use rustc_data_structures::profiling::TimePassesFormat;
use rustc_errors::{emitter::HumanReadableErrorType, registry, ColorConfig};
use rustc_session::config::{
build_configuration, build_session_options, rustc_optgroups, BranchProtection, CFGuard, Cfg,
CollapseMacroDebuginfo, CoverageOptions, DebugInfo, DumpMonoStatsFormat, ErrorOutputType,
ExternEntry, ExternLocation, Externs, FunctionReturn, InliningThreshold, Input,
InstrumentCoverage, InstrumentXRay, LinkSelfContained, LinkerPluginLto, LocationDetail, LtoCli,
NextSolverConfig, OomStrategy, Options, OutFileName, OutputType, OutputTypes, PAuthKey, PacRet,
Passes, Polonius, ProcMacroExecutionStrategy, Strip, SwitchWithOptPath, SymbolManglingVersion,
WasiExecModel,
CollapseMacroDebuginfo, CoverageLevel, CoverageOptions, DebugInfo, DumpMonoStatsFormat,
ErrorOutputType, ExternEntry, ExternLocation, Externs, FunctionReturn, InliningThreshold,
Input, InstrumentCoverage, InstrumentXRay, LinkSelfContained, LinkerPluginLto, LocationDetail,
LtoCli, NextSolverConfig, OomStrategy, Options, OutFileName, OutputType, OutputTypes, PAuthKey,
PacRet, Passes, Polonius, ProcMacroExecutionStrategy, Strip, SwitchWithOptPath,
SymbolManglingVersion, WasiExecModel,
};
use rustc_session::lint::Level;
use rustc_session::search_paths::SearchPath;
Expand Down Expand Up @@ -761,7 +761,7 @@ fn test_unstable_options_tracking_hash() {
})
);
tracked!(codegen_backend, Some("abc".to_string()));
tracked!(coverage_options, CoverageOptions { branch: true, mcdc: true });
tracked!(coverage_options, CoverageOptions { level: CoverageLevel::Mcdc });
tracked!(crate_attr, vec!["abc".to_string()]);
tracked!(cross_crate_inline_threshold, InliningThreshold::Always);
tracked!(debug_info_for_profiling, true);
Expand Down
24 changes: 20 additions & 4 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,26 @@ pub enum InstrumentCoverage {
/// Individual flag values controlled by `-Z coverage-options`.
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Default)]
pub struct CoverageOptions {
/// Add branch coverage instrumentation.
pub branch: bool,
/// Add mcdc coverage instrumentation.
pub mcdc: bool,
pub level: CoverageLevel,
// Other boolean or enum-valued options might be added here.
}

/// Controls whether branch coverage or MC/DC coverage is enabled.
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
pub enum CoverageLevel {
/// Instrument for coverage at the MIR block level.
Block,
/// Also instrument branch points (includes block coverage).
Branch,
/// Instrument for MC/DC. Mostly a superset of branch coverage, but might
/// differ in some corner cases.
Mcdc,
}

impl Default for CoverageLevel {
fn default() -> Self {
Self::Block
}
}

/// Settings for `-Z instrument-xray` flag.
Expand Down
14 changes: 4 additions & 10 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ mod desc {
pub const parse_optimization_fuel: &str = "crate=integer";
pub const parse_dump_mono_stats: &str = "`markdown` (default) or `json`";
pub const parse_instrument_coverage: &str = parse_bool;
pub const parse_coverage_options: &str = "either `no-branch`, `branch` or `mcdc`";
pub const parse_coverage_options: &str = "`block` | `branch` | `mcdc`";
pub const parse_instrument_xray: &str = "either a boolean (`yes`, `no`, `on`, `off`, etc), or a comma separated list of settings: `always` or `never` (mutually exclusive), `ignore-loops`, `instruction-threshold=N`, `skip-entry`, `skip-exit`";
pub const parse_unpretty: &str = "`string` or `string=string`";
pub const parse_treat_err_as_bug: &str = "either no value or a non-negative number";
Expand Down Expand Up @@ -946,15 +946,9 @@ mod parse {

for option in v.split(',') {
match option {
"no-branch" => {
slot.branch = false;
slot.mcdc = false;
}
"branch" => slot.branch = true,
"mcdc" => {
slot.branch = true;
slot.mcdc = true;
}
"block" => slot.level = CoverageLevel::Block,
"branch" => slot.level = CoverageLevel::Branch,
"mcdc" => slot.level = CoverageLevel::Mcdc,
_ => return false,
}
}
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::code_stats::CodeStats;
pub use crate::code_stats::{DataTypeKind, FieldInfo, FieldKind, SizeKind, VariantInfo};
use crate::config::{
self, CrateType, FunctionReturn, InstrumentCoverage, OptLevel, OutFileName, OutputType,
RemapPathScopeComponents, SwitchWithOptPath,
self, CoverageLevel, CrateType, FunctionReturn, InstrumentCoverage, OptLevel, OutFileName,
OutputType, RemapPathScopeComponents, SwitchWithOptPath,
};
use crate::config::{ErrorOutputType, Input};
use crate::errors;
Expand Down Expand Up @@ -349,11 +349,13 @@ impl Session {
}

pub fn instrument_coverage_branch(&self) -> bool {
self.instrument_coverage() && self.opts.unstable_opts.coverage_options.branch
self.instrument_coverage()
&& self.opts.unstable_opts.coverage_options.level >= CoverageLevel::Branch
}

pub fn instrument_coverage_mcdc(&self) -> bool {
self.instrument_coverage() && self.opts.unstable_opts.coverage_options.mcdc
self.instrument_coverage()
&& self.opts.unstable_opts.coverage_options.level >= CoverageLevel::Mcdc
}

pub fn is_sanitizer_cfi_enabled(&self) -> bool {
Expand Down
6 changes: 1 addition & 5 deletions src/doc/rustc/src/instrument-coverage.md
Original file line number Diff line number Diff line change
Expand Up @@ -348,11 +348,7 @@ $ llvm-cov report \

## `-Z coverage-options=<options>`

This unstable option provides finer control over some aspects of coverage
instrumentation. Pass one or more of the following values, separated by commas.

- Either `no-branch`, `branch` or `mcdc`
- `branch` enables branch coverage instrumentation and `mcdc` further enables modified condition/decision coverage instrumentation. `no-branch` disables branch coverage instrumentation, which is same as do not pass `branch` or `mcdc`.
[This unstable option is described in the Unstable Book.](../unstable-book/compiler-flags/coverage-options.html)

## Other references

Expand Down
11 changes: 10 additions & 1 deletion src/doc/unstable-book/src/compiler-flags/coverage-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,13 @@ This option controls details of the coverage instrumentation performed by

Multiple options can be passed, separated by commas. Valid options are:

- `no-branch`, `branch` or `mcdc`: `branch` enables branch coverage instrumentation and `mcdc` further enables modified condition/decision coverage instrumentation. `no-branch` disables branch coverage instrumentation as well as mcdc instrumentation, which is same as do not pass `branch` or `mcdc`.
- `block`, `branch`, `mcdc`:
Sets the level of coverage instrumentation.
Setting the level will override any previously-specified level.
- `block` (default):
Blocks in the control-flow graph will be instrumented for coverage.
- `branch`:
In addition to block coverage, also enables branch coverage instrumentation.
- `mcdc`:
In addition to block and branch coverage, also enables MC/DC instrumentation.
(Branch coverage instrumentation may differ in some cases.)
2 changes: 1 addition & 1 deletion tests/ui/instrument-coverage/coverage-options.bad.stderr
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
error: incorrect value `bad` for unstable option `coverage-options` - either `no-branch`, `branch` or `mcdc` was expected
error: incorrect value `bad` for unstable option `coverage-options` - `block` | `branch` | `mcdc` was expected

11 changes: 4 additions & 7 deletions tests/ui/instrument-coverage/coverage-options.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
//@ needs-profiler-support
//@ revisions: branch no-branch bad
//@ revisions: block branch bad
//@ compile-flags -Cinstrument-coverage

//@ [block] check-pass
//@ [block] compile-flags: -Zcoverage-options=block

//@ [branch] check-pass
//@ [branch] compile-flags: -Zcoverage-options=branch

//@ [no-branch] check-pass
//@ [no-branch] compile-flags: -Zcoverage-options=no-branch

//@ [mcdc] check-pass
//@ [mcdc] compile-flags: -Zcoverage-options=mcdc

//@ [bad] check-fail
//@ [bad] compile-flags: -Zcoverage-options=bad

//@ [conflict] check-fail
//@ [conflict] compile-flags: -Zcoverage-options=no-branch,mcdc

fn main() {}

0 comments on commit f9dca46

Please sign in to comment.