Skip to content

RFC2229 Only compute place if upvars can be resolved #88039

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Aug 20, 2021

Conversation

roxelo
Copy link
Member

@roxelo roxelo commented Aug 15, 2021

Closes #87987

This PR fixes an ICE when trying to unwrap an Err. This error appears when trying to convert a PlaceBuilder into Place when upvars can't yet be resolved. We should only try to convert a PlaceBuilder into Place if upvars can be resolved.

r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 15, 2021

fn main() {
// Test 1
let props_2 = Props { //~ WARNING: unused variable: `props_2`
Copy link
Member Author

Choose a reason for hiding this comment

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

This warning is inconsistent with what happens when let _: Props = props_2; is used outside of a closure. This is a different problem so I have created a new issue for it.

rust-lang/project-rfc-2229#57

@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

@roxelo have some questions...

var_ty: ty,
binding_mode: mode,
});
if let Ok(place_resolved) =
Copy link
Contributor

Choose a reason for hiding this comment

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

Under what conditions would this fail to succeed?

Copy link
Member

Choose a reason for hiding this comment

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

in the case we dont capture the root variable

.ty(&self.local_decls, tcx)
.ty
.kind()
let (min_length, exact_size) = if let Ok(place_resolved) =
Copy link
Contributor

Choose a reason for hiding this comment

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

likewise here

Copy link
Member Author

Choose a reason for hiding this comment

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

let _: Props = props_2; when we don't let it ice on line 158 of the simplify.rs file

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 19, 2021

📌 Commit 9c32b5b has been approved by nikomatsakis

@bors 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 Aug 19, 2021
@Mark-Simulacrum
Copy link
Member

@bors p=1 - wanted for crater in #87066

camsteffen added a commit to camsteffen/rust that referenced this pull request Aug 19, 2021
RFC2229 Only compute place if upvars can be resolved

Closes rust-lang#87987

This PR fixes an ICE when trying to unwrap an Err. This error appears when trying to convert a PlaceBuilder into Place when upvars can't yet be resolved. We should only try to convert a PlaceBuilder into Place if upvars can be resolved.

r? `@nikomatsakis`
@bors
Copy link
Collaborator

bors commented Aug 20, 2021

⌛ Testing commit 9c32b5b with merge 7611fe4...

@bors
Copy link
Collaborator

bors commented Aug 20, 2021

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing 7611fe4 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 20, 2021
@bors bors merged commit 7611fe4 into rust-lang:master Aug 20, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 20, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE: unwrap on an Err PlaceBuilder
8 participants