slice iter cleanup: replace checked_sub with saturating_sub #150262
Conversation
6fbdca3 to
0fda856
Compare
library/core/src/slice/iter.rs
Outdated
| Some(sum) => sum, | ||
| None => 0, | ||
| }; | ||
| let start = end.checked_sub(self.chunk_size).unwrap_or_default(); |
There was a problem hiding this comment.
| let start = end.checked_sub(self.chunk_size).unwrap_or_default(); | |
| let start = end.saturating_sub(self.chunk_size); |
wouldn't this be even better?
There was a problem hiding this comment.
Yes, that's correct, and I'm now using that.
0fda856 to
fd29ce0
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
|
This comment has been minimized.
This comment has been minimized.
fd29ce0 to
16b219a
Compare
|
Thanks! |
slice iter cleanup: replace checked_sub with saturating_sub Continuation of rust-lang#146436 r? ``@joboet``
Rollup of 6 pull requests Successful merges: - #150108 (Offload: Build offload as a single Step) - #150262 (slice iter cleanup: replace checked_sub with saturating_sub) - #150427 (add has_offload/needs-offload to the test infra) - #150458 (fix running stdlib doctests in Miri in CI) - #150477 (Fix enum variant suggestion consuming trailing parenthesis) - #150478 (Fix new bors config) r? `@ghost` `@rustbot` modify labels: rollup
| // Therefore the bounds check in split_at_mut guarantees the split point is inbounds. | ||
| let (nth, _) = unsafe { tail.split_at_mut(end - start) }; | ||
| self.v = head; | ||
| // SAFETY: The self.v contract ensures that any split_at_mut is valid. |
There was a problem hiding this comment.
Why did you change this line? It's not "cleanup" and it's also now wrong, because the contract is on self, not self.v.
(It makes it seem like you did this with AI without actually looking at the diff.)
There was a problem hiding this comment.
No AI was involved.
I'm not sure I agree that this is wrong (or that it would be correct with self instead of self.v). The call to split_at_mut only puts a requirement on self.v; if self.v.len() is correct, then any split_at_mut call is safe. This could even become split_at_mut_unchecked since it is always in bounds due to let end = self.v.len() - end;
I do think the safety comment for the second call to split_at_mut is suboptimal, since the receiver is not self.v, but rest, but this problem is pre-existing.
Rollup of 6 pull requests Successful merges: - rust-lang/rust#150108 (Offload: Build offload as a single Step) - rust-lang/rust#150262 (slice iter cleanup: replace checked_sub with saturating_sub) - rust-lang/rust#150427 (add has_offload/needs-offload to the test infra) - rust-lang/rust#150458 (fix running stdlib doctests in Miri in CI) - rust-lang/rust#150477 (Fix enum variant suggestion consuming trailing parenthesis) - rust-lang/rust#150478 (Fix new bors config) r? `@ghost` `@rustbot` modify labels: rollup
Continuation of #146436
r? @joboet