Skip to content

add an internal lint for nightly-channel-only features without a feature gate #139892

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.

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

Open
jyn514 opened this issue Apr 16, 2025 · 6 comments
Open
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-stability Area: `#[stable]`, `#[unstable]` etc. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Apr 16, 2025

we really should not have features that are available only on nightly but do not have a feature gate. this leads to people unknowingly using unstable features, which in turn leads to situations where we have no choice but to stabilize features unchanged because otherwise we break too much of the ecosystem. it also defeats the whole point of -Z allow-features.

we should do two things:

  • audit all existing features to make sure they have a feature gate. @jieyouxu tells me that at least #[cfg(target_has_atomic)] does not.
  • add an internal lint against UnstableFeatures::is_nightly_build. basically the only reason it's valid to check this is for diagnostics, or very early in session building, and both of those can add an explicit allow().

@rustbot label A-stability

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 16, 2025
@jieyouxu jieyouxu added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. 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 Apr 16, 2025
@rustbot rustbot added the A-stability Area: `#[stable]`, `#[unstable]` etc. label Apr 16, 2025
@Zalathar
Copy link
Contributor

Zalathar commented Apr 16, 2025

There are a few different ways for arbitrary compiler code to check whether the compiler is nightly:

  • Session::is_nightly_build (on tcx.sess)
  • UnstableFeatures::is_nightly_build (obtained via UnstableFeatures::from_environment)
  • Functions in nightly_options

So it would be useful to audit all of these.

I could also imagine splitting some of these APIs into diagnostic-only and non-diagnostic variants, so that it's easier to audit the legitimate exceptions.

@RalfJung
Copy link
Member

RalfJung commented Apr 16, 2025

FWIW, cfg(target_feature) also changes behavior on nighty by adding information about nightly target features. I think the reason is the lack of a good discoverable alternative: we don't really have a way to gate cfg(target_feature = "nightly_feature") specifically; that expression compiles fine on stable, it just always evaluates to false. We could say you have to set #[feature(nightly_target_features)], or set the feature gate guarding that specific nightly target feature, but then cfg(target_feature = "nightly_feature") would just silently become "false" and users wouldn't know they have to set a feature gate.

@bjorn3
Copy link
Member

bjorn3 commented Apr 16, 2025

All unstable cfg's are implicitly available on the nightly channel. Note that cargo needs to know which cfg's are set before compiling a single line of rust code (so #![feature] wouldn't work for it) and rustc doesn't know which features are enabled until the cfg's for the crate root are evaluated.

@jyn514
Copy link
Member Author

jyn514 commented Apr 16, 2025

Note that cargo needs to know which cfg's are set before compiling a single line of rust code

what does cargo need this information for? my understanding is that this is an internal feature that is only used by libstd - we shouldn't have [cfg.'target_has_atomic'.dependencies] or anything like that.

rustc doesn't know which features are enabled until the cfg's for the crate root are evaluated.

we should ban unstable cfgs at the crate root IMO.

we don't really have a way to gate cfg(target_feature = "nightly_feature") specifically; that expression compiles fine on stable, it just always evaluates to false. We could say you have to set #[feature(nightly_target_features)], or set the feature gate guarding that specific nightly target feature, but then cfg(target_feature = "nightly_feature") would just silently become "false" and users wouldn't know they have to set a feature gate.

that makes sense to me and is a little trickier - perhaps it could continue to be "false", but not silently? where we give a future-incompat lint that the behavior may change, and you can use feature() to opt in today?

@bjorn3
Copy link
Member

bjorn3 commented Apr 16, 2025

my understanding is that this is an internal feature that is only used by libstd

target_has_atomic_load_store and target_has_atomic_equal_alignment are not the only unstable cfgs (target_has_atomic is stable by the way). The example Ralf gave of cfg(target_feature = "nightly_feature") is another one which will become stable in the future. And target_has_atomic_load_store seems like it would be useful to stabilize in the future. And some code (especially those that use inline asm) needs cfg(relocation_model).

@jyn514
Copy link
Member Author

jyn514 commented Apr 16, 2025

got it, that makes sense. i still think "continues to evaluate to false until you enable a feature gate, warns if the gate is disabled" is a reasonable solution and better than enabling unconditionally on nightly.

target_has_atomic

target_has_atomic with a value (e.g. 16) is stable, but target_has_atomic without a value is not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-stability Area: `#[stable]`, `#[unstable]` etc. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants