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

mir_build: Use let-chains in prefix_slice_suffix, and note an edge case #135756

Closed
wants to merge 4 commits into from
Closed
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
45 changes: 33 additions & 12 deletions compiler/rustc_mir_build/src/builder/matches/match_pair.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,40 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
suffix: &'pat [Box<Pat<'tcx>>],
) {
let tcx = self.tcx;
let (min_length, exact_size) = if let Some(place_resolved) = place.try_to_place(self) {
match place_resolved.ty(&self.local_decls, tcx).ty.kind() {
ty::Array(_, length) => (
length
.try_to_target_usize(tcx)
.expect("expected len of array pat to be definite"),
true,
),
_ => ((prefix.len() + suffix.len()).try_into().unwrap(), false),
}

// Minimum length of the sequence being matched, needed by projections.
// If `exact_size` is true, that "minimum" is actually an exact length.
//
// The caller of `prefix_slice_suffix` is responsible for enforcing
// that length if necessary, e.g. by checking the scrutinee's length.
let min_length: u64;
let exact_size: bool;

if let Some(place_resolved) = place.try_to_place(self)
&& let ty::Array(_, length) = place_resolved.ty(&self.local_decls, tcx).ty.kind()
{
min_length =
length.try_to_target_usize(tcx).expect("expected len of array pat to be definite");
exact_size = true;
} else {
((prefix.len() + suffix.len()).try_into().unwrap(), false)
};
min_length = u64::try_from(prefix.len() + suffix.len()).unwrap();
exact_size = false;
}

// Normally `exact_size` is true iff this is an array pattern, but there
// is at least one edge-case exception. Consider code like this:
//
// ```ignore (illustrative)
// let arr = [1, 2, 3];
// let closure = || match arr {
// [_, ..] => {}
// };
// ```
//
// Under Rust 2021 disjoint-capture rules, the array place isn't
// actually captured, because no part of it is actually read or bound
// by the match. So the above code can't resolve it, and falls back to
// `exact_size = false`. This appears to be benign, but keep it in mind.

match_pairs.extend(prefix.iter().enumerate().map(|(idx, subpattern)| {
let elem =
Expand Down
1 change: 0 additions & 1 deletion src/tools/tidy/src/issues.txt
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,6 @@ ui/closure_context/issue-26046-fn-once.rs
ui/closure_context/issue-42065.rs
ui/closures/2229_closure_analysis/issue-118144.rs
ui/closures/2229_closure_analysis/issue-87378.rs
ui/closures/2229_closure_analysis/issue-87987.rs
ui/closures/2229_closure_analysis/issue-88118-2.rs
ui/closures/2229_closure_analysis/issue-88476.rs
ui/closures/2229_closure_analysis/issue-89606.rs
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// MIR for `prefix_only::{closure#0}` after built

fn prefix_only::{closure#0}(_1: &{closure@$DIR/array_non_capture.rs:21:19: 21:21}) -> u32 {
let mut _0: u32;

bb0: {
_0 = const 101_u32;
goto -> bb2;
}

bb1: {
unreachable;
}

bb2: {
return;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// MIR for `prefix_slice_only::{closure#0}` after built

fn prefix_slice_only::{closure#0}(_1: &{closure@$DIR/array_non_capture.rs:30:19: 30:21}) -> u32 {
let mut _0: u32;

bb0: {
_0 = const 102_u32;
goto -> bb2;
}

bb1: {
unreachable;
}

bb2: {
return;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// MIR for `prefix_slice_suffix::{closure#0}` after built

fn prefix_slice_suffix::{closure#0}(_1: &{closure@$DIR/array_non_capture.rs:39:19: 39:21}) -> u32 {
let mut _0: u32;

bb0: {
_0 = const 103_u32;
goto -> bb2;
}

bb1: {
unreachable;
}

bb2: {
return;
}
}
43 changes: 43 additions & 0 deletions tests/mir-opt/building/match/array_non_capture.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
//@ edition: 2021
// skip-filecheck

// Under the Rust 2021 disjoint capture rules, a "captured" place sometimes
// doesn't actually need to be captured, if it is only matched against
// irrefutable patterns that don't bind anything.
//
// When that happens, there is currently some MIR-building code
// (`Builder::prefix_slice_suffix`) that can no longer distinguish between
// array patterns and slice patterns, so it falls back to the code for dealing
// with slice patterns.
//
// That appears to be benign, but it's worth having a test that explicitly
// triggers the edge-case scenario. If someone makes a change that assumes the
// edge case can't happen, then hopefully this test will demand attention by
// either triggering an ICE, or needing its MIR to be re-blessed.

// EMIT_MIR array_non_capture.prefix_only-{closure#0}.built.after.mir
fn prefix_only() -> u32 {
let arr = [1, 2, 3];
let closure = || match arr {
[_, _, _] => 101u32,
};
closure()
}

// EMIT_MIR array_non_capture.prefix_slice_only-{closure#0}.built.after.mir
fn prefix_slice_only() -> u32 {
let arr = [1, 2, 3];
let closure = || match arr {
[_, ..] => 102u32,
};
closure()
}

// EMIT_MIR array_non_capture.prefix_slice_suffix-{closure#0}.built.after.mir
fn prefix_slice_suffix() -> u32 {
let arr = [1, 2, 3];
let closure = || match arr {
[_, .., _] => 103u32,
};
closure()
}
27 changes: 0 additions & 27 deletions tests/ui/closures/2229_closure_analysis/issue-87987.rs

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
//@ run-pass
//@ edition:2021

// When a closure syntactically captures a place, but doesn't actually capture
// it, make sure MIR building doesn't ICE when handling that place.
//
// Under the Rust 2021 disjoint capture rules, this sort of non-capture can
// occur when a place is only inspected by infallible non-binding patterns.

struct Props {
field_1: u32, //~ WARNING: fields `field_1` and `field_2` are never read
field_2: u32,
}

fn main() {
// Test 1
let props_2 = Props { field_1: 1, field_2: 1 };

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

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

let mref = &mut arr;

// These array patterns don't need to inspect the array, so the array
// isn't captured.
let _c = || match arr {
[_, _, _, _] => println!("C"),
};
let _d = || match arr {
[_, .., _] => println!("D"),
};
let _e = || match arr {
[_, ..] => println!("E"),
};

println!("{:#?}", mref);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: fields `field_1` and `field_2` are never read
--> $DIR/issue-87987.rs:5:5
--> $DIR/unresolvable-upvar-issue-87987.rs:11:5
|
LL | struct Props {
| ----- fields in this struct
Expand Down
Loading