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

RFC2229 Only compute place if upvars can be resolved #88039

Merged
merged 1 commit into from
Aug 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 23 additions & 15 deletions compiler/rustc_mir_build/src/build/matches/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
ascription: thir::Ascription { variance, user_ty, user_ty_span },
} => {
// Apply the type ascription to the value at `match_pair.place`, which is the
candidate.ascriptions.push(Ascription {
span: user_ty_span,
user_ty,
source: match_pair.place.clone().into_place(self.tcx, self.typeck_results),
variance,
});
if let Ok(place_resolved) =
match_pair.place.clone().try_upvars_resolved(self.tcx, self.typeck_results)
{
candidate.ascriptions.push(Ascription {
span: user_ty_span,
user_ty,
source: place_resolved.into_place(self.tcx, self.typeck_results),
variance,
});
}

candidate.match_pairs.push(MatchPair::new(match_pair.place, subpattern));

Expand All @@ -173,15 +177,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}

PatKind::Binding { name, mutability, mode, var, ty, ref subpattern, is_primary: _ } => {
candidate.bindings.push(Binding {
name,
mutability,
span: match_pair.pattern.span,
source: match_pair.place.clone().into_place(self.tcx, self.typeck_results),
var_id: var,
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

match_pair.place.clone().try_upvars_resolved(self.tcx, self.typeck_results)
{
candidate.bindings.push(Binding {
name,
mutability,
span: match_pair.pattern.span,
source: place_resolved.into_place(self.tcx, self.typeck_results),
var_id: var,
var_ty: ty,
binding_mode: mode,
});
}

if let Some(subpattern) = subpattern.as_ref() {
// this is the `x @ P` case; have to keep matching against `P` now
Expand Down
21 changes: 13 additions & 8 deletions compiler/rustc_mir_build/src/build/matches/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
suffix: &'pat [Pat<'tcx>],
) {
let tcx = self.tcx;
let (min_length, exact_size) = match place
.clone()
.into_place(tcx, self.typeck_results)
.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

place.clone().try_upvars_resolved(tcx, self.typeck_results)
{
ty::Array(_, length) => (length.eval_usize(tcx, self.param_env), true),
_ => ((prefix.len() + suffix.len()).try_into().unwrap(), false),
match place_resolved
.into_place(tcx, self.typeck_results)
.ty(&self.local_decls, tcx)
.ty
.kind()
{
ty::Array(_, length) => (length.eval_usize(tcx, self.param_env), true),
_ => ((prefix.len() + suffix.len()).try_into().unwrap(), false),
}
} else {
((prefix.len() + suffix.len()).try_into().unwrap(), false)
};

match_pairs.extend(prefix.iter().enumerate().map(|(idx, subpattern)| {
Expand Down
30 changes: 30 additions & 0 deletions src/test/ui/closures/2229_closure_analysis/issue-87987.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// run-pass
// edition:2021

struct Props {
field_1: u32, //~ WARNING: field is never read: `field_1`
field_2: u32, //~ WARNING: field is never read: `field_2`
}

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

field_1: 1,
field_2: 1,
};

let _ = || {
let _: Props = props_2;
};

// Test 2
let mut arr = [1, 3, 4, 5];

let mref = &mut arr;

let _c = || match arr {
[_, _, _, _] => println!("A")
};

println!("{:#?}", mref);
}
24 changes: 24 additions & 0 deletions src/test/ui/closures/2229_closure_analysis/issue-87987.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
warning: unused variable: `props_2`
--> $DIR/issue-87987.rs:11:9
|
LL | let props_2 = Props {
| ^^^^^^^ help: if this is intentional, prefix it with an underscore: `_props_2`
|
= note: `#[warn(unused_variables)]` on by default

warning: field is never read: `field_1`
--> $DIR/issue-87987.rs:5:5
|
LL | field_1: u32,
| ^^^^^^^^^^^^
|
= note: `#[warn(dead_code)]` on by default

warning: field is never read: `field_2`
--> $DIR/issue-87987.rs:6:5
|
LL | field_2: u32,
| ^^^^^^^^^^^^

warning: 3 warnings emitted