Rollup merge of #117199 - Zalathar:instrument-coverage-on, r=oli-obk,…

…Nadrieril

Change the documented implicit value of `-C instrument-coverage` to `=yes`

The option-value parser for `-Cinstrument-coverage=` currently accepts the following stable values:

- `all` (implicit value of plain `-Cinstrument-coverage`)
- `yes`, `y`, `on`, `true` (undocumented aliases for `all`)
- `off` (default; same as not specifying `-Cinstrument-coverage`)
- `no`, `n`, `false`, `0` (undocumented aliases for `off`)

I'd like to rearrange and re-document the stable values as follows:

- `no` (default; same as not specifying `-Cinstrument-coverage`)
- `n`, `off`, `false` (documented aliases for `no`)
- `0` (undocumented alias for `no`)
- `yes` (implicit value of plain `-Cinstrument-coverage`)
- `y`, `on`, `true` (documented aliases for `yes`)
- `all` (documented as *currently* an alias for `yes` that may change; discouraged but not deprecated)

The main changes being:

- Documented default value changes from `off` to `no`
- Documented implicit value changes from `all` to `yes`
- Other boolean aliases (`n`, `off`, `false`, `y`, `on`, `true`) are explicitly documented
- `all` remains currently an alias for `yes`, but is explicitly documented as being able to change in the future
- `0` remains an undocumented but stable alias for `no`
- The actual behaviour of coverage instrumentation does not change

# Why?

The choice of `all` as the implicit value only really makes sense in the context of the unstable `except-unused-functions` and `except-unused-generics` values. That arrangement was fine for an unstable flag, but it's confusing for a stable flag whose only other stable value is `off`, and will only become more confusing if we eventually want to stabilize other fine-grained coverage option values.

(Currently I'm not aware of any plans to stabilize other coverage option values, but that's why I think now is a fine time to make this change, well before anyone actually has to care about it.)

For example, if we ever add support for opt-in instrumentation of things that are *not* instrumented by `-Cinstrument-coverage` by default, it will be very strange for the `all` value to not actually instrument all things that we know how to instrument.

# Compatibility impact

Because this is not a functional change, there is no immediate compatibility impact. However, changing the documented semantics of `all` opens up the possibility of future changes that could be considered retroactively breaking.

I don't think this is going to be a big deal in practice, for a few reasons:

- The exact behaviour of coverage instrumentation is allowed to change, so changing the behaviour of `all` is not a *stability-breaking* change, as long as it still exists and does something reasonable.
- `-Cinstrument-coverage` is mainly used by tools or scripts that can be easily updated if necessary. It's unusual for users to pass the flag directly, because processing the profiler output is complicated enough that tools/scripts tend to be necessary anyway.
- Most people who are using coverage are probably relying on `-Cinstrument-coverage` rather than explicitly passing `-Cinstrument-coverage=all`, so the number of users actually affected by this change is likely to be low, and plausibly zero.

compiler/rustc_interface/src/tests.rs

