Skip to content

Allow while let chains on all editions #140204

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
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

est31
Copy link
Member

@est31 est31 commented Apr 23, 2025

PR #132833 has stabilized if and while let chains on edition 2024, with the reasoning that editions before 2024 have bad scoping wrt the else clause (#124085).

This argument applies to if let chains, but while doesn't require the rescoping behaviour of edition 2024. So we can actually stabilize it on all editions.

See also: #140198

@rustbot
Copy link
Collaborator

rustbot commented Apr 23, 2025

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 23, 2025
@est31
Copy link
Member Author

est31 commented Apr 23, 2025

Note: if this is merged, one needs to adjust rust-lang/edition-guide#337 . If this is NOT merged, one needs to adjust rust-lang/reference#1740 on the other hand, as there is only an "Edition differences" notice for if chains.

@BoxyUwU
Copy link
Member

BoxyUwU commented Apr 23, 2025

This needs a lang FCP right?

@est31
Copy link
Member Author

est31 commented Apr 23, 2025

Yes, a FCP is needed or at least a statement from them that no FCP is needed :). @traviscross has nominated #140198, maybe it makes sense to discuss this one in the same meeting as well?

@traviscross traviscross added T-lang Relevant to the language team, which will review and decide on the PR/issue. I-lang-nominated Nominated for discussion during a lang team meeting. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 23, 2025
@joshtriplett
Copy link
Member

@est31 If you're on an older edition, is there currently a clear error message if you try to use let chains, that tells you it's available in the Rust 2024 edition and newer?

That would help make sure people know what works in what edition, rather than just getting a parse error.

@tmandry
Copy link
Member

tmandry commented Apr 23, 2025

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 23, 2025

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

Concerns:

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!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Apr 23, 2025
@traviscross
Copy link
Contributor

@rfcbot reviewed

@traviscross traviscross added I-lang-radar Items that are on lang's radar and will need eventual work or consideration. and removed I-lang-nominated Nominated for discussion during a lang team meeting. labels Apr 23, 2025
@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Apr 23, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 23, 2025

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Apr 23, 2025
@BoxyUwU BoxyUwU added S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 23, 2025
@est31
Copy link
Member Author

est31 commented Apr 23, 2025

@joshtriplett

If you're on an older edition, is there currently a clear error message if you try to use let chains, that tells you it's available in the Rust 2024 edition and newer?

The error message says that the feature is unstable. Which is suboptimal of course. The way that #132833 was implemented is that it removed the gating for let chains on newer editions, so the feature is still there and unstable. Back when I filed the PR, this was a necessity as the compiler hadn't been on 2024 yet (and it makes heavy use of let chains). Now that it is on 2024, it's a different situation.

Once clippy has updated their rustc to one with the stabilization, and we know precisely which syntax is restricted to newer editions (which is what this PR and #140198 are about), I think we can introduce a proper error like there is for async await, remove the feature gate, and mark the feature as stabilized. One might even be able to use the machinery added by #115677, not sure. It might not make it into 1.88.0 any more but I think that's not a big problem.

@est31
Copy link
Member Author

est31 commented Apr 23, 2025

Hmmm doing a check, the bugs are still there with while let chains:

So it turns out that it's not as simple as I've thought... we'll probably need to add this as a concern to the FCP or maybe even cancel. I'm sorry for the back and forth.

As good news though, if I modify the examples to use match and if let guards, they don't reproduce the error/ICE on edition 2021, so we won't need #140198 I guess?

@traviscross
Copy link
Contributor

@rfcbot concern bugs

Let us know what you find there with more investigation.

@rfcbot rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Apr 23, 2025
@rfcbot rfcbot removed the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Apr 23, 2025
@bors
Copy link
Collaborator

bors commented Apr 26, 2025

☔ The latest upstream changes (presumably #140324) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants