From e82af0bdac93f549d538ccac974250e9ae81071b Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 8 Mar 2024 18:07:04 +1100 Subject: [PATCH] coverage: Migrate unstable options to new flag `-Zcoverage-options` Moving these unstable values over to a separate unstable flag has a few benefits: - No need to distinguish stable/unstable values of `-Cinstrument-coverage`. - Multiple options can be enabled/disabled independently. - New behavior can be prototyped behind its own flag, and then enabled by default when ready. --- compiler/rustc_session/src/config.rs | 69 ++++++++----------- compiler/rustc_session/src/options.rs | 44 +++++++----- compiler/rustc_session/src/session.rs | 6 +- src/doc/rustc/src/instrument-coverage.md | 22 +++--- tests/coverage/except-unused.default.coverage | 4 +- .../coverage/except-unused.functions.coverage | 4 +- .../coverage/except-unused.generics.coverage | 4 +- tests/coverage/except-unused.rs | 4 +- .../instrument-coverage/bad-value.bad.stderr | 2 +- .../bad-value.blank.stderr | 2 +- .../coverage-options.bad.stderr | 2 + .../instrument-coverage/coverage-options.rs | 14 ++++ .../unstable.branch.stderr | 2 - .../unstable.except-unused-functions.stderr | 2 - .../unstable.except-unused-generics.stderr | 2 - tests/ui/instrument-coverage/unstable.rs | 6 -- 16 files changed, 98 insertions(+), 91 deletions(-) create mode 100644 tests/ui/instrument-coverage/coverage-options.bad.stderr create mode 100644 tests/ui/instrument-coverage/coverage-options.rs delete mode 100644 tests/ui/instrument-coverage/unstable.branch.stderr delete mode 100644 tests/ui/instrument-coverage/unstable.except-unused-functions.stderr delete mode 100644 tests/ui/instrument-coverage/unstable.except-unused-generics.stderr delete mode 100644 tests/ui/instrument-coverage/unstable.rs diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index b947ac818057b..2bf9dfaa1598b 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -135,30 +135,32 @@ pub enum LtoCli { /// selected, code structure, and enabled attributes. If errors are encountered, /// either while compiling or when generating `llvm-cov show` reports, consider /// lowering the optimization level, including or excluding `-C link-dead-code`, -/// or using `-Zunstable-options -C instrument-coverage=except-unused-functions` -/// or `-Zunstable-options -C instrument-coverage=except-unused-generics`. -/// -/// Note that `ExceptUnusedFunctions` means: When `mapgen.rs` generates the -/// coverage map, it will not attempt to generate synthetic functions for unused -/// (and not code-generated) functions (whether they are generic or not). As a -/// result, non-codegenned functions will not be included in the coverage map, -/// and will not appear, as covered or uncovered, in coverage reports. -/// -/// `ExceptUnusedGenerics` will add synthetic functions to the coverage map, -/// unless the function has type parameters. +/// or using `-Z coverage-options=no-unused-functions` or +/// `-Z coverage-options=no-unused-generics`. #[derive(Clone, Copy, PartialEq, Hash, Debug)] pub enum InstrumentCoverage { /// `-C instrument-coverage=no` (or `off`, `false` etc.) No, /// `-C instrument-coverage` or `-C instrument-coverage=yes` Yes, - /// Additionally, instrument branches and output branch coverage. - /// `-Zunstable-options -C instrument-coverage=branch` - Branch, - /// `-Zunstable-options -C instrument-coverage=except-unused-generics` - ExceptUnusedGenerics, - /// `-Zunstable-options -C instrument-coverage=except-unused-functions` - ExceptUnusedFunctions, +} + +/// Settings for the `-Z coverage-options` flag. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub struct CoverageOptions { + /// Add branch coverage instrumentation (placeholder flag; not yet implemented). + pub branch: bool, + /// Emit coverage mappings for unused functions. + pub unused_functions: bool, + /// When emitting coverage mappings for unused functions, include unused + /// generic functions. + pub unused_generics: bool, +} + +impl Default for CoverageOptions { + fn default() -> Self { + Self { branch: false, unused_functions: true, unused_generics: true } + } } /// Settings for `-Z instrument-xray` flag. @@ -2718,24 +2720,6 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M } } - // Check for unstable values of `-C instrument-coverage`. - // This is what prevents them from being used on stable compilers. - match cg.instrument_coverage { - // Stable values: - InstrumentCoverage::Yes | InstrumentCoverage::No => {} - // Unstable values: - InstrumentCoverage::Branch - | InstrumentCoverage::ExceptUnusedFunctions - | InstrumentCoverage::ExceptUnusedGenerics => { - if !unstable_opts.unstable_options { - early_dcx.early_fatal( - "`-C instrument-coverage=branch` and `-C instrument-coverage=except-*` \ - require `-Z unstable-options`", - ); - } - } - } - if cg.instrument_coverage != InstrumentCoverage::No { if cg.profile_generate.enabled() || cg.profile_use.is_some() { early_dcx.early_fatal( @@ -3204,12 +3188,12 @@ pub enum WasiExecModel { /// how the hash should be calculated when adding a new command-line argument. pub(crate) mod dep_tracking { use super::{ - BranchProtection, CFGuard, CFProtection, CollapseMacroDebuginfo, CrateType, DebugInfo, - DebugInfoCompression, ErrorOutputType, FunctionReturn, InliningThreshold, - InstrumentCoverage, InstrumentXRay, LinkerPluginLto, LocationDetail, LtoCli, - NextSolverConfig, OomStrategy, OptLevel, OutFileName, OutputType, OutputTypes, Polonius, - RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm, SplitDwarfKind, - SwitchWithOptPath, SymbolManglingVersion, WasiExecModel, + BranchProtection, CFGuard, CFProtection, CollapseMacroDebuginfo, CoverageOptions, + CrateType, DebugInfo, DebugInfoCompression, ErrorOutputType, FunctionReturn, + InliningThreshold, InstrumentCoverage, InstrumentXRay, LinkerPluginLto, LocationDetail, + LtoCli, NextSolverConfig, OomStrategy, OptLevel, OutFileName, OutputType, OutputTypes, + Polonius, RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm, + SplitDwarfKind, SwitchWithOptPath, SymbolManglingVersion, WasiExecModel, }; use crate::lint; use crate::utils::NativeLib; @@ -3279,6 +3263,7 @@ pub(crate) mod dep_tracking { CodeModel, TlsModel, InstrumentCoverage, + CoverageOptions, InstrumentXRay, CrateType, MergeFunctions, diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index ea4b8f2463e7f..119e671413a75 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -395,7 +395,9 @@ mod desc { pub const parse_linker_flavor: &str = ::rustc_target::spec::LinkerFlavorCli::one_of(); 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 = "either a boolean (`yes`, `no`, `on`, `off`, etc) or (unstable) one of `branch`, `except-unused-generics`, `except-unused-functions`"; + pub const parse_instrument_coverage: &str = parse_bool; + pub const parse_coverage_options: &str = + "one or more of `branch`, `no-unused-functions`, `no-unused-generics` (comma separated)"; 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"; @@ -930,19 +932,32 @@ mod parse { *slot = match v { "all" => InstrumentCoverage::Yes, - "branch" => InstrumentCoverage::Branch, - "except-unused-generics" | "except_unused_generics" => { - InstrumentCoverage::ExceptUnusedGenerics - } - "except-unused-functions" | "except_unused_functions" => { - InstrumentCoverage::ExceptUnusedFunctions - } "0" => InstrumentCoverage::No, _ => return false, }; true } + pub(crate) fn parse_coverage_options(slot: &mut CoverageOptions, v: Option<&str>) -> bool { + let Some(v) = v else { return true }; + + for option in v.split(',') { + let (option, enabled) = match option.strip_prefix("no-") { + Some(without_no) => (without_no, false), + None => (option, true), + }; + let slot = match option { + "branch" => &mut slot.branch, + "unused-functions" => &mut slot.unused_functions, + "unused-generics" => &mut slot.unused_generics, + _ => return false, + }; + *slot = enabled; + } + + true + } + pub(crate) fn parse_instrument_xray( slot: &mut Option, v: Option<&str>, @@ -1445,14 +1460,9 @@ options! { "set the threshold for inlining a function"), #[rustc_lint_opt_deny_field_access("use `Session::instrument_coverage` instead of this field")] instrument_coverage: InstrumentCoverage = (InstrumentCoverage::No, parse_instrument_coverage, [TRACKED], - "instrument the generated code to support LLVM source-based code coverage \ - reports (note, the compiler build config must include `profiler = true`); \ - implies `-C symbol-mangling-version=v0`. Optional values are: - `=no` `=n` `=off` `=false` (default) - `=yes` `=y` `=on` `=true` (implicit value) - `=branch` (unstable) - `=except-unused-generics` (unstable) - `=except-unused-functions` (unstable)"), + "instrument the generated code to support LLVM source-based code coverage reports \ + (note, the compiler build config must include `profiler = true`); \ + implies `-C symbol-mangling-version=v0`"), link_arg: (/* redirected to link_args */) = ((), parse_string_push, [UNTRACKED], "a single extra argument to append to the linker invocation (can be used several times)"), link_args: Vec = (Vec::new(), parse_list, [UNTRACKED], @@ -1574,6 +1584,8 @@ options! { "set option to collapse debuginfo for macros"), combine_cgu: bool = (false, parse_bool, [TRACKED], "combine CGUs into a single one"), + coverage_options: CoverageOptions = (CoverageOptions::default(), parse_coverage_options, [TRACKED], + "control details of coverage instrumentation"), crate_attr: Vec = (Vec::new(), parse_string_push, [TRACKED], "inject the given attribute in the crate"), cross_crate_inline_threshold: InliningThreshold = (InliningThreshold::Sometimes(100), parse_inlining_threshold, [TRACKED], diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index ab636b14d19a4..8a318c3c6e1f3 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -353,15 +353,15 @@ impl Session { } pub fn instrument_coverage_branch(&self) -> bool { - self.opts.cg.instrument_coverage() == InstrumentCoverage::Branch + self.instrument_coverage() && self.opts.unstable_opts.coverage_options.branch } pub fn instrument_coverage_except_unused_generics(&self) -> bool { - self.opts.cg.instrument_coverage() == InstrumentCoverage::ExceptUnusedGenerics + self.instrument_coverage() && !self.opts.unstable_opts.coverage_options.unused_generics } pub fn instrument_coverage_except_unused_functions(&self) -> bool { - self.opts.cg.instrument_coverage() == InstrumentCoverage::ExceptUnusedFunctions + self.instrument_coverage() && !self.opts.unstable_opts.coverage_options.unused_functions } pub fn is_sanitizer_cfi_enabled(&self) -> bool { diff --git a/src/doc/rustc/src/instrument-coverage.md b/src/doc/rustc/src/instrument-coverage.md index 2f93252eddcd7..65c178def94b2 100644 --- a/src/doc/rustc/src/instrument-coverage.md +++ b/src/doc/rustc/src/instrument-coverage.md @@ -346,14 +346,20 @@ $ llvm-cov report \ more fine-grained coverage options are added. Using this value is currently not recommended. -### Unstable values - -- `-Z unstable-options -C instrument-coverage=branch`: - Placeholder for potential branch coverage support in the future. -- `-Z unstable-options -C instrument-coverage=except-unused-generics`: - Instrument all functions except unused generics. -- `-Z unstable-options -C instrument-coverage=except-unused-functions`: - Instrument only used (called) functions and instantiated generic functions. +## `-Z coverage-options=` + +This unstable option provides finer control over some aspects of coverage +instrumentation. Pass one or more of the following values, separated by commas. + +- `branch` or `no-branch` + - Placeholder for potential branch coverage support in the future. +- `unused-functions` or `no-unused-functions` + - When enabled (default), coverage metadata includes entries for unused functions. + - When disabled, unused functions are not included in coverage metadata. +- `unused-generics` or `no-unused-generics` + - When enabled (default), coverage metadata includes entries for unused generic functions. + - When disabled, unused generic functions are not included in coverage metadata. + - No effect if `no-unused-functions` is also set. ## Other references diff --git a/tests/coverage/except-unused.default.coverage b/tests/coverage/except-unused.default.coverage index 43befa4745e60..961daab169e25 100644 --- a/tests/coverage/except-unused.default.coverage +++ b/tests/coverage/except-unused.default.coverage @@ -3,8 +3,8 @@ LL| | LL| |//@ edition: 2021 LL| |//@ revisions: default functions generics - LL| |//@ [functions] compile-flags: -Zunstable-options -Cinstrument-coverage=except-unused-functions - LL| |//@ [generics] compile-flags: -Zunstable-options -Cinstrument-coverage=except-unused-generics + LL| |//@ [functions] compile-flags: -Zcoverage-options=no-unused-functions + LL| |//@ [generics] compile-flags: -Zcoverage-options=no-unused-generics LL| | LL| 1|fn used_generic(_x: T) { LL| 1| println!("used_generic"); diff --git a/tests/coverage/except-unused.functions.coverage b/tests/coverage/except-unused.functions.coverage index 465d2461a7bfd..ea0740faed6b4 100644 --- a/tests/coverage/except-unused.functions.coverage +++ b/tests/coverage/except-unused.functions.coverage @@ -3,8 +3,8 @@ LL| | LL| |//@ edition: 2021 LL| |//@ revisions: default functions generics - LL| |//@ [functions] compile-flags: -Zunstable-options -Cinstrument-coverage=except-unused-functions - LL| |//@ [generics] compile-flags: -Zunstable-options -Cinstrument-coverage=except-unused-generics + LL| |//@ [functions] compile-flags: -Zcoverage-options=no-unused-functions + LL| |//@ [generics] compile-flags: -Zcoverage-options=no-unused-generics LL| | LL| 1|fn used_generic(_x: T) { LL| 1| println!("used_generic"); diff --git a/tests/coverage/except-unused.generics.coverage b/tests/coverage/except-unused.generics.coverage index dd3af7971711d..ca3b1e256b675 100644 --- a/tests/coverage/except-unused.generics.coverage +++ b/tests/coverage/except-unused.generics.coverage @@ -3,8 +3,8 @@ LL| | LL| |//@ edition: 2021 LL| |//@ revisions: default functions generics - LL| |//@ [functions] compile-flags: -Zunstable-options -Cinstrument-coverage=except-unused-functions - LL| |//@ [generics] compile-flags: -Zunstable-options -Cinstrument-coverage=except-unused-generics + LL| |//@ [functions] compile-flags: -Zcoverage-options=no-unused-functions + LL| |//@ [generics] compile-flags: -Zcoverage-options=no-unused-generics LL| | LL| 1|fn used_generic(_x: T) { LL| 1| println!("used_generic"); diff --git a/tests/coverage/except-unused.rs b/tests/coverage/except-unused.rs index af85cb6f70bb0..0814a85273add 100644 --- a/tests/coverage/except-unused.rs +++ b/tests/coverage/except-unused.rs @@ -3,8 +3,8 @@ //@ edition: 2021 //@ revisions: default functions generics -//@ [functions] compile-flags: -Zunstable-options -Cinstrument-coverage=except-unused-functions -//@ [generics] compile-flags: -Zunstable-options -Cinstrument-coverage=except-unused-generics +//@ [functions] compile-flags: -Zcoverage-options=no-unused-functions +//@ [generics] compile-flags: -Zcoverage-options=no-unused-generics fn used_generic(_x: T) { println!("used_generic"); diff --git a/tests/ui/instrument-coverage/bad-value.bad.stderr b/tests/ui/instrument-coverage/bad-value.bad.stderr index 0411a1f98a3ee..d3415fb35d062 100644 --- a/tests/ui/instrument-coverage/bad-value.bad.stderr +++ b/tests/ui/instrument-coverage/bad-value.bad.stderr @@ -1,2 +1,2 @@ -error: incorrect value `bad-value` for codegen option `instrument-coverage` - either a boolean (`yes`, `no`, `on`, `off`, etc) or (unstable) one of `branch`, `except-unused-generics`, `except-unused-functions` was expected +error: incorrect value `bad-value` for codegen option `instrument-coverage` - one of: `y`, `yes`, `on`, `true`, `n`, `no`, `off` or `false` was expected diff --git a/tests/ui/instrument-coverage/bad-value.blank.stderr b/tests/ui/instrument-coverage/bad-value.blank.stderr index b3a8e7cf94784..04c0eab28489d 100644 --- a/tests/ui/instrument-coverage/bad-value.blank.stderr +++ b/tests/ui/instrument-coverage/bad-value.blank.stderr @@ -1,2 +1,2 @@ -error: incorrect value `` for codegen option `instrument-coverage` - either a boolean (`yes`, `no`, `on`, `off`, etc) or (unstable) one of `branch`, `except-unused-generics`, `except-unused-functions` was expected +error: incorrect value `` for codegen option `instrument-coverage` - one of: `y`, `yes`, `on`, `true`, `n`, `no`, `off` or `false` was expected diff --git a/tests/ui/instrument-coverage/coverage-options.bad.stderr b/tests/ui/instrument-coverage/coverage-options.bad.stderr new file mode 100644 index 0000000000000..9077d0aa38e74 --- /dev/null +++ b/tests/ui/instrument-coverage/coverage-options.bad.stderr @@ -0,0 +1,2 @@ +error: incorrect value `branch,bad` for unstable option `coverage-options` - one or more of `branch`, `no-unused-functions`, `no-unused-generics` (comma separated) was expected + diff --git a/tests/ui/instrument-coverage/coverage-options.rs b/tests/ui/instrument-coverage/coverage-options.rs new file mode 100644 index 0000000000000..53a711551ece3 --- /dev/null +++ b/tests/ui/instrument-coverage/coverage-options.rs @@ -0,0 +1,14 @@ +//@ needs-profiler-support +//@ revisions: comma one bad +//@ compile-flags -Cinstrument-coverage + +//@ [comma] check-pass +//@ [comma] compile-flags: -Zcoverage-options=branch,no-unused-functions + +//@ [one] check-pass +//@ [one] compile-flags: -Zcoverage-options=no-branch + +//@ [bad] check-fail +//@ [bad] compile-flags: -Zcoverage-options=branch,bad + +fn main() {} diff --git a/tests/ui/instrument-coverage/unstable.branch.stderr b/tests/ui/instrument-coverage/unstable.branch.stderr deleted file mode 100644 index acc633a2a6d2f..0000000000000 --- a/tests/ui/instrument-coverage/unstable.branch.stderr +++ /dev/null @@ -1,2 +0,0 @@ -error: `-C instrument-coverage=branch` and `-C instrument-coverage=except-*` require `-Z unstable-options` - diff --git a/tests/ui/instrument-coverage/unstable.except-unused-functions.stderr b/tests/ui/instrument-coverage/unstable.except-unused-functions.stderr deleted file mode 100644 index acc633a2a6d2f..0000000000000 --- a/tests/ui/instrument-coverage/unstable.except-unused-functions.stderr +++ /dev/null @@ -1,2 +0,0 @@ -error: `-C instrument-coverage=branch` and `-C instrument-coverage=except-*` require `-Z unstable-options` - diff --git a/tests/ui/instrument-coverage/unstable.except-unused-generics.stderr b/tests/ui/instrument-coverage/unstable.except-unused-generics.stderr deleted file mode 100644 index acc633a2a6d2f..0000000000000 --- a/tests/ui/instrument-coverage/unstable.except-unused-generics.stderr +++ /dev/null @@ -1,2 +0,0 @@ -error: `-C instrument-coverage=branch` and `-C instrument-coverage=except-*` require `-Z unstable-options` - diff --git a/tests/ui/instrument-coverage/unstable.rs b/tests/ui/instrument-coverage/unstable.rs deleted file mode 100644 index ea2279aaf0cde..0000000000000 --- a/tests/ui/instrument-coverage/unstable.rs +++ /dev/null @@ -1,6 +0,0 @@ -//@ revisions: branch except-unused-functions except-unused-generics -//@ [branch] compile-flags: -Cinstrument-coverage=branch -//@ [except-unused-functions] compile-flags: -Cinstrument-coverage=except-unused-functions -//@ [except-unused-generics] compile-flags: -Cinstrument-coverage=except-unused-generics - -fn main() {}