Semantics of `-Cinstrument-coverage=all`

[Zalathar]

Proposal

Short summary:

• Make -Cinstrument-coverage an alias for =yes instead of =all.
• Change the documented semantics of =all to allow for potential future changes (but leave the current behaviour intact for now).

---

(copied from rust-lang/rust#117199, which implements the proposed change)

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.

---

Mentors or Reviewers

The work is already done; I'm mostly going through MCP because this is technically a user-visible change (and arguably a breaking one).

Process

The main points of the Major Change Process are as follows:

• File an issue describing the proposal.
• A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
  • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
  • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
• Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

[rustbot]

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/compiler @rust-lang/compiler-contributors

[oli-obk]

@rustbot second

[apiraino]

@rustbot label -final-comment-period +major-change-accepted