You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Tracking issue for RFC #2175: or-patterns in if / while let expressions #48215
Arguably we should retitle the issue and RFC to make it clear that the ultimately approved proposal actuallyincludeslet itself (in addition to if let and while let).
This seems likely to be a rather straightforward change: currently [...]
Er, this remark was terribly naïve: the change to the parser might be that simple, but then the IfLet (&c.) AST nodes have to hold a vector rather than a single pattern, and much of the code that handles them (e.g., HIR lowering) needs to change correspondingly.
Er, this remark was terribly naïve: the change to the parser might be that simple, but then the IfLet (&c.) AST nodes have to hold a vector rather than a single pattern, and much of the code that handles them (e.g., HIR lowering) needs to change correspondingly.
Sounds about right. but it should still be something that can be done purely in HIR lowering, right? I would think so.
I would argue yes because examining a large number of cases in that fashion would be advantageous over non-prepended verts (assuming you prefer the style). Maintaining consistency with match definitely is a benefit.
Additionally, I wouldn't expect many downsides to this. The main ones would be a complexity increase, dislike from people who disapprove of the style, and perhaps macro particulars being more complicated.
@petrochenkov the let Ok(x) | Err(x) bits and the changes in rust-lang/rfcs#2530 are not implemented as far as I know, both of which are part of RFC 2175 + amendment.
@Centril
As I said in one of the previous comments, let Ok(x) | Err(x) is a complex feature equivalent to full #54883, and the leading vert can be added in the stabilization PR or later, it's not a blocker/back-compat concern.
My goal here is to stabilize this occasionally useful feature now, and not delay its stabilization on larger implementation work required to support its extensions.
@petrochenkov being the new kid on the lang team block, I'm unsure as to whether we are OK with stabilizing parts of RFCs or whether we require them to come in units... In particular, @nikomatsakis noted in RFC 2175 that let Ok(x) | Err(x) = ... was a requirement to accept 2175... But the team is in agreement to go much further; so I don't know. I will defer to Niko here (or someone who has been in @rust-lang/lang longer than me...).
I think it's fine to stabilize part of an RFC, we do it all the time. What I would recommend is that we open up a new issue dedicated to the stabilization (this is what we should always do, in my opinion). The issue header can describe clearly :
What behavior is being stabilized
How this deviates from the RFC (it it does)
What unresolved questions are being resolved through this stabilization
What behavior is NOT being stabilized
and why
Test cases for all of the above
A good example where we did this "staged stabilization" was for the ? in main RFC. If you check out the tracking issue #43301 you will see that it has a section called "Stabilizations", which lists two sub-issues:
Those issues are examples of the kind of report I would like to see (I did them myself, so asking for anything more would be somewhat hypocritical I suppose :)
@petrochenkov could you write up a report per #48215 (comment) in a new issue (or PR at your option...)? I'll fcp merge that :) I'd like the stabilization PR to include leading vert at least also.
Activity
zackmdavis commentedon Feb 15, 2018
This seems likely to be a rather straightforward change: currently
parse_armcallsparse_patsto parse the|-separated patterns of amatcharm, whereasparse_if_let_exprjust callsparse_pat.I'd like to take this.
zackmdavis commentedon Feb 15, 2018
Arguably we should retitle the issue and RFC to make it clear that the ultimately approved proposal actually includes
letitself (in addition toif letandwhile let).zackmdavis commentedon Feb 15, 2018
Er, this remark was terribly naïve: the change to the parser might be that simple, but then the
IfLet(&c.) AST nodes have to hold a vector rather than a single pattern, and much of the code that handles them (e.g., HIR lowering) needs to change correspondingly.nikomatsakis commentedon Feb 15, 2018
@zackmdavis
Sounds about right. but it should still be something that can be done purely in HIR lowering, right? I would think so.
zackmdavis commentedon Feb 15, 2018
Visitors, pretty-printing, and resolution are also affected.
nikomatsakis commentedon Feb 15, 2018
Sounds about right. I guess what I meant is "after HIR lowering, I wouldn't expect much other code to care"
zackmdavis commentedon Feb 24, 2018
(I was actively working on this today, but would seem to have been pre-empted by #48490.)
Rollup merge of rust-lang#48490 - petrochenkov:orpat, r=eddyb
dtolnay commentedon Mar 31, 2018
Does optional leading vert need to be supported here for consistency with #44101?
@mdinger
mdinger commentedon Apr 1, 2018
@dtolnay
I would argue yes because examining a large number of cases in that fashion would be advantageous over non-prepended verts (assuming you prefer the style). Maintaining consistency with
matchdefinitely is a benefit.Additionally, I wouldn't expect many downsides to this. The main ones would be a complexity increase, dislike from people who disapprove of the style, and perhaps macro particulars being more complicated.
mdinger commentedon Apr 1, 2018
Also, the RFC grammar should be updated to reflect this change.
10 remaining items
petrochenkov commentedon Nov 1, 2018
@Centril
Could you start an FCP for stabilization?
The feature was implemented in #48490 as trivial desugaring into
match, that PR also has tests.Centril commentedon Nov 1, 2018
@petrochenkov the
let Ok(x) | Err(x)bits and the changes in rust-lang/rfcs#2530 are not implemented as far as I know, both of which are part of RFC 2175 + amendment.petrochenkov commentedon Nov 1, 2018
@Centril
As I said in one of the previous comments,
let Ok(x) | Err(x)is a complex feature equivalent to full #54883, and the leading vert can be added in the stabilization PR or later, it's not a blocker/back-compat concern.My goal here is to stabilize this occasionally useful feature now, and not delay its stabilization on larger implementation work required to support its extensions.
Centril commentedon Nov 1, 2018
@petrochenkov being the new kid on the lang team block, I'm unsure as to whether we are OK with stabilizing parts of RFCs or whether we require them to come in units... In particular, @nikomatsakis noted in RFC 2175 that
let Ok(x) | Err(x) = ...was a requirement to accept 2175... But the team is in agreement to go much further; so I don't know. I will defer to Niko here (or someone who has been in @rust-lang/lang longer than me...).nikomatsakis commentedon Nov 2, 2018
I think it's fine to stabilize part of an RFC, we do it all the time. What I would recommend is that we open up a new issue dedicated to the stabilization (this is what we should always do, in my opinion). The issue header can describe clearly :
A good example where we did this "staged stabilization" was for the
?in main RFC. If you check out the tracking issue #43301 you will see that it has a section called "Stabilizations", which lists two sub-issues:mainwith non-() return types (Stabilizemainwith non-() return types #48453) Merged in Stabilize termination_trait, split out termination_trait_test #49162()return type #48854)Those issues are examples of the kind of report I would like to see (I did them myself, so asking for anything more would be somewhat hypocritical I suppose :)
Centril commentedon Nov 2, 2018
@petrochenkov could you write up a report per #48215 (comment) in a new issue (or PR at your option...)? I'll fcp merge that :) I'd like the stabilization PR to include leading vert at least also.
petrochenkov commentedon Nov 2, 2018
Ok, will do.
Centril commentedon Nov 25, 2018
Proposed in #48215.
glaebhoerl commentedon Nov 25, 2018
@Centril You mean #56212 😛
Centril commentedon Nov 25, 2018
@glaebhoerl hihi =P // blames copy buffer
Centril commentedon Jan 12, 2019
Stabilization happened in #57532.
Documentation issue: rust-lang/reference#505
The remaining work is tracked in #54883.