[MIR] Make sure candidates are reversed before match_candidates.
#30553
Conversation
| .flat_map(|(target_block, mut target_candidates)| { | ||
| // We need to preserve the fact that the candidates | ||
| // are in the reversed order compared to the source. | ||
| target_candidates.reverse(); | ||
| self.match_candidates(span, |
There was a problem hiding this comment.
Perhaps match_candidates could be refactored to consume an iterator, thus allowing us to avoid a potentially expensive operation that is reversal of elements in a Vector?
There was a problem hiding this comment.
Alternatively you could just reverse the target_candidates iterator from which the vector is built from just above.
There was a problem hiding this comment.
I think I would prefer if sort_candidate above already produced list in the correct order.
There was a problem hiding this comment.
I tried reversing just the creation of the target_candidates Vec but that turned out to be a lot more annoying. So, instead I just changed all the code to deal with the candidates in the same order they're presented in the source rather than reversed.
|
@luqmana 😍 |
| @@ -361,15 +358,17 @@ impl<'a,'tcx> Builder<'a,'tcx> { | |||
|
|
|||
| // If all candidates were sorted into `target_candidates` somewhere, then | |||
| // the initial set was inexhaustive. | |||
| let untested_candidates = candidates.len() - tested_candidates; | |||
| let untested_candidates = unmatched_candidates.len() - tested_candidates; | |||
There was a problem hiding this comment.
Maybe do a let untestd_candidates = unmatched_candidates.split_off(tested_candidates) here and then next check becomes if untested_candidates.len() == 0.
|
Looks really good to me now. |
luqmana
force-pushed
the
mir-match-arm-guards
branch
from
3c937bc to
f88c808
Compare
|
@bors r+ Seems good. |
|
📌 Commit f88c808 has been approved by |
Fixes #30527. ```Rust fn main() { let _abc = match Some(101i8) { Some(xyz) if xyz > 100 => xyz, Some(_) => -1, None => -2 }; } ``` Resulting MIR now includes the `Some(xyz)` arm, guard and all:  ~~Not quite sure how to write a test for this.~~ Thinking too hard, just tested the end result. r? @nikomatsakis
Fixes #30527.
Resulting MIR now includes the
Some(xyz)arm, guard and all:Not quite sure how to write a test for this.Thinking too hard, just tested the end result.r? @nikomatsakis