1
Fork 0

Rollup merge of #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 #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 #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 commit is contained in:
Matthias Krüger 2024-03-13 06:41:22 +01:00 committed by GitHub
commit 8b9ef3b996
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 105 additions and 124 deletions

View file

@ -134,31 +134,26 @@ pub enum LtoCli {
/// and higher). Nevertheless, there are many variables, depending on options
/// 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.
/// lowering the optimization level, or including/excluding `-C link-dead-code`.
#[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,
}
/// Individual flag values controlled by `-Z coverage-options`.
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
pub struct CoverageOptions {
/// Add branch coverage instrumentation (placeholder flag; not yet implemented).
pub branch: bool,
}
impl Default for CoverageOptions {
fn default() -> Self {
Self { branch: false }
}
}
/// Settings for `-Z instrument-xray` flag.
@ -2718,24 +2713,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 +3181,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 +3256,7 @@ pub(crate) mod dep_tracking {
CodeModel,
TlsModel,
InstrumentCoverage,
CoverageOptions,
InstrumentXRay,
CrateType,
MergeFunctions,

View file

@ -395,7 +395,8 @@ 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 = "`branch` or `no-branch`";
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";
@ -928,21 +929,34 @@ mod parse {
return true;
};
// Parse values that have historically been accepted by stable compilers,
// even though they're currently just aliases for boolean values.
*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,
_ => return false,
};
*slot = enabled;
}
true
}
pub(crate) fn parse_instrument_xray(
slot: &mut Option<InstrumentXRay>,
v: Option<&str>,
@ -1445,14 +1459,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<String> = (Vec::new(), parse_list, [UNTRACKED],
@ -1574,6 +1583,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<String> = (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],

View file

@ -353,15 +353,7 @@ impl Session {
}
pub fn instrument_coverage_branch(&self) -> bool {
self.opts.cg.instrument_coverage() == InstrumentCoverage::Branch
}
pub fn instrument_coverage_except_unused_generics(&self) -> bool {
self.opts.cg.instrument_coverage() == InstrumentCoverage::ExceptUnusedGenerics
}
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.branch
}
pub fn is_sanitizer_cfi_enabled(&self) -> bool {