Skip to content

Fake capture closures if typeck results are empty - #100452

Closed
ouz-a wants to merge 2 commits into
rust-lang:masterfrom
ouz-a:issue-93242
Closed

Fake capture closures if typeck results are empty#100452
ouz-a wants to merge 2 commits into
rust-lang:masterfrom
ouz-a:issue-93242

Conversation

@ouz-a

@ouz-aouz-a commented Aug 12, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

This ICE happens because closure_min_captures is empty, the reason it's empty is with the 2021 edition enable_precise_capture is set to true, which makes it so that we can't fake capture any information because that result of the unwrap is none hence the ICE.

Other solution is editing maybe_read_scrutinee to this since empty slice contains no sub patterns.

Fixes#93242

PatKind::Slice(_, ref slice, _) => {if slice.is_none(){
need_to_be_read = true;}}// instead ofPatKind::Or(_)
| PatKind::Box(_)
| PatKind::Slice(..)
| PatKind::Ref(..)
| PatKind::Wild => {}

@rustbotrustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 12, 2022
@rust-highfive

rust-highfive commented Aug 12, 2022

Copy link
Copy Markdown
Contributor

r? @nagisa

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 12, 2022
@nagisa

nagisa commented Aug 15, 2022

Copy link
Copy Markdown
Member

r? rust-lang/compiler

@ouz-a

ouz-a commented Aug 30, 2022

Copy link
Copy Markdown
ContributorAuthor

r? rust-lang/compiler

@ouz-a

ouz-a commented Sep 7, 2022

Copy link
Copy Markdown
ContributorAuthor

r? rust-lang/compiler

@jackh726

jackh726 commented Sep 7, 2022

Copy link
Copy Markdown
Member

Should ping @nikomatsakis about this. He probably has the most background on this.

jackh726
jackh726 reviewed Sep 7, 2022
View reviewed changes
Comment threadcompiler/rustc_typeck/src/check/upvar.rs Outdated

@jackh726jackh726Sep 7, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update this comment to include an explanation for the added logic? And also add a reference to the issue number?

@jackh726

jackh726 commented Sep 7, 2022

Copy link
Copy Markdown
Member

r=me with updated comment

@fee1-deadfee1-dead assigned jackh726 and unassigned fee1-deadSep 8, 2022
@jackh726jackh726 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 12, 2022
@ouz-a

ouz-a commented Sep 23, 2022

Copy link
Copy Markdown
ContributorAuthor

r? @jackh726

@ouz-a

ouz-a commented Sep 23, 2022•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

cc @nikomatsakis I feel like there is a bigger issue I am missing here and this fix feels a bit like hack, is there anything you would like to add?

@bors

bors commented Sep 27, 2022

Copy link
Copy Markdown
Collaborator

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

@ouz-a
ouz-a requested a review from jackh726October 21, 2022 11:07
@bors

bors commented Oct 21, 2022

Copy link
Copy Markdown
Collaborator

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

@jackh726

jackh726 commented Oct 21, 2022

Copy link
Copy Markdown
Member

@bors delegate+

r=me after rebase

@bors

bors commented Oct 21, 2022

Copy link
Copy Markdown
Collaborator

✌️ @ouz-a can now approve this pull request

@ouz-a

ouz-a commented Oct 23, 2022

Copy link
Copy Markdown
ContributorAuthor

@bors r=jackh726

@Dylan-DPCDylan-DPC mentioned this pull request Oct 24, 2022
Closed
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Oct 24, 2022
@Dylan-DPC
Rollup merge ofrust-lang#100452- ouz-a:issue-93242, r=jackh726
48ade8e
Fake capture closures if typeck results are empty
This ICE happens because `closure_min_captures` is empty, the reason it's empty is with the 2021 edition `enable_precise_capture` is set to true, which makes it so that we can't fake capture any information because that result of the `unwrap` is none hence the ICE.
Other solution is editing [maybe_read_scrutinee](https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_typeck/expr_use_visitor.rs.html#453-463) to this since empty slice contains no sub patterns.
Fixesrust-lang#93242
```rust
PatKind::Slice(_, ref slice, _) => {
if slice.is_none(){
need_to_be_read = true;
}
}
// instead of
PatKind::Or(_)
| PatKind::Box(_)
| PatKind::Slice(..)
| PatKind::Ref(..)
| PatKind::Wild => {}
```
@Dylan-DPCDylan-DPC mentioned this pull request Oct 24, 2022
Closed
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Oct 25, 2022
@Dylan-DPC
Rollup merge ofrust-lang#100452- ouz-a:issue-93242, r=jackh726
8c5d4e6
Fake capture closures if typeck results are empty
This ICE happens because `closure_min_captures` is empty, the reason it's empty is with the 2021 edition `enable_precise_capture` is set to true, which makes it so that we can't fake capture any information because that result of the `unwrap` is none hence the ICE.
Other solution is editing [maybe_read_scrutinee](https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_typeck/expr_use_visitor.rs.html#453-463) to this since empty slice contains no sub patterns.
Fixesrust-lang#93242
```rust
PatKind::Slice(_, ref slice, _) => {
if slice.is_none(){
need_to_be_read = true;
}
}
// instead of
PatKind::Or(_)
| PatKind::Box(_)
| PatKind::Slice(..)
| PatKind::Ref(..)
| PatKind::Wild => {}
```
@Dylan-DPCDylan-DPC mentioned this pull request Oct 25, 2022
Closed
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Oct 26, 2022
@compiler-errors
Rollup merge ofrust-lang#100452- ouz-a:issue-93242, r=jackh726
355e23e
Fake capture closures if typeck results are empty
This ICE happens because `closure_min_captures` is empty, the reason it's empty is with the 2021 edition `enable_precise_capture` is set to true, which makes it so that we can't fake capture any information because that result of the `unwrap` is none hence the ICE.
Other solution is editing [maybe_read_scrutinee](https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_typeck/expr_use_visitor.rs.html#453-463) to this since empty slice contains no sub patterns.
Fixesrust-lang#93242
```rust
PatKind::Slice(_, ref slice, _) => {
if slice.is_none(){
need_to_be_read = true;
}
}
// instead of
PatKind::Or(_)
| PatKind::Box(_)
| PatKind::Slice(..)
| PatKind::Ref(..)
| PatKind::Wild => {}
```
@compiler-errorscompiler-errors mentioned this pull request Oct 26, 2022
Closed
@Dylan-DPCDylan-DPC mentioned this pull request Oct 28, 2022
Closed
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Oct 28, 2022
@Dylan-DPC
Rollup merge ofrust-lang#100452- ouz-a:issue-93242, r=jackh726
eb6cd05
Fake capture closures if typeck results are empty
This ICE happens because `closure_min_captures` is empty, the reason it's empty is with the 2021 edition `enable_precise_capture` is set to true, which makes it so that we can't fake capture any information because that result of the `unwrap` is none hence the ICE.
Other solution is editing [maybe_read_scrutinee](https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_typeck/expr_use_visitor.rs.html#453-463) to this since empty slice contains no sub patterns.
Fixesrust-lang#93242
```rust
PatKind::Slice(_, ref slice, _) => {
if slice.is_none(){
need_to_be_read = true;
}
}
// instead of
PatKind::Or(_)
| PatKind::Box(_)
| PatKind::Slice(..)
| PatKind::Ref(..)
| PatKind::Wild => {}
```
@Dylan-DPC

Dylan-DPC commented Oct 28, 2022

Copy link
Copy Markdown
Member

failed in rollup

@bors r-

@borsbors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 28, 2022
@rustbot

rustbot commented Oct 31, 2022

Copy link
Copy Markdown
Collaborator

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@ouz-a

ouz-a commented Oct 31, 2022

Copy link
Copy Markdown
ContributorAuthor

I am not sure about the results of these tests would love to get some approval from @rust-lang/clippy

@bors

bors commented Dec 29, 2022

Copy link
Copy Markdown
Collaborator

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

@jackh726

jackh726 commented Dec 30, 2022

Copy link
Copy Markdown
Member

Well, thank goodness for clippy. This PR regresses the following code with edition 2021 (it fails for previous editions):

#[derive(Clone)]structAlpha;fnclone_then_move_cloned(){fnfoo<F:Fn()>(_:&Alpha, _:F){}let x = Alpha;// ok, data is moved while the clone is in use.foo(&x,move || {let _ = x;});}fnmain(){clone_then_move_cloned()}

Can you please add the above as a test? Looks like we need to come up with a different solution.

@flip1995

flip1995 commented Dec 30, 2022

Copy link
Copy Markdown
Member

Yeah this Clippy test change looks fishy. Usually when a Clippy test changes without any Clippy changes it means some assertions Clippy made about compiler internals changed.

@ouz-a

ouz-a commented Dec 30, 2022

Copy link
Copy Markdown
ContributorAuthor

Can you please add the above as a test? Looks like we need to come up with a different solution.

Yeah, will look into another solution.

@jackh726

jackh726 commented Feb 18, 2023

Copy link
Copy Markdown
Member

@ouz-a any updates here?

@ouz-a

ouz-a commented Feb 19, 2023

Copy link
Copy Markdown
ContributorAuthor

@ouz-a any updates here?

Sorry completely forget about this, going to try to solve this, this week if can't solve it will post what I found.

@bors

bors commented Mar 1, 2023

Copy link
Copy Markdown
Collaborator

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

@ouz-a

ouz-a commented Mar 9, 2023

Copy link
Copy Markdown
ContributorAuthor

I spent some time trying to uncover what's happening but I couldn't find a solution:
For the code below [] is captured as Upvar and to_upvars_resolved_place_builder can't resolve it, on the other hand when we remove // from [1], [] is captured as Local and everything works, however I couldn't find out why this behavior emerges so I'm closing this PR in favor of better future solution to this issue.

pubasyncfnsomething(path:&[usize]) -> usize{// Without this async block it doesn't ICEasync{match path {[] => 1,//[1] => 2,
_ => 1,}}.await}

@ouz-aouz-a closed this Mar 9, 2023
@ouz-a
ouz-a deleted the issue-93242 branch July 26, 2023 12:56
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jackh726jackh726Awaiting requested review from jackh726

Assignees

@jackh726jackh726

Labels

S-waiting-on-authorStatus: This is awaiting some action (such as code changes or more information) from the author.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ICE for async block containing match on empty slice and wildcard

10 participants

@ouz-a@rust-highfive@nagisa@jackh726@bors@Dylan-DPC@rustbot@flip1995@wesleywiser@fee1-dead