Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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