Auto merge of rust-lang#124507 - Zalathar:coverage-level, r=compiler-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`

Author: bors

compiler/rustc_interface/src/tests.rs

@@ -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;
@@ -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);

compiler/rustc_session/src/config.rs

@@ -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.

compiler/rustc_session/src/options.rs

@@ -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";
@@ -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,
             }
         }

compiler/rustc_session/src/session.rs

@@ -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;
@@ -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 {

src/doc/rustc/src/instrument-coverage.md

@@ -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
 

src/doc/unstable-book/src/compiler-flags/coverage-options.md

@@ -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.)

tests/ui/instrument-coverage/coverage-options.bad.stderr

@@ -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
 

tests/ui/instrument-coverage/coverage-options.rs

@@ -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() {}