Require Cargo feature opt-in for min_specialize

[alexcrichton]

This commit switches away from implicitly enabling the min_specialize Rust nightly feature whenever a Nightly compiler is used to instead requiring an explicit opt-in with a new Cargo feature nightly-specialize. The goal of this commit is to fix #238 and has two primary motivations:

1. In Require an opt-in for enabling min_specialization rustc feature #238 I'm trying to build something that depends on this crate as part of the Rust bootstrap process but this crate fails to build due to min_specialize not being allowed but a nightly compiler is in use. This is due to the fact that the way -Zallow-features is managed in the bootstrap is different than the standard Cargo way of doing so.

2. This removes a failure mode where if one day the min_specialize feature changes this crate won't break when built on nightly. Users of Nightly compilers will be able to continue using this crate if the feature was not explicitly opted-in to.

[riking]

This pattern also causes issues with Crater and other workflows that use a current nightly to build old code. It should generally be avoided.

[tkaitchuck]

Enabling this feature on stable will cause a compilation error. Given that any package in your transitive dependencies can enable it and there isn't way to override and disable a feature, this seems likely to cause more problems.

[alexcrichton]

That's correct yeah, but I only chose this route due to the preexistence of the nightly-arm-aes Cargo feature which has the same weakness. Despite this weakness my impression is that the feature name is enough to convey to users that it's there for testing but code published to crates.io which relies on the feature will itself be considered buggy.

Personally I think that the downside of a Cargo feature is less than the downside of "can be broken by any new nightly coming out" for users not opting-in to nightly features.

[kornelski]

ahash depends on the version_check crate, but that crate isn't actually checking whether the feature is supported, and answers true in almost every case.

SergioBenitez/version_check#24

[SergioBenitez]

ahash depends on the version_check crate, but that crate isn't actually checking whether the feature is supported, and answers true in almost every case.

That's a gross oversimplification of what version_check does. The docs say:

/// Returns true iff [is_feature_flaggable()] returns true and the
/// feature is not disabled via exclusion in allow-features via RUSTFLAGS or
/// CARGO_ENCODED_RUSTFLAGS. If the version could not be determined, returns
/// None.

And is_feature_flaggable() says:

Returns true if the channel is either "nightly" or "dev".

So saying it "answers true in almost every case" is misrepresenting its functionality wholly. It answers true exactly when the release channel is known to be nightly or dev and the feature has not been disabled via RUSTFLAGS or CARGO_ENCODED_RUSTFLAGS. That's significantly less than "almost every case."

For this particular feature, given that 1) min_specialiaztion was introduced in a commit in 2020 (roughly rustc 1.43) and 2) the MSRV of the crate is 1.60 and checked via rust-version, this check is fairly "correct" and will only be incorrect in exactly the cases that @alexcrichton points out in this PR, i.e, you use an undocumented and unknown mechanism to disable features or some new Rust release breaks your expectations of the feature (or you lie about your compiler being nightly or dev).

I don't mean to minimize the problem but rather clarify it.

Dealing with the first concern, using some unknown mechanism to disable features where they would otherwise be enabled, feels like a tricky situation to tackle in general. Either we teach version_check these unknown tricks, have that custom compiler also use the known mechanisms, or probe the compiler more directly about its functionality. rustc has no mechanism to probe it directly about the features it supports, and much less about how those features work. The only approach I'm aware of is to ask it to compile a small, hopefully representative example, and proceed if it works.

The second concern, that some new Rust release will break your expectations of the feature, feels impossible to solve well in general without support from rustc itself. But a few ideas are:

1. Limit enabling the features to rustc versions you're sure work the way you think.
2. Probe the compiler by asking it to compile a small hopefully representative example.
3. Try compiling your whole crate, and if it fails, fall-back to one where you don't use the feature.

An intermediary solution might be to do what this PR is doing, IE add an opt-in feature, as well as what this crate was doing, i.e, a best effort check to see if the feature can be enabled at all. That avoids the concern in #239 (comment) without requiring probes. The downside, of course, is that you don't get the functionality without opting-in even if your toolchain is perfectly capable of handing it.

---

In my opinion, this problem can only be properly solved at the rustc side. Everything is just trying to guess if rustc can or can't do something. It would be great if we could just ask it directly. If rustc allowed probing it for feature support, and if features were versioned semantically, this problem would go away. You could just ask whether it implements (for example) min_specialization = 0.4. Being able to do this enables even nicer interfaces, such as a cfg that removes all of the usability concerns:

#[cfg(unstable(min_specialization = "0.4"))]
fn use_min_specialization() { .. }

If that cfg were built-in, you'd remove the need to invoke the compiler entirely!

[kornelski]

Users don't typically disable any feature flags themselves. This manual exception is too rare to matter.

Sorry I wasn't clear that I'm talking about nightly compilers only. It responds true by default for all nightly compilers, and that is a problem. Checking for the nightly channel is not sufficient to allow nightly features.

version_check's supports_feature("does_not_exist") returns true for all nightly compilers. I'm not even talking about behavior changes in the feature. There's no check at all whether the given feature even exists in the compiler.

[SergioBenitez]

#242 is the way the go.

[alexcrichton]

Clearing out my open PRs, this looks like it's unlikely to land, so I'm going to close.