Disallows `#![feature(no_coverage)]` on stable and beta (using standard crate-level gating) by richkadel · Pull Request #84871 · rust-lang/rust

richkadel · 2021-05-03T15:56:20Z

Removes the function-level feature gating solution originally implemented, and solves the same problem using allow_internal_unstable, so normal crate-level feature gating mechanism can still be used (which disallows the feature on stable and beta).

I tested this, building the compiler with and without CFG_DISABLE_UNSTABLE_FEATURES=1

With unstable features disabled, I get the expected result as shown here:

$ ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc src/test/run-make-fulldeps/coverage/no_cov_crate.rs
error[E0554]: `#![feature]` may not be used on the dev release channel
--> src/test/run-make-fulldeps/coverage/no_cov_crate.rs:2:1
|
2 |#![feature(no_coverage)]| ^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to previous error
For more information about this error, try `rustc --explain E0554`.

r? @Mark-Simulacrum
cc: @tmandry @wesleywiser

ghost · 2021-05-03T16:57:44Z

This does not seem like what's described in #84836 (comment) (emphasis mine)?

It should be possible to adjust the normal feature gating to correctly handle macro expanded sources - that is, allow the built-in derives to generate it without it being stabilized. We do this for the Structural trait impls somehow, for example; possibly enough to set the span appropriately.

The macro generates impl for feature-gated StructuralEq without expanding to any #[feature]: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=7d5fca8b78b2ce2dcc18c750d60af69a (click "TOOLS" -> "Expand macros")

richkadel · 2021-05-03T17:43:03Z

This does not seem like what's described in #84836 (comment) (emphasis mine)?

No, probably not. I don't fully understand the option Mark described, and I'm not sure if it's a general solution, or one that will only work for the core library.

But this solution has the desired effect.

tmandry · 2021-05-05T01:26:04Z

This will work for now but given #84836 (comment) I don't think we should continue using this mechanism without an MCP.

This wouldn't work anyway if we decided to stabilize -Zinstrument-coverage without the no_coverage feature.

richkadel · 2021-05-05T02:47:57Z

I just uploaded a revised implementation that I believe meets everyone's expectations. Per @nagisa 's hints, I figured out how to apply the feature gating at the crate level, and used allow_internal_unstable when injecting the derived method from Eq.

I updated the ui test to include a struct that derives Eq to make sure it still fails on #[no_coverage] applied elsewhere.

Tests with #[no_coverage] that also enable the feature still work on nightly, and they still fail on beta/stable (validated using the same method described in this PR description above).

nagisa · 2021-05-05T09:56:41Z

This doesn't need to be conditional on is_nightly_build, does it?

I'll try to change it and verify it still builds core and still disables use on beta/stable.

nagisa · 2021-05-05T09:57:49Z

Can you adjust the description and the title of the PR please?

richkadel · 2021-05-05T14:46:22Z

Can you adjust the description and the title of the PR please?

I added an exclamation point to the attribute in the title.

The description looks fine to me. (I had already removed the function-level example.)

Let me know if I'm missing something.

using allow_internal_unstable (as recommended) Fixes: rust-lang#84836 ```shell $ ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc src/test/run-make-fulldeps/coverage/no_cov_crate.rs error[E0554]: `#![feature]` may not be used on the dev release channel --> src/test/run-make-fulldeps/coverage/no_cov_crate.rs:2:1 | 2 | #![feature(no_coverage)] | ^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error For more information about this error, try `rustc --explain E0554`. ```

richkadel · 2021-05-05T14:56:46Z

@nagisa - I think this is ready:

I addressed your comment in eq.rs and removed the conditional (verified it still blocks on beta/stable, and it still works otherwise)
I updated the PR title, and the commit message. I had already updated the PR commit message last night.
I squashed the commits

Thanks

nagisa · 2021-05-05T16:48:22Z

Let me know if I'm missing something.

Ah, I meant that its no longer precisely about disallowing the attribute on stable/beta (in particular no code specifically addresses this particular point) but more like making no_coverage use the usual feature gating infrastructure. But I don't care too strongly either way.

@bors r+

bors · 2021-05-05T16:48:23Z

