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

Rename test to unresolvable-upvar-issue-87987.rs and add some notes #135985

Merged
merged 2 commits into from
Jan 25, 2025
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
1 change: 0 additions & 1 deletion src/tools/tidy/src/issues.txt
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,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
27 changes: 0 additions & 27 deletions tests/ui/closures/2229_closure_analysis/issue-87987.rs

This file was deleted.

14 changes: 0 additions & 14 deletions tests/ui/closures/2229_closure_analysis/issue-87987.stderr

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
//! When a closure syntactically captures a place, but doesn't actually capture
//! it, make sure MIR building doesn't ICE when handling that place.
Copy link
Member

Choose a reason for hiding this comment

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

pre-existing but if it's really about ICEs then this most likely doesn't need to be a run-pass test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You might be right, but one thing that makes me hesitant is that non-capturing array patterns have really bad test coverage in the test suite overall, in terms of invoking the later parts of the compiler.

So having this test run and not crash feels like it does provide some tiny bit of value? Not sure.

Copy link
Member

@lqd lqd Jan 24, 2025

Choose a reason for hiding this comment

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

We're not checking the output so we'd only catch some catastrophic runtime failure? That is, this could be a build-pass and still add coverage to the patterns' codegen. But either way it should be good to document why we want this to exercise the build/run stages.

//!
//! 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.

// FIXME(#135985): On its own, this test should probably just be check-pass.
// But there are few/no other tests that use non-binding array patterns and
// invoke the later parts of the compiler, so building/running has some value.

//@ run-pass
//@ edition:2021

#[expect(dead_code)]
struct Props {
field_1: u32,
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);
}
Loading