Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

schroedingers instrument-coverage #116980

Closed
matthiaskrgr opened this issue Oct 20, 2023 · 3 comments · Fixed by #117111
Closed

schroedingers instrument-coverage #116980

matthiaskrgr opened this issue Oct 20, 2023 · 3 comments · Fixed by #117111
Labels
A-CLI Area: Command-line interface (CLI) to the compiler A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@matthiaskrgr
Copy link
Member

It seems that we have both -Zinstrument-coverage=val and -Cinstrument-coverage=val at the same time, since its already stabilized I guess we could remove the corresponding -Zflag?
@matthiaskrgr matthiaskrgr added C-bug Category: This is a bug. A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) A-CLI Area: Command-line interface (CLI) to the compiler labels Oct 20, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 20, 2023
@saethlin saethlin added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 20, 2023
@Zalathar
Copy link
Contributor

Zalathar commented Oct 21, 2023 •
edited
Loading

It looks like the stabilization PR (#90132) deliberately left in the -Z flag to ease migration for people who were already using coverage.

Given that it's now been stable for almost 2 years, and the -C flag supports the unstable values (via -Zunstable-options) I can't imagine anyone would object too loudly to its removal now.

@Zalathar
Copy link
Contributor

Note that the same thing was also done when stabilizing -Csymbol-mangling-version=v0 at around the same time, so that flag is probably in the same boat.

@Zalathar
Copy link
Contributor

Also it looks like the stabilization PR also wired up -Zinstrument-coverage to issue a deprecation warning on use.

This was referenced Oct 22, 2023
Merged
Merged
@bors bors closed this as completed in 24254d2 Oct 25, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 25, 2023
@rust-timer
Unrolled build for rust-lang#117111
a98c7df 
Rollup merge of rust-lang#117111 - Zalathar:zinstrument, r=compiler-errors

Remove support for alias `-Z instrument-coverage`

This flag was stabilized in rustc 1.60.0 (2022-04-07) as `-C instrument-coverage`, but the old unstable flag was kept around (with a warning) as an alias to ease migration.

It should now be reasonable to remove the somewhat tricky code that implemented that alias.

Fixes rust-lang#116980.
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
A-CLI Area: Command-line interface (CLI) to the compiler A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove support for alias -Z instrument-coverage Zalathar/rust
4 participants
@matthiaskrgr @saethlin @Zalathar @rustbot