search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.23k stars 12.7k forks source link

Decide whether we want MSRV-gating #129445

Open traviscross opened 2 months ago

traviscross commented 2 months ago

There's an open proposal to add an --msrv flag to the compiler:

The idea is that this could be used, in a similar way to the current --edition flag, to gate lints (or features?) based on the MSRV.

As a process matter, since lang approves new lints and how they are gated, it seems that it might be helpful to T-compiler to know whether and how lang might intend to use this gating. So let's discuss that.

@rustbot labels +T-lang +C-discussion +I-lang-nominated

cc @rust-lang/lang @compiler-errors

scottmcm commented 2 months ago

It's interesting that this is a place where that version we're recording on stabilized features might actually be useful for something. AFAIK it's completely unused today.

I think it should never be the normal-and-expected things that lints have to look at the MSRV, nor that lints only apply if your MSRV is at least as high as the version in which the lint was added.

But a clippy lint for "use this new standard library API" or a rustc lint for "use this new thing" both seem like places were it would be reasonable for a lint to make some minor effort to not emit the lint, if the thing it's suggesting is reasonably-new. (If it's suggesting something that's -- strawman -- over a year old then I don't think we should ask people to bother.)

And it should never be expected to be perfect. If there are edge cases where things aren't gated, I think that's entirely fine, just like how the version number in the standard library docs doesn't update every time a tweak is made to a method.

The full answer must always be that the only 100% reliable way is to only deny lints on your MSRV CI build, and permit them on any builds you do on newer toolchains. This should be a "reasonable-effort to let us lint on things sooner" mechanism (rather than waiting a few releases like we often do today), rather than a strict "anything that leaks through is necessarily a bug" hard requirement.

VorpalBlade commented 2 months ago

Could this be used to prevent future breakage like the recent issue with time? With this Rustc could hide impls that weren't available on the given MSRV.

Furthermore it could be used to completely hide other items from newer than MSRV as well, which would reduce the need for testing on older versions. (A suitable lint that thr item is available on a newer MSRV would be needed in this case.)

joshtriplett commented 2 months ago

I don't think we should use this to gate features; let's not turn random Rust versions into pseudo-editions. But I very much want to see this used both to reduce the impact of lints and to tailor lint suggestions.

If we're emitting a lint already, I don't think we should omit lint suggestions based on MSRV, but I think we should give an indication if applying the suggestion will raise MSRV. Ideally, that indication would be machine-readable, and cargo fix could then decide either "don't apply suggestions that would raise MSRV" or "apply suggestions that would raise MSRV and update the MSRV accordingly".

tmandry commented 2 months ago

There are a number of related problems that can be tackled here, and I think we should go for a solution that can address multiple of them.

Disabling lints that contradict earlier compiler behavior

My understanding of the thread is that this is the original problem statement: "lint is introduced that makes you write code not accepted in an earlier version of Rust". While I think that's a problem worth solving, it feels too narrow to tackle by itself with a compiler flag.

If we're emitting a lint already, I don't think we should omit lint suggestions based on MSRV, but I think we should give an indication if applying the suggestion will raise MSRV.

I think I agree, but "if we're emitting a lint already" is doing a lot of work there. I would never want to emit the lint removed by #129103 in crates targeting older MSRVs, because it's more of a style than correctness issue, and bumping MSRV is therefore not a reasonable action to recommend.

Linting on language and standard library features stabilized later

I think it would be really nice to lint (at least as a best effort) against use of features that are later than the MSRV. This is to aid crate authors, not create actual language-level editions in every new version of Rust. As above, the warning cases in the compiler can be cleaned up after some number of Rust versions.

Controlling item visibility with #[stable]

I think really have to solve the specific problem @VorpalBlade mentions, of breaking crates when we introduce new items in std, in one way or another.

Controlling visibility of crate items with a version requirement is, in my view, much less scary than a general edition mechanism. In the case of core/alloc/std, the version number happens to be >=MSRV, but it's important to note that this is for std the crate and not the compiler: If we ever supported decoupling the version of std from the version of rustc, we would look at the version requirement for std and not the MSRV.

In the case of other crates, we would check whatever version requirement is listed in your Cargo.toml against any #[stable] attributes in that crate (assuming of course that we stabilize the attribute). This is useful not only for limiting breakage when a toolchain or crate graph is globally updated, but also for checking the version requirements listed in a crate's manifest.

Easing compiler transitions in large codebases

122759 is an attempt at easing compiler updates in large repos by implementing significant expansions as new lints with lint groups, or by requiring an auto-fix; both aid in rolling out a compiler without disabling major lints. However, as was pointed out in https://github.com/rust-lang/rust/pull/129103#issuecomment-2303557991, endlessly adding new lints to cover expansions is not really a great strategy. (#122759 mitigates this by only imposing such a requirement for "significant expansions" affecting a certain number of crates, but it's still a problem.)

Another way is to have a flag that says "lint as if you were this version" and decouple that from updating the actual compiler. This flag nearly does that, but only in the very narrow sense I mentioned above. --lint-version seems like a decent name.

We can and should only do this as a best-effort and for significant expansions as mentioned before. As an added benefit, we can remove the special casing after some number of Rust releases, without the baggage of a separate lint name.

Flags

There are like three RFCs in this comment, but anyway. These flags map to the following:

Conclusion

I think we should have both flags, but it would not be worth it to stabilize --msrv and only ever use it to solve the original problem in this thread. --msrv does nothing to solve the more general problem of lint churn, especially in large codebases where there is no MSRV, just ~one Rust version.

scottmcm commented 2 months ago

Could this be used to prevent future breakage like the recent issue with time? With this Rustc could hide impls that weren't available on the given MSRV.

I don't think so, for the same reason that editions can't change trait impls. Trait solving isn't localized to particular tokens, rather it encompasses things from your crate and any you call, which might be on different editions.

(Things like the array IntoIter changed method resolution between editions, but the actual impl was added to all editions.)