[MIR] Fix equality checks in matching code #30651
Conversation
nagisa
force-pushed
the
mir-fix-equality-checks
branch
from
69c1caa to
39f74b0
Compare
| }; | ||
| PatternKind::Constant { value: literal } | ||
| let pat = const_eval::const_expr_to_pat(self.cx.tcx, const_expr, | ||
| pat.span); |
There was a problem hiding this comment.
Would be nice if we can use HAIR to avoid allocating here.
There was a problem hiding this comment.
Nevermind, that would overcomplicate too many things.
nagisa
force-pushed
the
mir-fix-equality-checks
branch
from
39f74b0 to
209d0a6
Compare
nagisa
force-pushed
the
mir-fix-equality-checks
branch
from
209d0a6 to
add7410
Compare
|
Given that this is how we implement matching on constants currently, I don't see any problems with this. |
|
📌 Commit add7410 has been approved by |
|
⌛ Testing commit add7410 with merge a9a42a0... |
|
💔 Test failed - auto-win-gnu-64-nopt-t |
|
@bors retry |
This is not a fix to checks themselves per se (though we still use `Eq` MIR test instead of calling `PartialEq::eq`), but rather how we handle items we encounter in pattern position.
Previously we would just call `PartialEq` with the constant and the matchee, but now we essentially inline the constant instead. E.g. these two snippets are functionally equivalent at MIR level:
```
match val { Some(42) => true, _ => false }
```
and
```
const SECRET: Option<u8> = Some(42);
match val { SECRET => true, _ => false }
```
This approach also allows for more optimizations of matches. I.e. It can now exploit `SwitchInt` to switch on number inside a `Some` regardless of whether the value being an item or not.
This is based on @tsion’s already approved PR so I could reuse the file for more tests.
r? @eddyb
cc @nikomatsakis @tsion
|
⌛ Testing commit add7410 with merge 191ff2d... |
|
💔 Test failed - auto-linux-64-nopt-t |
|
@bors retry |
|
⚡ Previous build results for auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-opt, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt are reusable. Rebuilding only auto-linux-64-nopt-t, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt... |
|
On Sun, Jan 03, 2016 at 01:21:16PM -0800, Eduard-Mihai Burtescu wrote:
Agreed (though I didn't yet look at the patch). I'd prefer to change |
This is not a fix to checks themselves per se (though we still use
EqMIR test instead of callingPartialEq::eq), but rather how we handle items we encounter in pattern position.Previously we would just call
PartialEqwith the constant and the matchee, but now we essentially inline the constant instead. E.g. these two snippets are functionally equivalent at MIR level:and
This approach also allows for more optimizations of matches. I.e. It can now exploit
SwitchIntto switch on number inside aSomeregardless of whether the value being an item or not.This is based on @tsion’s already approved PR so I could reuse the file for more tests.
r? @eddyb
cc @nikomatsakis @tsion