-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Conversation
@@ -0,0 +1,42 @@ | |||
//! When a closure syntactically captures a place, but doesn't actually capture | |||
//! it, make sure MIR building doesn't ICE when handling that place. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
r? @lqd |
Cool, thanks for the added note. r=me with CI green and commits squashed |
This also suppresses an irrelevant warning, to avoid having to re-bless the output snapshot.
Fully squashing would make git's heuristics lose track of the rename, so I've kept that separate from the other improvements. |
@bors r=lqd rollup |
Rename test to `unresolvable-upvar-issue-87987.rs` and add some notes Extracted from rust-lang#135756. I had to figure out what this test was trying to test, so I might as well write it down for future reference.
Rollup of 6 pull requests Successful merges: - rust-lang#135728 (document order of items in iterator from drain) - rust-lang#135829 (Rustc dev guide subtree update) - rust-lang#135886 (Document purpose of closure in from_fn.rs more clearly) - rust-lang#135977 (Fix `FormattingOptions` instantiation with `Default`) - rust-lang#135983 (Doc difference between extend and extend_from_slice) - rust-lang#135985 (Rename test to `unresolvable-upvar-issue-87987.rs` and add some notes) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#135971 (Properly report error when object type param default references self) - rust-lang#135977 (Fix `FormattingOptions` instantiation with `Default`) - rust-lang#135985 (Rename test to `unresolvable-upvar-issue-87987.rs` and add some notes) - rust-lang#135991 (Fix set_name in thread mod for NuttX) - rust-lang#136009 (bootstrap: Handle bootstrap lockfile race condition better) - rust-lang#136018 (Use short ty string for move errors) - rust-lang#136027 (Skip suggestions in `derive`d code) - rust-lang#136029 (Bootstrap: Don't move ownership of job object) - rust-lang#136034 (fix(bootstrap): deserialize null as `f64::NAN`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#135985 - Zalathar:whats-upvar, r=lqd Rename test to `unresolvable-upvar-issue-87987.rs` and add some notes Extracted from rust-lang#135756. I had to figure out what this test was trying to test, so I might as well write it down for future reference.
(I hadn’t mentioned it earlier, but GG on the branch name :) |
Extracted from #135756. I had to figure out what this test was trying to test, so I might as well write it down for future reference.