Fix drop handling for if let
expressions by matthewjasper · Pull Request #88572 · rust-lang/rust (original) (raw)
MIR lowering for if let
expressions is now more complicated now that
if let
exists in HIR. This PR adds a scope for the variables bound in
an if let
expression and then uses an approach similar to how we
handle loops to ensure that we reliably drop the correct variables.
bors added S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
bors added S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
and removed S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
labels
bors added S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
bors mentioned this pull request
This was referenced
Sep 4, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request
…shtriplett
Stabilize let_chains
in Rust 1.64
Stabilization proposal
This PR proposes the stabilization of #![feature(let_chains)]
in a future-compatibility way that will allow the possible addition of the EXPR is PAT
syntax.
Tracking issue: rust-lang#53667 Version: 1.64 (beta => 2022-08-11, stable => 2022-10-22).
What is stabilized
The ability to chain let expressions along side local variable declarations or ordinary conditional expressions. For example:
pub enum Color {
Blue,
Red,
Violet,
}
pub enum Flower {
Rose,
Tulip,
Violet,
}
pub fn roses_are_red_violets_are_blue_printer(
(first_flower, first_flower_color): (Flower, Color),
(second_flower, second_flower_color): (Flower, Color),
pick_up_lines: &[&str],
) {
if let Flower::Rose = first_flower
&& let Color::Red = first_flower_color
&& let Flower::Violet = second_flower
&& let Color::Blue = second_flower_color
&& let &[first_pick_up_line, ..] = pick_up_lines
{
println!("Roses are red, violets are blue, {}", first_pick_up_line);
}
}
fn main() {
roses_are_red_violets_are_blue_printer(
(Flower::Rose, Color::Red),
(Flower::Violet, Color::Blue),
&["sugar is sweet and so are you"],
);
}
Motivation
The main motivation for this feature is improving readability, ergonomics and reducing paper cuts.
For more examples, see the RFC.
What isn't stabilized
Let chains in match guards (
if_let_guard
)Resolution of divergent non-terminal matchers
The
EXPR is PAT
syntax
History
- On 2017-12-24, RFC: if- and while-let-chains
- On 2018-07-12, eRFC: if- and while-let-chains, take 2
- On 2018-08-24, Tracking issue for eRFC 2497, "if- and while-let-chains, take 2
- On 2019-03-19, Run branch cleanup after copy prop
- On 2019-03-26, Generalize diagnostic for x = y where bool is the expected type
- On 2019-04-24, Introduce hir::ExprKind::Use and employ in for loop desugaring
- On 2019-03-19, [let_chains, 1/6] Remove hir::ExprKind::If
- On 2019-05-15, [let_chains, 2/6] Introduce Let(..) in AST, remove IfLet + WhileLet and parse let chains
- On 2019-06-20, [let_chains, 3/6] And then there was only Loop
- On 2020-11-22, Reintroduce hir::ExprKind::If
- On 2020-12-24, Introduce hir::ExprKind::Let - Take 2
- On 2021-02-19, Lower condition of if expression before it's "then" block
- On 2021-09-01, Fix drop handling for
if let
expressions - On 2021-09-04, Formally implement let chains
- On 2022-01-19, Add tests to ensure that let_chains works with if_let_guard
- On 2022-01-18, Introduce
enhanced_binary_op
feature - On 2022-01-22, Fix
let_chains
andif_let_guard
feature flags - On 2022-02-25, Initiate the inner usage of
let_chains
- On 2022-01-28, [WIP] Introduce ast::StmtKind::LetElse to allow the usage of
let_else
withlet_chains
- On 2022-02-26, 1 - Make more use of
let_chains
- On 2022-02-26, 2 - Make more use of
let_chains
- On 2022-02-27, 3 - Make more use of
let_chains
- On 2022-02-28, 4 - Make more use of
let_chains
- On 2022-02-28, 5 - Make more use of
let_chains
- On 2022-02-28, 6 - Make more use of
let_chains
- On 2022-03-01, 7 - Make more use of
let_chains
- On 2022-03-01, 8 - Make more use of
let_chains
- On 2022-03-01, 9 - Make more use of
let_chains
- On 2022-03-08, Warn users about
||
in let chain expressions
From the first RFC (2017-12-24) to the theoretical future stabilization day (2022-10-22), it can be said that this feature took 4 years, 9 months and 28 days of research, development, discussions, agreements and headaches to be settled.
Divergent non-terminal matchers
More specifically, rust-lang#86730.
macro_rules! mac {
($e:expr) => {
if $e {
true
} else {
false
}
};
}
fn main() {
// OK!
assert_eq!(mac!(true && let 1 = 1), true);
// ERROR! Anything starting with `let` is not considered an expression
assert_eq!(mac!(let 1 = 1 && true), true);
}
To the best of my knowledge, such error or divergence is orthogonal, does not prevent stabilization and can be tackled independently in the near future or effectively in the next Rust 2024 edition. If not, then https://github.com/c410-f3r/rust/tree/let-macro-blah contains a set of changes that will consider let
an expression.
It is possible that none of the solutions above satisfies all applicable constraints but I personally don't know of any other plausible answers.
Alternative syntax
Taking into account the usefulness of this feature and the overwhelming desire to use both now and in the past, let PAT = EXPR
will be utilized for stabilization but it doesn't or shall create any obstacle for a possible future addition of EXPR is PAT
.
The introductory snippet would then be written as the following.
if first_flower is Flower::Rose
&& first_flower_color is Color::Red
&& second_flower is Flower::Violet
&& second_flower_color is Color::Blue
&& pick_up_lines is &[first_pick_up_line, ..]
{
println!("Roses are red, violets are blue, {}", first_pick_up_line);
}
Just to reinforce, this PR only unblocks a possible future road for EXPR is PAT
and does emphasize what is better or what is worse.
Tests
AST lowering does not wrap let chains in an
DropTemps
expressionA collection of statements where
let
expressions are forbiddenAll or at least most of the places where let chains are allowed
issue-88498.rs, issue-90722.rs, issue-92145.rs and issue-93150.rs were bugs found by third parties and fixed overtime.
Indexing was triggering a ICE due to a wrongly constructed MIR graph
Most of the infra-structure used by let chains is also used by if
expressions in stable compiler versions since rust-lang#80357 and rust-lang#88572. As a result, no bugs were found since the integration of rust-lang#88642.
Possible future work
Let chains in match guards is implemented and working but stabilization is blocked by
if_let_guard
.The usage of
let_chains
withlet_else
is possible but not implemented. Regardless, one attempt was introduced and closed in rust-lang#93437.
Thanks @Centril
for creating the RFC and huge thanks (again) to @matthewjasper
for all the reviews, mentoring and MIR implementations.
Fixes rust-lang#53667
workingjubilee pushed a commit to tcdi/postgrestd that referenced this pull request
Stabilize let_chains
in Rust 1.64
Stabilization proposal
This PR proposes the stabilization of #![feature(let_chains)]
in a future-compatibility way that will allow the possible addition of the EXPR is PAT
syntax.
Tracking issue: #53667 Version: 1.64 (beta => 2022-08-11, stable => 2022-10-22).
What is stabilized
The ability to chain let expressions along side local variable declarations or ordinary conditional expressions. For example:
pub enum Color {
Blue,
Red,
Violet,
}
pub enum Flower {
Rose,
Tulip,
Violet,
}
pub fn roses_are_red_violets_are_blue_printer(
(first_flower, first_flower_color): (Flower, Color),
(second_flower, second_flower_color): (Flower, Color),
pick_up_lines: &[&str],
) {
if let Flower::Rose = first_flower
&& let Color::Red = first_flower_color
&& let Flower::Violet = second_flower
&& let Color::Blue = second_flower_color
&& let &[first_pick_up_line, ..] = pick_up_lines
{
println!("Roses are red, violets are blue, {}", first_pick_up_line);
}
}
fn main() {
roses_are_red_violets_are_blue_printer(
(Flower::Rose, Color::Red),
(Flower::Violet, Color::Blue),
&["sugar is sweet and so are you"],
);
}
Motivation
The main motivation for this feature is improving readability, ergonomics and reducing paper cuts.
For more examples, see the RFC.
What isn't stabilized
Let chains in match guards (
if_let_guard
)Resolution of divergent non-terminal matchers
The
EXPR is PAT
syntax
History
- On 2017-12-24, RFC: if- and while-let-chains
- On 2018-07-12, eRFC: if- and while-let-chains, take 2
- On 2018-08-24, Tracking issue for eRFC 2497, "if- and while-let-chains, take 2
- On 2019-03-19, Run branch cleanup after copy prop
- On 2019-03-26, Generalize diagnostic for x = y where bool is the expected type
- On 2019-04-24, Introduce hir::ExprKind::Use and employ in for loop desugaring
- On 2019-03-19, [let_chains, 1/6] Remove hir::ExprKind::If
- On 2019-05-15, [let_chains, 2/6] Introduce Let(..) in AST, remove IfLet + WhileLet and parse let chains
- On 2019-06-20, [let_chains, 3/6] And then there was only Loop
- On 2020-11-22, Reintroduce hir::ExprKind::If
- On 2020-12-24, Introduce hir::ExprKind::Let - Take 2
- On 2021-02-19, Lower condition of if expression before it's "then" block
- On 2021-09-01, Fix drop handling for
if let
expressions - On 2021-09-04, Formally implement let chains
- On 2022-01-19, Add tests to ensure that let_chains works with if_let_guard
- On 2022-01-18, Introduce
enhanced_binary_op
feature - On 2022-01-22, Fix
let_chains
andif_let_guard
feature flags - On 2022-02-25, Initiate the inner usage of
let_chains
- On 2022-01-28, [WIP] Introduce ast::StmtKind::LetElse to allow the usage of
let_else
withlet_chains
- On 2022-02-26, 1 - Make more use of
let_chains
- On 2022-02-26, 2 - Make more use of
let_chains
- On 2022-02-27, 3 - Make more use of
let_chains
- On 2022-02-28, 4 - Make more use of
let_chains
- On 2022-02-28, 5 - Make more use of
let_chains
- On 2022-02-28, 6 - Make more use of
let_chains
- On 2022-03-01, 7 - Make more use of
let_chains
- On 2022-03-01, 8 - Make more use of
let_chains
- On 2022-03-01, 9 - Make more use of
let_chains
- On 2022-03-08, Warn users about
||
in let chain expressions
From the first RFC (2017-12-24) to the theoretical future stabilization day (2022-10-22), it can be said that this feature took 4 years, 9 months and 28 days of research, development, discussions, agreements and headaches to be settled.
Divergent non-terminal matchers
More specifically, rust-lang/rust#86730.
macro_rules! mac {
($e:expr) => {
if $e {
true
} else {
false
}
};
}
fn main() {
// OK!
assert_eq!(mac!(true && let 1 = 1), true);
// ERROR! Anything starting with `let` is not considered an expression
assert_eq!(mac!(let 1 = 1 && true), true);
}
To the best of my knowledge, such error or divergence is orthogonal, does not prevent stabilization and can be tackled independently in the near future or effectively in the next Rust 2024 edition. If not, then https://github.com/c410-f3r/rust/tree/let-macro-blah contains a set of changes that will consider let
an expression.
It is possible that none of the solutions above satisfies all applicable constraints but I personally don't know of any other plausible answers.
Alternative syntax
Taking into account the usefulness of this feature and the overwhelming desire to use both now and in the past, let PAT = EXPR
will be utilized for stabilization but it doesn't or shall create any obstacle for a possible future addition of EXPR is PAT
.
The introductory snippet would then be written as the following.
if first_flower is Flower::Rose
&& first_flower_color is Color::Red
&& second_flower is Flower::Violet
&& second_flower_color is Color::Blue
&& pick_up_lines is &[first_pick_up_line, ..]
{
println!("Roses are red, violets are blue, {}", first_pick_up_line);
}
Just to reinforce, this PR only unblocks a possible future road for EXPR is PAT
and does emphasize what is better or what is worse.
Tests
AST lowering does not wrap let chains in an
DropTemps
expressionA collection of statements where
let
expressions are forbiddenAll or at least most of the places where let chains are allowed
issue-88498.rs, issue-90722.rs, issue-92145.rs and issue-93150.rs were bugs found by third parties and fixed overtime.
Indexing was triggering a ICE due to a wrongly constructed MIR graph
Most of the infra-structure used by let chains is also used by if
expressions in stable compiler versions since rust-lang/rust#80357 and rust-lang/rust#88572. As a result, no bugs were found since the integration of rust-lang/rust#88642.
Possible future work
Let chains in match guards is implemented and working but stabilization is blocked by
if_let_guard
.The usage of
let_chains
withlet_else
is possible but not implemented. Regardless, one attempt was introduced and closed in rust-lang/rust#93437.
Thanks @Centril
for creating the RFC and huge thanks (again) to @matthewjasper
for all the reviews, mentoring and MIR implementations.
Fixes #53667
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request
…jasper
Don't pass a break scope to Builder::break_for_else
This method would previously take a target scope, and then verify that it was equal to the scope on top of the if-then scope stack.
In practice, this means that callers have to go out of their way to pass around redundant scope information that's already on the if-then stack.
So it's easier to just retrieve the correct scope directly from the if-then stack, and simplify the other code that was passing it around.
Both ways of indicating the break target were introduced in rust-lang#88572. I haven't been able to find any strong indication of whether this was done deliberately, or whether it was just an implementation artifact. But to me it doesn't seem useful to carefully pass around the same scope in two different ways.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#122137 - Zalathar:if-break-scope, r=matthewjasper
Don't pass a break scope to Builder::break_for_else
This method would previously take a target scope, and then verify that it was equal to the scope on top of the if-then scope stack.
In practice, this means that callers have to go out of their way to pass around redundant scope information that's already on the if-then stack.
So it's easier to just retrieve the correct scope directly from the if-then stack, and simplify the other code that was passing it around.
Both ways of indicating the break target were introduced in rust-lang#88572. I haven't been able to find any strong indication of whether this was done deliberately, or whether it was just an implementation artifact. But to me it doesn't seem useful to carefully pass around the same scope in two different ways.