`--msrv=version` option so the compiler can take MSRV into account when linting

[joshtriplett]

trafficstars

Proposal

Add a --msrv=version command-line option to rustc, which cargo would then pass in based on the rust-version option in Cargo.toml.

The rationale for this is the same reason clippy wants to know rust-version: it allows smarter linting and suggestions. For instance, we could avoid making fix suggestions that would raise MSRV, or alternatively we could emit some metadata saying we'd increase MSRV so that cargo fix could automatically increase MSRV.

As an additional rationale, when we have a cfg for the compiler version, having the MSRV would let us determine if the cfg is always true or always false: https://github.com/rust-lang/rust/issues/64796#issuecomment-2289358630

I'm proposing that the initial implementation of this just validate that its argument is a valid version number (accepting exactly what rust-version accepts), and otherwise ignore it; this would be enough for cargo to start passing it to compilers that support it. Any additional functionality would come later.

(Until this is stable, it'll require -Zunstable-options.)

Mentors or Reviewers

I don't have a specific mentor/reviewer in mind here.

Process

The main points of the Major Change Process are as follows:

• [x] File an issue describing the proposal.
• [ ] A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
  • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
  • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
• [ ] Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

[rustbot]

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

Concerns or objections to the proposal should be discussed on Zulip and formally registered here by adding a comment with the following syntax:

@rustbot concern reason-for-concern 
<description of the concern> 

Concerns can be lifted with:

@rustbot resolve reason-for-concern 

See documentation at https://forge.rust-lang.org

cc @rust-lang/compiler @rust-lang/compiler-contributors

[ojeda]

Another use case: the compiler could also use this information to detect usage of features that definitely cannot work with the specified MSRV version (even if it is best-effort). For instance, using let ... else before it was supported.

It may be easy for a CI to test for that, especially in projects without too much conditional compilation, but it does not hurt if developers know as soon as possible while writing the code.

@rustbot label A-rust-for-linux

[rustbot]

Error: Label A-rust-for-linux can only be set by Rust team members

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[ojeda]

Ah, sorry, it is not the main repository.

Also, originally discussed in Clippy, but related: https://github.com/rust-lang/rust-clippy/issues/11227, i.e. using the given MSRV to potentially ignore new lints too, which may enable denying lint groups in some projects and could potentially make it slightly easier for enable lints or raise their severity if most crates out there declare an MSRV. Though this use case is not as clear cut as others.

[Nadrieril]

To quote rustbot above: this issue is not meant to be used for technical discussion, please use the Zulip stream for that :pray:

[traviscross]

cc @rust-lang/lang

[estebank]

If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.

@rfcbot fcp merge

[rfcbot]

Team member @estebank has proposed to merge this. The next step is review by the rest of the tagged team members:

• [ ] @Aaron1011
• [ ] @cjgillot
• [ ] @compiler-errors
• [ ] @davidtwco
• [x] @eddyb
• [x] @estebank
• [ ] @jackh726
• [ ] @lcnr
• [ ] @matthewjasper
• [ ] @michaelwoerister
• [ ] @nagisa
• [ ] @oli-obk
• [ ] @petrochenkov
• [ ] @pnkfelix
• [ ] @wesleywiser

Concerns:

• ~~design-needs-fleshing-out~~ resolved by https://github.com/rust-lang/compiler-team/issues/772#issuecomment--1979209605

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[compiler-errors]

I feel like this should be FCP'd when it's ready for stabilization. This simply needs a second, I believe? Or at least that's what other unstable flags have done in the recent past.

That being said, I'm somewhat hesitant to add a flag that does nothing right now. It feel like this design should be fleshed out more before, ideally as an RFC, but at least via some sort of design doc that people can look towards to guide implementation moving forward. I'll say a bit more in zulip, but this is somewhat process related so it's worth putting here.

[compiler-errors]

@rfcbot concern design-needs-fleshing-out

[traviscross]

Speaking of process, 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. I've opened and nominated an issue for us to discuss that:

• https://github.com/rust-lang/rust/issues/129445

[compiler-errors]

We decided in the lang team meeting that this should:

1. The flag should be called --hint-msrv= or something similar that makes it clear that this is informational.
2. Not be load-bearing -- this should not affect inference, trait solving, etc. without a full proposal, and I believe that full proposal should be RFC'd. This effectively limits the behavior today to best-effort linting, but we could come up with.
3. The documentation should note that this is best-effort only, and that the only complete way to ensure MSRV is by downloading the right compiler version and testing it there. This will happen whenever we actually add the flag.

For now, let's not discuss more radical extensions of this feature like affecting inference, trait solving, or having any sort of guarantee for correctness.

@rfcbot resolve design-needs-fleshing-out

[compiler-errors]

That being said, this doesn't need to be fcp'd until stabilization. It should be gated behind -Zunstable-options.

@rfcbot cancel

[rfcbot]

@compiler-errors proposal cancelled.

[compiler-errors]

That being said ×2, I'm happy with the flag in this form. Though other compiler (+contributor) team members can and should speak up and raise a concern if they disagree.

@rustbot second

[joshtriplett]

@tmandry made the point of wanting to have a separate flag for if we decide to use this as an alternative for things like the split of unreachable_patterns/impossible_patterns. Pending such a proposal from @tmandry, I'd suggest that we not use this for that kind of case. (That's separate from the original rationale of improving linting/suggestions in other ways.) That would also make it less of a "hint".

I'll leave it to @tmandry to elaborate on that use case. This isn't a gate/blocker for adding this.

[tmandry]

@joshtriplett I responded in Zulip to keep technical discussion off this thread.

[apiraino]

@rustbot label -final-comment-period +major-change-accepted