📌 Commit 3584c1d has been approved by nagisa

richkadel · 2021-05-05T17:04:56Z

Oh, I gotcha. Done.

…, r=nagisa Disallows `#![feature(no_coverage)]` on stable and beta (using standard crate-level gating) Fixes: rust-lang#84836 Removes the function-level feature gating solution originally implemented, and solves the same problem using `allow_internal_unstable`, so normal crate-level feature gating mechanism can still be used (which disallows the feature on stable and beta). I tested this, building the compiler with and without `CFG_DISABLE_UNSTABLE_FEATURES=1` With unstable features disabled, I get the expected result as shown here: ```shell $ ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc src/test/run-make-fulldeps/coverage/no_cov_crate.rs error[E0554]: `#![feature]` may not be used on the dev release channel --> src/test/run-make-fulldeps/coverage/no_cov_crate.rs:2:1 | 2 | #![feature(no_coverage)] | ^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error For more information about this error, try `rustc --explain E0554`. ``` r? `@Mark-Simulacrum` cc: `@tmandry` `@wesleywiser`

…, r=nagisa Disallows `#![feature(no_coverage)]` on stable and beta (using standard crate-level gating) Fixes: rust-lang#84836 Removes the function-level feature gating solution originally implemented, and solves the same problem using `allow_internal_unstable`, so normal crate-level feature gating mechanism can still be used (which disallows the feature on stable and beta). I tested this, building the compiler with and without `CFG_DISABLE_UNSTABLE_FEATURES=1` With unstable features disabled, I get the expected result as shown here: ```shell $ ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc src/test/run-make-fulldeps/coverage/no_cov_crate.rs error[E0554]: `#![feature]` may not be used on the dev release channel --> src/test/run-make-fulldeps/coverage/no_cov_crate.rs:2:1 | 2 | #![feature(no_coverage)] | ^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error For more information about this error, try `rustc --explain E0554`. ``` r? ``@Mark-Simulacrum`` cc: ``@tmandry`` ``@wesleywiser``

…, r=nagisa Disallows `#![feature(no_coverage)]` on stable and beta (using standard crate-level gating) Fixes: rust-lang#84836 Removes the function-level feature gating solution originally implemented, and solves the same problem using `allow_internal_unstable`, so normal crate-level feature gating mechanism can still be used (which disallows the feature on stable and beta). I tested this, building the compiler with and without `CFG_DISABLE_UNSTABLE_FEATURES=1` With unstable features disabled, I get the expected result as shown here: ```shell $ ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc src/test/run-make-fulldeps/coverage/no_cov_crate.rs error[E0554]: `#![feature]` may not be used on the dev release channel --> src/test/run-make-fulldeps/coverage/no_cov_crate.rs:2:1 | 2 | #![feature(no_coverage)] | ^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error For more information about this error, try `rustc --explain E0554`. ``` r? ```@Mark-Simulacrum``` cc: ```@tmandry``` ```@wesleywiser```

Rollup of 11 pull requests Successful merges: - rust-lang#84409 (Ensure TLS destructors run before thread joins in SGX) - rust-lang#84500 (Add --run flag to compiletest) - rust-lang#84728 (Add test for suggestion to borrow unsized function parameters) - rust-lang#84734 (Add `needs-unwind` and beginning of support for testing `panic=abort` std to compiletest) - rust-lang#84755 (Allow using `core::` in intra-doc links within core itself) - rust-lang#84871 (Disallows `#![feature(no_coverage)]` on stable and beta (using standard crate-level gating)) - rust-lang#84872 (Wire up tidy dependency checks for cg_clif) - rust-lang#84896 (Handle incorrect placement of parentheses in trait bounds more gracefully) - rust-lang#84905 (CTFE engine: rename copy → copy_intrinsic, move to intrinsics.rs) - rust-lang#84953 (Remove unneeded call to with_default_session_globals in rustdoc highlight) - rust-lang#84987 (small nits) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

emilyalbini · 2021-05-07T09:03:37Z

My understanding is that this should be backported to 1.53 beta, correct?

wesleywiser · 2021-05-07T12:59:21Z

Yes, nominating for beta-backport.

