Reduce MIR dump file count for MIR-opt tests #110773
Conversation
| r? @oli-obk (rustbot has picked a reviewer for you, use r? to override) |
rustbot added A-testsuite 
This comment has been minimized.
This comment has been minimized.
mj10021 force-pushed the issue-109502-fix branch from 4a21da9 to 0766d14 Compare This comment has been minimized.
This comment has been minimized.
mj10021 force-pushed the issue-109502-fix branch from 0766d14 to 47144a2 Compare 
oli-obk left a comment
oli-obk left a comment There was a problem hiding this comment.
just a bunch of impl nits.
Please also add some comments explaining the new field and argument
mj10021 force-pushed the issue-109502-fix branch from 47144a2 to b4714ae Compare There was a problem hiding this comment.
What does this magic number 3 mean? (Add the answer as a comment to the code)
There was a problem hiding this comment.
Haha got it, adding
There was a problem hiding this comment.
You could also use rsplit and the nth(3) operation on the iterator
There was a problem hiding this comment.
Is the comment I added good or is there more info that is missing?
There was a problem hiding this comment.
Ugh sorry, github didn't send my comment because I used a review
There was a problem hiding this comment.
O no worries, thanks for the update 🙂
mj10021 force-pushed the issue-109502-fix branch 2 times, most recently from fe36aec to 0812a49 Compare There was a problem hiding this comment.
You should be able to use split_once here to avoid collecting or iterating
There was a problem hiding this comment.
You could also use rsplit and the nth(3) operation on the iterator
There was a problem hiding this comment.
Ugh sorry, github didn't send my comment because I used a review
| Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
| oops fixing |
blessed new test
| Thanks! @bors r+ rollup |
bors added S-waiting-on-bors 
| let proc_res = self.compile_test(should_run, self.should_emit_metadata(pm)); | ||
| let proc_res = self.compile_test(should_run, self.should_emit_metadata(pm), Vec::new()); |
There was a problem hiding this comment.
Why changing compile_test function at all, if in all places except one, last parameter will be Vec::new()? You can just add something like compile_test_with_passes and use it exactly in that one place.
There was a problem hiding this comment.
good point, I'll make a follow up with that change
| | ||
| alloc1 (static: FOO, size: 8, align: 4) { | ||
| ╾─alloc18─╼ 03 00 00 00 │ ╾──╼.... | ||
| ╾─alloc19─╼ 03 00 00 00 │ ╾──╼.... |
There was a problem hiding this comment.
Why this affects alloc numbers at all?
Rollup of 6 pull requests Successful merges: - rust-lang#103056 (Fix `checked_{add,sub}_duration` incorrectly returning `None` when `other` has more than `i64::MAX` seconds) - rust-lang#108801 (Implement RFC 3348, `c"foo"` literals) - rust-lang#110773 (Reduce MIR dump file count for MIR-opt tests) - rust-lang#110876 (Added default target cpu to `--print target-cpus` output and updated docs) - rust-lang#111068 (Improve check-cfg implementation) - rust-lang#111238 (btree_map: `Cursor{,Mut}::peek_prev` must agree) Failed merges: - rust-lang#110694 (Implement builtin # syntax and use it for offset_of!(...)) r? `@ghost` `@rustbot` modify labels: rollup
…oli-obk Issue 109502 follow up, remove unnecessary Vec::new() from compile_test() As mentioned in comment on PR rust-lang#110773 , adding a separate function to pass the test passes into the `dump-mir` is a bit nicer
6 participants
As referenced in issue #109502 , mir-opt tests previously used the -Zdump-mir=all flag, which generates very large output. This PR only dumps the passes under test, greatly reducing dump output.