WIP: Fix codegen bug for mixed range and literal patterns #21313
Conversation
|
r? @pcwalton (rust_highfive has picked a reviewer for you, use r? to override) |
|
cc @jakub- |
| ast::PatTup(ref pats) => pats.iter().all(|p| all_wild_pat(&**p)), | ||
| _ => false, | ||
| } | ||
| } |
There was a problem hiding this comment.
I think this should be made more general by checking the usefulness of a wild pattern (PatWildSingle) against a matrix of a single pattern pat, much like we check reachability.
There was a problem hiding this comment.
But this is in trans already so the reachability has been checked (and passed) in typeck, isn't it?
There was a problem hiding this comment.
Ah, I misread your comment. Will do.
There was a problem hiding this comment.
@jakub-, it works except that a constant pattern such as:
enum A { AA }
const B: A = A::AA;
match A::AA {
B => ()
}triggers const pattern should've been rewritten error condition in your check_match::pat_constructors function. I'm not terribly familiar with it. Any thoughts?
There was a problem hiding this comment.
@edwardw I think you'll have to use arm_pats rather than arms for that check. arm_pats have already had the const patterns inlined.
There was a problem hiding this comment.
Works like a charm. Please review again, thanks.
|
@edwardw could you use a more descriptive title for the commit and PR ? |
edwardw
changed the title
edwardw
changed the title
|
@flaper87 done. |
|
@bors: r+ a87e04c |
|
⌛ Testing commit a87e04c with merge a52c28d... |
|
💔 Test failed - auto-mac-32-opt |
for match expressions to address comments.
If there are both range and literal patterns in the same column of a match expression, rustc may generate incorrect code. This patch fixes it. Closes rust-lang#18060
|
The patch has been redone and the test expanded. Please review again. |
|
On a side note, the latest commit can probably be used as a template to simplify |
|
r? @jakub- @nikomatsakis |
|
⌛ Testing commit d55c3fa with merge 57ae298... |
|
💔 Test failed - auto-mac-64-opt |
edwardw
changed the title
|
Oh, just realised this hadn't landed yet. @edwardw Did you manage to identify what the problem with your PR was? Otherwise I'll try to pick it up. :) It's a bit embarrassing to have this codegen issue in 1.0, IMHO. |
|
@jakub-, my patch had a regression, namely, |
If there are both range and literal patterns in the same column of a
match expression, rustc may generate incorrect code. This patch fixes
it.
Closes #18060