pnkfelix · 2021-05-13T14:20:16Z

discussed in T-compiler meeting, beta backport approved.

…ulacrum [beta] backports * Backport 1.52.1 release notes rust-lang#85404 * remove InPlaceIterable marker from Peekable due to unsoundness rust-lang#85340 * rustdoc: Call initSidebarItems in root module of crate rust-lang#85304 * Update LLVM submodule rust-lang#85236 * Do not ICE on invalid const param rust-lang#84913 * Disallows #![feature(no_coverage)] on stable and beta (using standard crate-level gating) rust-lang#84871 * Ensure TLS destructors run before thread joins in SGX rust-lang#84409

rust-highfive assigned Mark-Simulacrum May 3, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 3, 2021

richkadel force-pushed the no-coverage-unstable-only branch from 0c51f39 to f5b0874Compare May 3, 2021 15:57

richkadel mentioned this pull request May 3, 2021
no_coverage feature-gated on function, not crate? #84836
Closed

This comment has been minimized.
Sign in to view

richkadel force-pushed the no-coverage-unstable-only branch from f5b0874 to 90204feCompare May 3, 2021 16:28

This comment has been minimized.
Sign in to view

richkadel force-pushed the no-coverage-unstable-only branch from 90204fe to a40d701Compare May 3, 2021 16:39

This comment has been minimized.
Sign in to view

tmandry reviewed May 5, 2021
View reviewed changes

Comment threadcompiler/rustc_typeck/src/collect.rs Outdated

nagisa reviewed May 5, 2021
View reviewed changes

richkadel changed the title ~~Disallows #[feature(no_coverage)] on stable and beta~~Disallows #![feature(no_coverage)] on stable and betaMay 5, 2021

richkadel force-pushed the no-coverage-unstable-only branch from 22b355a to 3584c1dCompare May 5, 2021 14:54

richkadel mentioned this pull request May 5, 2021
Update coverage docs and command line help #84815
Merged

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 5, 2021

richkadel changed the title ~~Disallows #![feature(no_coverage)] on stable and beta~~Disallows #![feature(no_coverage)] on stable and beta (using standard crate-level gating)May 5, 2021

Dylan-DPC-zz mentioned this pull request May 6, 2021
Rollup of 16 pull requests #84975
Closed

This was referenced May 6, 2021
Rollup of 11 pull requests #85005
Closed
Rollup of 11 pull requests #85006
Closed

Dylan-DPC-zz mentioned this pull request May 6, 2021
Rollup of 11 pull requests #85014
Merged

bors merged commit aaf2389 into rust-lang:masterMay 7, 2021

rustbot added this to the 1.54.0 milestone May 7, 2021

wesleywiser added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 7, 2021

pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label May 13, 2021

jyn514 mentioned this pull request May 22, 2021
Bump bootstrap compiler to beta 1.53.0 #84985
Merged

Mark-Simulacrum modified the milestones: 1.54.0, 1.53.0 May 22, 2021

Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 22, 2021

Mark-Simulacrum mentioned this pull request May 22, 2021
[beta] backports #85588
Merged

Uh oh!

Uh oh!

Conversation

richkadel commented May 3, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ghost commented May 3, 2021

Uh oh!

richkadel commented May 3, 2021

Uh oh!

tmandry commented May 5, 2021

Uh oh!

Uh oh!

richkadel commented May 5, 2021

Uh oh!

nagisaMay 5, 2021

Choose a reason for hiding this comment

Uh oh!

richkadelMay 5, 2021

Choose a reason for hiding this comment

Uh oh!

richkadelMay 5, 2021

Choose a reason for hiding this comment

Uh oh!

nagisa commented May 5, 2021

Uh oh!

richkadel commented May 5, 2021

Uh oh!

richkadel commented May 5, 2021

Uh oh!

nagisa commented May 5, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented May 5, 2021

Uh oh!

richkadel commented May 5, 2021

Uh oh!

emilyalbini commented May 7, 2021

Uh oh!

wesleywiser commented May 7, 2021

Uh oh!

pnkfelix commented May 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

richkadel commented May 3, 2021•
edited
Loading

nagisa commented May 5, 2021•
edited
Loading