Skip to content
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

Tracking issue for RFC 2294, "if let guard" #51114

Open
1 of 8 tasks
Centril opened this issue May 27, 2018 · 28 comments
Open
1 of 8 tasks

Tracking issue for RFC 2294, "if let guard" #51114

Centril opened this issue May 27, 2018 · 28 comments
Labels
B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-if_let_guard `#![feature(if_let_guard)]` S-tracking-impl-incomplete Status: The implementation is incomplete. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Centril
Copy link
Contributor

Centril commented May 27, 2018

This is a tracking issue for the RFC "if let guard" (rust-lang/rfcs#2294).

Steps:

Unresolved questions:

  • Work out the exact semantics / borrowck (cc @nikomatsakis, @pnkfelix)
    • Add unit tests analogous to the existing if guard ones that test if let guard instead
    • Ensure that the documentation of these details, if any, give equal footing to if and if let guards.
    • (If there is no documentation of if guards behavior w.r.t. move semantics, I do not think that should hold up stabilizing this feature. We can just file an issue that those things need to be documented.)
  • pnkfelix: The fact that both this feature and let else were implemented but had problems discovered relatively late with their handling of certain details w.r.t. locals and temporary r-values is a hint that we may need to revisit our approach to testing features like this more broadly, in terms of how we advise implementors of such features on how to probe for such cases (and maybe also in the form of some kind of language-oriented test-generation tooling?)
@Centril Centril added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-lang Relevant to the language team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels May 27, 2018
bors added a commit that referenced this issue Sep 1, 2018
refactor match guard

This is the first step to implement RFC 2294: if-let-guard. Tracking issue: #51114

The second step should be introducing another variant `IfLet` in the Guard enum. I separated them into 2 PRs for the convenience of reviewers.

r? @petrochenkov
@imjacobclark
Copy link
Contributor

👋 After a conversation about this change with @varkor I'm starting to take a look at this with a view to implementing it!

@alexreg
Copy link
Contributor

alexreg commented May 6, 2019

@imjacobclark Hey. Any progress over the past few months? :-)

@ebkalderon
Copy link
Contributor

Just ran into a case where this ability would be useful. I look forward to this feature landing someday!

@Vincent-liuwingsang
Copy link

+1 This would be a nice feature to have!

bors added a commit that referenced this issue Jun 23, 2019
Remove `ast::Guard`

With the introduction of `ast::ExprKind::Let` in #60861, the `ast::Guard` structure is now redundant in terms of representing [`if let` guards](#51114) in AST since it can be represented by `ExprKind::Let` syntactically. Therefore, we remove `ast::Guard` here.

However, we keep `hir::Guard` because the semantic representation is a different matter and this story is more unclear right now (might involve `goto 'arm` in HIR or something...).

r? @petrochenkov
Centril added a commit to Centril/rust that referenced this issue Jun 23, 2019
…rochenkov

Remove `ast::Guard`

With the introduction of `ast::ExprKind::Let` in rust-lang#60861, the `ast::Guard` structure is now redundant in terms of representing [`if let` guards](rust-lang#51114) in AST since it can be represented by `ExprKind::Let` syntactically. Therefore, we remove `ast::Guard` here.

However, we keep `hir::Guard` because the semantic representation is a different matter and this story is more unclear right now (might involve `goto 'arm` in HIR or something...).

r? @petrochenkov
bors added a commit that referenced this issue Jun 24, 2019
Remove `ast::Guard`

With the introduction of `ast::ExprKind::Let` in #60861, the `ast::Guard` structure is now redundant in terms of representing [`if let` guards](#51114) in AST since it can be represented by `ExprKind::Let` syntactically. Therefore, we remove `ast::Guard` here.

However, we keep `hir::Guard` because the semantic representation is a different matter and this story is more unclear right now (might involve `goto 'arm` in HIR or something...).

r? @petrochenkov
@Silhalnor
Copy link

Am currently learning Rust and thought it was unintuitive that I couldn't write this syntax. (Thought I must be mis-writing it somehow.) Lo, here it is in progress! Woo~

@varkor varkor added the E-help-wanted Call for participation: Help is requested to fix this issue. label Sep 28, 2019
@Boscop
Copy link

Boscop commented Mar 17, 2020

Any update on this? :)

@LeSeulArtichaut
Copy link
Contributor

I was willing to take a look at this, but I didn't have time to... I'll try to free some time for this ASAP!

@Antikyth
Copy link

Hi @Antikyth, thanks for the help! Here is, to my knowledge, the status of the work on if-let guards:

If you are interested in writing some tests or the Rust by Example chapter, please go ahead! Thanks again for your help.

