mirror of
https://github.com/rust-lang/rust.git
synced 2026-05-22 02:00:00 +03:00
Matthias Krüger
7afed92749
Simpler checked shifts in MIR building Doing masking to check unsigned shift amounts is overcomplicated; just comparing the shift directly saves a statement and a temporary, as well as is much easier to read as a human. And shifting by unsigned is the canonical case -- notably, all the library shifting methods (that don't support every type) take shift RHSs as `u32` -- so we might as well make that simpler since it's easy to do so. This PR also changes *signed* shift amounts to `IntToInt` casts and then uses the same check as for unsigned. The bit-masking is a nice trick, but for example LLVM actually canonicalizes it to an unsigned comparison anyway <https://rust.godbolt.org/z/8h59fMGT4> so I don't think it's worth the effort and the extra `Constant`. (If MIR's `assert` was `assert_nz` then the masking might make sense, but when the `!=` uses another statement I think the comparison is better.) To review, I suggest looking at https://github.com/rust-lang/rust/pull/109475/commits/2ee0468c49be9f8ea68b7b935ea1f4c2555849e9 first -- that's the interesting code change and has a MIR diff. My favourite part of the diff: ```diff - _20 = BitAnd(_19, const 340282366920938463463374607431768211448_u128); // scope 0 at $DIR/shifts.rs:+2:34: +2:44 - _21 = Ne(move _20, const 0_u128); // scope 0 at $DIR/shifts.rs:+2:34: +2:44 - assert(!move _21, "attempt to shift right by `{}`, which would overflow", _19) -> [success: bb3, unwind: bb7]; // scope 0 at $DIR/shifts.rs:+2:34: +2:44 + _18 = Lt(_17, const 8_u128); // scope 0 at $DIR/shifts.rs:+2:34: +2:44 + assert(move _18, "attempt to shift right by `{}`, which would overflow", _17) -> [success: bb3, unwind: bb7]; // scope 0 at $DIR/shifts.rs:+2:34: +2:44 ```