-
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
[mir-opt] simplify Repeat
s that don't actually repeat the operand
#135322
base: master
Are you sure you want to change the base?
Conversation
r? @Nadrieril rustbot has assigned @Nadrieril. Use |
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
This comment has been minimized.
This comment has been minimized.
b1a6483
to
699703e
Compare
// CHECK-LABEL: fn remove_generic_array | ||
// CHECK: debug a => const ZeroSized: [T; 0]; | ||
// CHECK-NOT: = []; | ||
let a = [x; 0]; |
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.
Also add a test where the zero comes from a constant not a literal
// CHECK: _0 = [move {{_[0-9]+}}]; | ||
// CHECK-NOT: [move {{_[0-9]+}}; 1] | ||
|
||
[x; 1] |
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.
Also test getting the 1
from a constant, not a literal. I think the normalization for that has already happened, but we've had some issues with that recently
// unreachable or can't be ZST | ||
_ => false, | ||
_ => Some(false), |
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.
exhaustively match here. This branch must not have false positives given that it means "definitely not"
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.
Good point. Spurious false
is benign in how it's actually used, but that's not what the comments say, so I should re-work it. Maybe just have the wildcard be _ => None
, since that's more obviously fine, and add the known-false cases explicitly.
☔ The latest upstream changes (presumably #135274) made this pull request unmergeable. Please resolve the merge conflicts. |
Created because when I was writing this case in GVN
https://github.com/rust-lang/rust/pull/133324/files#diff-292b215fdc6b59e3f3c4173c2270b14591c5673832fbfb05cd69a05c2ef0c30eR977-R979
I happened to notice that it worked for
[x]
but not for[x; 1]
, so figured it would be good to simplify thatRepeat
to the simplerAggregate
.