no_coverage feature-gated on function, not crate? #84836
Comments
Mark-Simulacrum
added
T-compiler
|
cc #84605 |
jonas-schievink
added
A-code-coverage
Correct. The driving reason for adding the feature was to resolve #83601, and that issue occurred because Feature gating at the function level resolved the problem, even though it was non-standard. I do allow feature-gating at the crate level as well, but it's not used by
I didn't realize this also needs to be checked. Is there an example of how to do this, so I can also implement it in the function-based feature-gate check? |
|
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. If necessary, I can either provide or track down someone who can, further hints. It might be good to look at how the structural equality worked before the move to the trait impls as it had a similar built-in attribute. In the meantime we'll likely revert the PR on just branched beta (1.53) so we don't accidentally stabilize the feature. |
|
@Mark-Simulacrum: I just uploaded a PR to fix this, as an alternative to reverting: #84871 Your call. Thanks! |
|
In procedural macros and derive macros stability propagation can be carefully handled (AFAIK) by setting the span (or more precisely its expansion id) up in such a way that it refers to the crate that has feature gates enabled. I'm confident there's somebody on T-compiler who can say something more precise. I don't think we should be introducing new ways to enable unstable features (by e.g. allowing them to be specified as attributes on items) without a broader buy-in. The ability to have non-crate-level feature gates definitely sounds like at least a MCP material to me, primarily due to complications in the implementation of the compiler & tooling surrounding the unstable feature tracking and relative ease with which an oversight can happen and a feature accidentally stabilized. |
|
I should have caught this, sorry. #84871 can work as a stopgap, but if we need an MCP to use this mechanism at all we should probably revert. |
Thanks @nagisa ! This was the hint I needed. I uploaded a revised PR commit with a change in line with your suggestion. All tests work as before (with the exception of test that enabled the feature at the function level; these are now removed). |
Just confirming, but I think @nagisa was only referring to function-level feature gating (now removed) as needing an MCP. But since this is gone, I think the feature-gated Correct me if I'm wrong. |
…, 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`
|
Assigning P-critical for the record, as discussed in the Zulip thread of the Prioritization Working Group. @rustbot label -I-prioritize +P-critical |
rustbot
added
P-critical
…, 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````
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`. ```
It looks like the no_coverage attribute was added by checking for feature(no_coverage) on the function, but this isn't how feature gating usually works. Additionally, I don't believe that restricts usage of this on stable/beta, since nothing is actually checking this feature gate.
Noticed based on the test case (https://github.com/rust-lang/rust/blob/master/src/test/ui/feature-gates/feature-gate-no_coverage.rs) and while looking at the feature-gating code.
cc @tmandry @richkadel
The text was updated successfully, but these errors were encountered: