warn against automatically using nightly features

[RalfJung]

See https://github.com/tkaitchuck/aHash/issues/200 for an issue caused by using version_check::supports_feature without considering the consequences.

IMO supports_feature should be removed entirely, but maybe there are ways to use it that are not as fragile?

[Noratrieb]

is_feature_flaggable is harmful as well. Though it's probably more useful to not delete it but keep it and deprecate it saying "DONT DO THIS, use a feature flag". That will be more effective at deterring people.

[tkaitchuck]

@Nilstrieb A feature flag does not solve the same problem and hence is not a viable replacement.

[RalfJung]

Using supports_feature is also not a viable replacement for anything. Rustc devs are actively discouraging this pattern since it can and will cause wide-spread build breakage. This breakage will incorrectly be blamed on rustc (when it's really crates using nightly detection that are to blame). We'd strongly prefer if we didn't have to try and use technical means to prevent such issues.

[tkaitchuck]

Perhaps a feature could have a version. If that could be accessed through cfg conditionals, there would not need to be any features or risk of incompatibility .

[RalfJung]

@SergioBenitez any opinion on this? As-is this create implicitly recommends code to be written in a way that leads to nightly rustc development being harder than it should be, making it generally an anti-pattern (see https://github.com/rust-lang/rust/issues/120804).

[SergioBenitez]

I agree that we should include more verbiage with respect to how this crate can be used in a less error-prone fashion.

On the broader subject, I believe there are two threads to consider:

1. The responsibility of a crate author using nightly features (the dependency).
2. The responsibility of a user depending on a crate that may or may not use nightly features (the dependent).

The argument pushed forward by this PR and by the comment in https://github.com/tkaitchuck/aHash/issues/200#issuecomment-1928961077 is that the dependent should be responsible for opting-in to potential nightly breakage via a feature flag or similar. While I believe this is a fair recommendation, I also believe that this "opting-in" is already happening as a result of a nightly compiler being used, and that a second opting-in should be viewed as redundant.

By their very nature, nightly compilers are expected to break. It shouldn't be seen as catastrophic when something does break. We shouldn't normalize using a nightly compiler or try to make it seem as stable as stable. I fear that this PR and the aforementioned recommendation perpetuates that sentiment, however.

Finally, I believe the dependency should have the responsibility of staying on top of nightly releases, should they wish to use nightly features, and by doing so, decrease significantly or entirely eliminate any kind of breakage. This is the approach we took with Rocket early on and one we continue to take. It's a large responsibility, but it's attenuated by 1) the ability for users to roll-back to a "good" nightly release, and 2) the fact that not everyone updates their nightly toolchain immediately. In some ideal word, rustc and/or cargo would alleviate this burden, but alas that is not the current world.

To summarize, I agree that we should add a word of warning to this crate's documentation, but my feeling is that that wording should be a bit more conservative than the proposed.

[RalfJung]

As someone who uses nightly compilers a lot, I generally expect that cargo +stable build and cargo +nightly build will build the same code, just with slightly different compilers. That is an extremely viable property, e.g. when determining whether there was a regression in the compiler.

So I must disagree here, using nightly is not an opt-in inviting rustc to just use potentially breaking features in whatever way it pleases. If that were true, we wouldn't bother with all these feature attributes, we'd just enable them all automatically on nightly!

I also think historically this is pretty clear: the opt-in to a potential breakage is the #[feature] attribute. Furthermore, that opt-in is only available on nightly. But using nightly itself doesn't opt-in to anything, it merely provides the ability to opt-in. It is only when crates started to become too clever in what they do in their build scripts, that this stopped being a universal truth.

Interpreting the use of nightly as opt-in is a risk for nightly rustc development since we can no longer easily test how a crate will behave when this compiler turns stable, and we can no longer change nightly features without risking making some crates that work fine on stable entirely impossible to build on nightly, thus severely impacting the usability of nightly compilers. If a crate wouldn't build on stable at all, then obviously none of this matters, but we're talking about crates that are ostensibly relying only on stable Rust, but then when I switch to nightly e.g. to use some -Z flag, suddenly they enable more functionality, thus changing their behavior (always fun during debugging) or they even fail to build -- that's bad. That's exactly why rustc developers are so concerned, and why https://github.com/rust-lang/rust/issues/120804 exists.

[SergioBenitez]

As someone who uses nightly compilers a lot, I generally expect that cargo +stable build and cargo +nightly build will build the same code, just with slightly different compilers. That is an extremely viable property, e.g. when determining whether there was a regression in the compiler.

I also use nightly almost exclusively and yet I don't have the same expectation. I generally expect that the compilers will build the same code, but I expect nearly 100% reliability from a stable compiler whereas I don't have the same expectation from a nightly compiler. Unfortunately it doesn't seem that any documentation anywhere defines what the user's expectations of each channel should be. I base my expectations on my own experience: ICEs and random compilation bugs happen with nightly with significant more frequency than stable. As a result, I don't trust it nearly as much.

So I must disagree here, using nightly is not an opt-in inviting rustc to just use potentially breaking features in whatever way it pleases. If that were true, we wouldn't bother with all these feature attributes, we'd just enable them all automatically on nightly!

I also think historically this is pretty clear: the opt-in to a potential breakage is the #[feature] attribute. Furthermore, that opt-in is only available on nightly. But using nightly itself doesn't opt-in to anything, it merely provides the ability to opt-in. It is only when crates started to become too clever in what they do in their build scripts, that this stopped working.

I think this is a good argument, and I think it would be an even stronger argument if feature was infectious in some sense, any sense. That would also absolve crate authors of the burden of abusing Cargo features for this purpose.

It should also be mentioned that there are numerous differences in nightly that, for one reason or another, aren't or can't be behind feature flags. These represent core differences between the channels that have no further means to opting in. Ultimately, this leads to work-arounds of this nature which would otherwise have no utility.

Interpreting the use of nightly as opt-in is a risk for nightly rustc development since we can no longer easily test how a crate will behave when this compiler turns stable, and we can no longer change nightly features without risking making some crates that work fine on stable entirely impossible to build on nightly, thus severely impacting the usability of nightly compilers.

The beta channel seems to have been forgotten here as it was at least initially suggested for exactly this use-case.

If a crate wouldn't build on stable at all, then obviously none of this matters, but we're talking about crates that are ostensibly relying only on stable Rust, but then when I switch to nightly e.g. to use some -Z flag, suddenly they enable more functionality, thus changing their behavior (always fun during debugging) or they even fail to build -- that's bad. That's exactly why rustc developers are so concerned, and why rust-lang/rust#120804 exists.

I understand, but as we've covered, I feel that you're absolving the user from any responsibility involved with the switch to nightly. Their code could well stop building as a result of a change in the nightly compiler that has nothing to do with unstable features. In fact, there are dozens of issues reporting such breakage in the last couple of weeks. What is the analogous argument but to suggest that one shouldn't use the nightly channel, or minimally, the affected nightly version?

I should clarify that I am not against adding a word of caution to this crate's documentation, and I am in the process of doing so, only that it shouldn't bias so much in any one direction.

[SergioBenitez]

Pushed a variant of this in https://github.com/SergioBenitez/version_check/commit/f4284aff45d7e128aa5d9f3483ef5ec1cd575a75.

[RalfJung]

I also use nightly almost exclusively and yet I don't have the same expectation. I generally expect that the compilers will build the same code, but I expect nearly 100% reliability from a stable compiler whereas I don't have the same expectation from a nightly compiler. Unfortunately it doesn't seem that any documentation anywhere defines what the user's expectations of each channel should be. I base my expectations on my own experience: ICEs and random compilation bugs happen with nightly with significant more frequency than stable. As a result, I don't trust it nearly as much.

Yes, nightly has more compiler bugs. It exists as a testbed for the compiler. But it was never intended to be a testbed for ostensibly stable crates to silently enable more functionality. This is something crate authors started doing, and some individuals on the compiler team are not happy about that since it interferes with the original role of nightly.

Incidents like the one with ahash make it noticeably harder to find and weed out these bugs in nightly before they hit beta and then stable. That's why we should push back hard against crates doing things like that. Some crates use a more sophisticated approach where they actually build a bit of demo code that uses the feature, and then only enable the feature if the demo code builds -- that has the big advantage that if the feature changes in ways that break the demo code, the crate keeps building. It is still problematic IMO, but it is much better than a crate just blindly enabling a nightly feature without any check whether that feature works the way it should. The latter is something that IMO should never be done.

It should also be mentioned that there are numerous differences in nightly that, for one reason or another, aren't or can't be behind feature flags. These represent core differences between the channels that have no further means to opting in. Ultimately, this leads to work-arounds of https://github.com/rust-lang/rust/issues/120804#issuecomment-1936604648 which would otherwise have no utility.

I think you misunderstood the work-around proposed there -- that proposal is intended to deal with code that uses functions like version_check::supports_features, by forcing it to always return "no, not supported". It is the hammer we wouldn't need if boot scripts wouldn't try to be more clever than is good for the ecosystem. I'd rather not enter a game of whack-a-mole with crate authors where the compiler gets increasingly clever at tricking crates into not realizing that they are running on nightly, and then crate authors find creative ways to detect the presence of nightly anyway.

The beta channel seems to have been forgotten here as it was at least initially suggested for exactly this use-case.

The beta channel is great, but when working on compiler features, if it takes 6 weeks until I can even test whether my code breaks anything that works on stable, that's really bad.

I understand, but as we've covered, I feel that you're absolving the user from any responsibility involved with the switch to nightly. Their code could well stop building as a result of a change in the nightly compiler that has nothing to do with unstable features. In fact, there are dozens of issues reporting such breakage in the last couple of weeks. What is the analogous argument but to suggest that one shouldn't use the nightly channel, or minimally, the affected nightly version?

Crates that use version_check::supports_features make it harder for compiler devs to find and fix these issues. This isn't about the responsibility of users, it is about compiler dev's ability to develop the compiler.

I don't understand what kind of analogy you are trying to make here. It should be the case that if a crate builds on stable and doesn't build on nightly, that's a compiler bug (or an expected breakage of a stable feature, which is rare but happens). Bugs happen, obviously. They are not the user's responsibility. But crates that use version_check::supports_features make it so that if a crate builds on stable and doesn't build on nightly, that's now sometimes not a compiler bug, throwing a wrench into Rust's nightly feature development story.

Just look at the amount of backlinks in https://github.com/tkaitchuck/aHash/issues/200. If that amount of fallout can arise from a nightly-only change not observable on stable, then something is wrong. This is not healthy. And the root of the problem is crates automatically enabling nightly features without even testing whether they work. That should be considered an anti-pattern and not acceptable to occur in any crate. Whether build probes are acceptable is still somewhat open; they are fragile in their own right.

Thank you for adding some cautionary wording. I worry it might not be strong enough to dissuade people from using these functions. I can't think of any situation where I would consider the use of these functions legitimate (maybe a "this was built with nightly rustc" note in an auto-generated bugreport or so, that seems reasonable). My PR was the compromise version; what IMO really should happen is that these functions should be removed. Given that the crate provides no build probe functionality, it also seems unlikely people will switch to that instead. Maybe it'd be worth linking to https://github.com/cuviper/autocfg which provides a way to fairly easily write such build probes?