Skip to content

Always capture slice when pattern requires checking the length #111831

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

Merged
merged 1 commit into from
May 26, 2023
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
19 changes: 13 additions & 6 deletions compiler/rustc_hir_typeck/src/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,12 +438,19 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
// to borrow discr.
needs_to_be_read = true;
}
PatKind::Or(_)
| PatKind::Box(_)
| PatKind::Slice(..)
| PatKind::Ref(..)
| PatKind::Wild => {
// If the PatKind is Or, Box, Slice or Ref, the decision is made later
PatKind::Slice(lhs, wild, rhs) => {
// We don't need to test the length if the pattern is `[..]`
if matches!((lhs, wild, rhs), (&[], Some(_), &[]))
// Arrays have a statically known size, so
// there is no need to read their length
|| discr_place.place.base_ty.is_array()
Copy link
Member

@BoxyUwU BoxyUwU Jun 14, 2023

Choose a reason for hiding this comment

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

Does this cause us to emit reads when matching on &[T; N]? If so that might be the cause of #112607

Copy link
Member

Choose a reason for hiding this comment

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

It doesn't matter if this is the cause, gce is free to be broken

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does yes. I'll fix it now, since this is definitely incorrect right now

{
} else {
needs_to_be_read = true;
}
}
PatKind::Or(_) | PatKind::Box(_) | PatKind::Ref(..) | PatKind::Wild => {
// If the PatKind is Or, Box, or Ref, the decision is made later
// as these patterns contains subpatterns
// If the PatKind is Wild, the decision is made based on the other patterns being
// examined
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
// edition:2021

#![feature(rustc_attrs)]
#![feature(stmt_expr_attributes)]

// Should capture the discriminant since a variant of a multivariant enum is
// mentioned in the match arm; the discriminant is captured by the closure regardless
// of if it creates a binding
fn test_1_should_capture() {
let variant = Some(2229);
let c = #[rustc_capture_analysis]
//~^ ERROR: attributes on expressions are experimental
//~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>

|| {
//~^ First Pass analysis includes:
//~| Min Capture analysis includes:
Expand All @@ -29,8 +27,6 @@ fn test_1_should_capture() {
fn test_2_should_not_capture() {
let variant = Some(2229);
let c = #[rustc_capture_analysis]
//~^ ERROR: attributes on expressions are experimental
//~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
|| {
//~^ First Pass analysis includes:
match variant {
Expand All @@ -50,8 +46,6 @@ enum SingleVariant {
fn test_3_should_not_capture_single_variant() {
let variant = SingleVariant::Points(1);
let c = #[rustc_capture_analysis]
//~^ ERROR: attributes on expressions are experimental
//~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
|| {
//~^ First Pass analysis includes:
match variant {
Expand All @@ -66,8 +60,6 @@ fn test_3_should_not_capture_single_variant() {
fn test_6_should_capture_single_variant() {
let variant = SingleVariant::Points(1);
let c = #[rustc_capture_analysis]
//~^ ERROR: attributes on expressions are experimental
//~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
|| {
//~^ First Pass analysis includes:
//~| Min Capture analysis includes:
Expand All @@ -88,8 +80,6 @@ fn test_6_should_capture_single_variant() {
fn test_4_should_not_capture_array() {
let array: [i32; 3] = [0; 3];
let c = #[rustc_capture_analysis]
//~^ ERROR: attributes on expressions are experimental
//~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
|| {
//~^ First Pass analysis includes:
match array {
Expand All @@ -112,8 +102,6 @@ enum MVariant {
fn test_5_should_capture_multi_variant() {
let variant = MVariant::A;
let c = #[rustc_capture_analysis]
//~^ ERROR: attributes on expressions are experimental
//~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
|| {
//~^ First Pass analysis includes:
//~| Min Capture analysis includes:
Expand All @@ -127,11 +115,69 @@ fn test_5_should_capture_multi_variant() {
c();
}

// Even though all patterns are wild, we need to read the discriminant
// in order to test the slice length
fn test_7_should_capture_slice_len() {
let slice: &[i32] = &[1, 2, 3];
let c = #[rustc_capture_analysis]
|| {
//~^ First Pass analysis includes:
//~| Min Capture analysis includes:
match slice {
//~^ NOTE: Capturing slice[] -> ImmBorrow
//~| NOTE: Min Capture slice[] -> ImmBorrow
[_,_,_] => {},
_ => {}
}
};
c();
let c = #[rustc_capture_analysis]
|| {
//~^ First Pass analysis includes:
//~| Min Capture analysis includes:
match slice {
//~^ NOTE: Capturing slice[] -> ImmBorrow
//~| NOTE: Min Capture slice[] -> ImmBorrow
[] => {},
_ => {}
}
};
c();
let c = #[rustc_capture_analysis]
|| {
//~^ First Pass analysis includes:
//~| Min Capture analysis includes:
match slice {
//~^ NOTE: Capturing slice[] -> ImmBorrow
//~| NOTE: Min Capture slice[] -> ImmBorrow
[_, .. ,_] => {},
_ => {}
}
};
c();
}

// Wild pattern that doesn't bind, so no capture
fn test_8_capture_slice_wild() {
let slice: &[i32] = &[1, 2, 3];
let c = #[rustc_capture_analysis]
|| {
//~^ First Pass analysis includes:
match slice {
[..] => {},
_ => {}
}
};
c();
}

fn main() {
test_1_should_capture();
test_2_should_not_capture();
test_3_should_not_capture_single_variant();
test_6_should_capture_single_variant();
test_4_should_not_capture_array();
test_5_should_capture_multi_variant();
test_7_should_capture_slice_len();
test_8_capture_slice_wild();
}
Loading