Skip to content

Commit 6d42707

Browse files
committed
Auto merge of #90346 - ferrocene:pa-short-circuit, r=oli-obk
Replace some operators in libcore with their short-circuiting equivalents In libcore there are a few occurrences of bitwise operators used in boolean expressions instead of their short-circuiting equivalents. This makes it harder to perform some kinds of source code analysis over libcore, for example [MC/DC] code coverage (a requirement in safety-critical environments). This PR aims to remove as many bitwise operators in boolean expressions from libcore as possible, without any performance regression and without other changes. This means not all bitwise operators are removed, only the ones that don't have any difference with their short-circuiting counterparts. This already simplifies achieving MC/DC coverage, and the other functions can be changed in future PRs. The PR is best reviewed commit-by-commit, and each commit has the resulting assembly in the message. ## Checked integer methods These methods recently switched to bitwise operators in PRs #89459 and #89351. I confirmed bitwise operators are needed in most of the functions, except these two: * `{integer}::checked_div` ([Godbolt link (nightly)](https://rust.godbolt.org/z/17efh5jPc)) * `{integer}::checked_rem` ([Godbolt link (nightly)](https://rust.godbolt.org/z/85qGWc94K)) `@tspiteri` already mentioned this was the case in #89459 (comment), but opted to also switch those two to bitwise operators for consistency. As that makes MC/DC analysis harder this PR proposes switching those two back to short-circuiting operators. ## `{unsigned_ints}::carrying_add` [Godbolt link (1.56.0)](https://rust.godbolt.org/z/vG9vx8x48) In this instance replacing the `|` with `||` produces the exact same assembly when optimizations are enabled, so switching to the short-circuiting operator shouldn't have any impact. ## `{unsigned_ints}::borrowing_sub` [Godbolt link (1.56.0)](https://rust.godbolt.org/z/asEfKaGE4) In this instance replacing the `|` with `||` produces the exact same assembly when optimizations are enabled, so switching to the short-circuiting operator shouldn't have any impact. ## String UTF-8 validation [Godbolt link (1.56.0)](https://rust.godbolt.org/z/a4rEbTvvx) In this instance replacing the `|` with `||` produces practically the same assembly, with the two operands for the "or" swapped: ```asm ; Old mov rax, qword ptr [rdi + rdx + 8] or rax, qword ptr [rdi + rdx] test rax, r9 je .LBB0_7 ; New mov rax, qword ptr [rdi + rdx] or rax, qword ptr [rdi + rdx + 8] test rax, r8 je .LBB0_7 ``` [MC/DC]: https://en.wikipedia.org/wiki/Modified_condition/decision_coverage
2 parents e99963c + 68a4460 commit 6d42707

File tree

3 files changed

+5
-7
lines changed

3 files changed

+5
-7
lines changed

library/core/src/num/int_macros.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -608,8 +608,7 @@ macro_rules! int_impl {
608608
without modifying the original"]
609609
#[inline]
610610
pub const fn checked_div(self, rhs: Self) -> Option<Self> {
611-
// Using `&` helps LLVM see that it is the same check made in division.
612-
if unlikely!(rhs == 0 || ((self == Self::MIN) & (rhs == -1))) {
611+
if unlikely!(rhs == 0 || ((self == Self::MIN) && (rhs == -1))) {
613612
None
614613
} else {
615614
// SAFETY: div by zero and by INT_MIN have been checked above
@@ -662,8 +661,7 @@ macro_rules! int_impl {
662661
without modifying the original"]
663662
#[inline]
664663
pub const fn checked_rem(self, rhs: Self) -> Option<Self> {
665-
// Using `&` helps LLVM see that it is the same check made in division.
666-
if unlikely!(rhs == 0 || ((self == Self::MIN) & (rhs == -1))) {
664+
if unlikely!(rhs == 0 || ((self == Self::MIN) && (rhs == -1))) {
667665
None
668666
} else {
669667
// SAFETY: div by zero and by INT_MIN have been checked above

library/core/src/num/uint_macros.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1525,7 +1525,7 @@ macro_rules! uint_impl {
15251525
// to generate optimal code for now, and LLVM doesn't have an equivalent intrinsic
15261526
let (a, b) = self.overflowing_add(rhs);
15271527
let (c, d) = a.overflowing_add(carry as $SelfT);
1528-
(c, b | d)
1528+
(c, b || d)
15291529
}
15301530

15311531
/// Calculates `self` + `rhs` with a signed `rhs`
@@ -1606,7 +1606,7 @@ macro_rules! uint_impl {
16061606
// to generate optimal code for now, and LLVM doesn't have an equivalent intrinsic
16071607
let (a, b) = self.overflowing_sub(rhs);
16081608
let (c, d) = a.overflowing_sub(borrow as $SelfT);
1609-
(c, b | d)
1609+
(c, b || d)
16101610
}
16111611

16121612
/// Computes the absolute difference between `self` and `other`.

library/core/src/str/validations.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ pub(super) fn run_utf8_validation(v: &[u8]) -> Result<(), Utf8Error> {
210210
// break if there is a nonascii byte
211211
let zu = contains_nonascii(*block);
212212
let zv = contains_nonascii(*block.offset(1));
213-
if zu | zv {
213+
if zu || zv {
214214
break;
215215
}
216216
}

0 commit comments

Comments
 (0)