@@ -590,7 +590,7 @@ fn test_codegen_options_tracking_hash() {
     tracked!(force_frame_pointers, Some(false));
     tracked!(force_unwind_tables, Some(true));
     tracked!(inline_threshold, Some(0xf007ba11));
-    tracked!(instrument_coverage, InstrumentCoverage::All);
+    tracked!(instrument_coverage, InstrumentCoverage::Yes);
     tracked!(link_dead_code, Some(true));
     tracked!(linker_plugin_lto, LinkerPluginLto::LinkerPluginAuto);
     tracked!(llvm_args, vec![String::from("1"), String::from("2")]);

compiler/rustc_session/src/config.rs

@@ -146,17 +146,17 @@ pub enum LtoCli {
 /// unless the function has type parameters.
 #[derive(Clone, Copy, PartialEq, Hash, Debug)]
 pub enum InstrumentCoverage {
-    /// Default `-C instrument-coverage` or `-C instrument-coverage=statement`
-    All,
+    /// `-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,
-    /// `-C instrument-coverage=off` (or `no`, etc.)
-    Off,
 }
 
 /// Settings for `-Z instrument-xray` flag.
@@ -2722,7 +2722,7 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M
     // This is what prevents them from being used on stable compilers.
     match cg.instrument_coverage {
         // Stable values:
-        InstrumentCoverage::All | InstrumentCoverage::Off => {}
+        InstrumentCoverage::Yes | InstrumentCoverage::No => {}
         // Unstable values:
         InstrumentCoverage::Branch
         | InstrumentCoverage::ExceptUnusedFunctions
@@ -2736,7 +2736,7 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M
         }
     }
 
-    if cg.instrument_coverage != InstrumentCoverage::Off {
+    if cg.instrument_coverage != InstrumentCoverage::No {
         if cg.profile_generate.enabled() || cg.profile_use.is_some() {
             early_dcx.early_fatal(
                 "option `-C instrument-coverage` is not compatible with either `-C profile-use` \

compiler/rustc_session/src/options.rs

@@ -394,8 +394,7 @@ 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 =
-        "`all` (default), `branch`, `except-unused-generics`, `except-unused-functions`, or `off`";
+    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_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";
@@ -918,26 +917,26 @@ mod parse {
         if v.is_some() {
             let mut bool_arg = false;
             if parse_bool(&mut bool_arg, v) {
-                *slot = if bool_arg { InstrumentCoverage::All } else { InstrumentCoverage::Off };
+                *slot = if bool_arg { InstrumentCoverage::Yes } else { InstrumentCoverage::No };
                 return true;
             }
         }
 
         let Some(v) = v else {
-            *slot = InstrumentCoverage::All;
+            *slot = InstrumentCoverage::Yes;
             return true;
         };
 
         *slot = match v {
-            "all" => InstrumentCoverage::All,
+            "all" => InstrumentCoverage::Yes,
             "branch" => InstrumentCoverage::Branch,
             "except-unused-generics" | "except_unused_generics" => {
                 InstrumentCoverage::ExceptUnusedGenerics
             }
             "except-unused-functions" | "except_unused_functions" => {
                 InstrumentCoverage::ExceptUnusedFunctions
             }
-            "off" | "no" | "n" | "false" | "0" => InstrumentCoverage::Off,
+            "0" => InstrumentCoverage::No,
             _ => return false,
         };
         true
@@ -1444,15 +1443,15 @@ options! {
     inline_threshold: Option<u32> = (None, parse_opt_number, [TRACKED],
         "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::Off, parse_instrument_coverage, [TRACKED],
+    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:
-        `=all` (implicit value)
-        `=branch`
-        `=except-unused-generics`
-        `=except-unused-functions`
-        `=off` (default)"),
+        `=no` `=n` `=off` `=false` (default)
+        `=yes` `=y` `=on` `=true` (implicit value)
+        `=branch` (unstable)
+        `=except-unused-generics` (unstable)
+        `=except-unused-functions` (unstable)"),
     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],

compiler/rustc_session/src/session.rs

@@ -352,7 +352,7 @@ impl Session {
     }
 
     pub fn instrument_coverage(&self) -> bool {
-        self.opts.cg.instrument_coverage() != InstrumentCoverage::Off
+        self.opts.cg.instrument_coverage() != InstrumentCoverage::No
     }
 
     pub fn instrument_coverage_branch(&self) -> bool {

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

@@ -331,10 +331,29 @@ $ llvm-cov report \
 
 ## `-C instrument-coverage=<options>`
 
--   `-C instrument-coverage=all`: Instrument all functions, including unused functions and unused generics. (This is the same as `-C instrument-coverage`, with no value.)
--   `-C instrument-coverage=off`: Do not instrument any functions. (This is the same as simply not including the `-C instrument-coverage` option.)
--   `-Zunstable-options -C instrument-coverage=except-unused-generics`: Instrument all functions except unused generics.
--   `-Zunstable-options -C instrument-coverage=except-unused-functions`: Instrument only used (called) functions and instantiated generic functions.
+- `-C instrument-coverage=no` (or `n`/`off`/`false`):
+  Don't enable coverage instrumentation. No functions will be instrumented for coverage.
+  - This is the same as not using the `-C instrument-coverage` flag at all.
+- `-C instrument-coverage=yes` (or `y`/`on`/`true`):
+  Enable coverage instrumentation with the default behaviour.
+  Currently this instruments all functions, including unused functions and unused generics.
+  - This is the same as `-C instrument-coverage` with no value.
+
+### Other values
+
+- `-C instrument-coverage=all`:
+  Currently an alias for `yes`, but may behave differently in the future if
+  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.
 
 ## Other references
 

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

@@ -1,2 +1,2 @@
-error: incorrect value `bad-value` for codegen option `instrument-coverage` - `all` (default), `branch`, `except-unused-generics`, `except-unused-functions`, or `off` was expected
+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
 

tests/ui/instrument-coverage/bad-value.blank.stderr

@@ -1,2 +1,2 @@
-error: incorrect value `` for codegen option `instrument-coverage` - `all` (default), `branch`, `except-unused-generics`, `except-unused-functions`, or `off` was expected
+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