I can see that if let guards are not documented in the unstable book. I suppose this is something that needs to be done?

I don't think that's particularly useful in the perspective of stabilization, but I might be wrong.

Ah, thanks for the reply! I'll have a look at:

  • where if let guards are used in my code, and where they could be used, so I can see if I can find any more issues or bad diagnostics
  • adding unit tests, if I can think of areas to test
  • contributing to a rust by example chapter

I may or may not get somewhere with those, will have to see if (a) I happen to have time to do so, (b) I can do something useful. But I'm happy to give it a try :)

@matthewjasper
Copy link
Contributor

matthewjasper commented May 21, 2023

Is the code here supposed to work? I would expect only direct use of a let expression (or maybe a chain with && if if let chains is also enabled) to be accepted.

https://github.com/rust-lang/rust/blob/master/compiler/rustc_mir_build/src/build/expr/as_operand.rs#L121

edit: Looks like it's not intended to be allowed (and was fixed by #111841)

weihanglo added a commit to weihanglo/rust that referenced this issue Aug 24, 2023
…r=cjgillot

Add more tests for if_let_guard

Adds tests for borrow checking, name shadowing and interaction with macros.

cc rust-lang#51114
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Sep 5, 2023
…, r=cjgillot

Make if let guard parsing consistent with normal guards

- Add tests that struct expressions are not allowed in `if let` and `while let` (no change, consistent with `if` and `while`)
- Allow struct expressions in `if let` guards (consistent with `if` guards).

r? `@cjgillot`

Closes rust-lang#93817
cc rust-lang#51114
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 6, 2023
…r=cjgillot

Make if let guard parsing consistent with normal guards

- Add tests that struct expressions are not allowed in `if let` and `while let` (no change, consistent with `if` and `while`)
- Allow struct expressions in `if let` guards (consistent with `if` guards).

r? `@cjgillot`

Closes rust-lang#93817
cc rust-lang#51114
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 20, 2023
…ests, r=compiler-errors

Add more if let guard tests

cc rust-lang#51114
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 20, 2023
Rollup merge of rust-lang#115965 - matthewjasper:extra-if-let-guard-tests, r=compiler-errors

Add more if let guard tests

cc rust-lang#51114
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 22, 2023
…guards, r=b-naber

Capture scrutinee of if let guards correctly

Previously we were always capturing by value.

cc rust-lang#51114
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 22, 2023
Rollup merge of rust-lang#115999 - matthewjasper:closure-capture-let-guards, r=b-naber

Capture scrutinee of if let guards correctly

Previously we were always capturing by value.

cc rust-lang#51114
@dtolnay
Copy link
Member

dtolnay commented Dec 7, 2023

I opened #118726 with a pretty-printer fix that is relevant to "if let guard". The rules for when struct literals must be enclosed in parentheses are different between if-let-guard and ordinary if-let.

#![feature(if_let_guard)]

macro_rules! stringify_expr {
    ($expr:expr) => {
        stringify!($expr)
    };
}

macro_rules! stringify_if_let {
    ($expr:expr) => {
        stringify_expr!(if let _ = $expr {})
    };
}

macro_rules! stringify_if_let_guard {
    ($expr:expr) => {
        stringify_expr!(match () { _ if let _ = $expr => {} })
    };
}

fn main() {
    println!("{}", stringify_if_let!(Struct {}));
    println!("{}", stringify_if_let_guard!(Struct {}));
}

stringify! must add parentheses around Struct {} in the if-let case, but it is not necessary to do so in the if-let-guard case because parsing of the expression is not terminated by { in that position.

if let _ = (Struct {}) {}
match () { _ if let _ = Struct {} => {} }

bors added a commit to rust-lang-ci/rust that referenced this issue Dec 25, 2023
…r=TaKO8Ki

Give temporaries in if let guards correct scopes

Temporaries in if-let guards have scopes that escape the match arm, this causes problems because the drops might be for temporaries that are not storage live. This PR changes the scope of temporaries in if-let guards to be limited to the arm:

```rust
_ if let Some(s) = std::convert::identity(&Some(String::new())) => {}
//                Temporary for Some(String::new()) is dropped here ^
```

We also now deduplicate temporaries between copies of the guard created for or-patterns:

```rust
// Only create a single Some(String::new()) temporary variable
_ | _ if let Some(s) = std::convert::identity(&Some(String::new())) => {}
```

This changes MIR building to pass around `ExprId`s rather than `Expr`s so that we have a way to index different expressions.

cc rust-lang#51114
Closes rust-lang#116079
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-if_let_guard `#![feature(if_let_guard)]` S-tracking-impl-incomplete Status: The implementation is incomplete. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests