Conversation
|
(rustbot has picked a reviewer for you, use r? to override) |
|
Is this true? Reading the code (and on godbolt) there does not appear to be any special-case for equality. |
From local testing it looks like it is, otherwise, I can add a check. pub const fn swap(&mut self, a: usize, b: usize) {
if a == b {
return;
}
...Well... We'd need to think if somebody might be relying on it's weird behavior, in case it has one.
There's optimization going on (I called |
|
Confirming, it is true by the underlying implementation of pub const unsafe fn swap<T>(x: *mut T, y: *mut T) {
// Give ourselves some scratch space to work with.
// We do not have to worry about drops: `MaybeUninit` does nothing when dropped.
let mut tmp = MaybeUninit::<T>::uninit();
// Perform the swap
// SAFETY: the caller must guarantee that `x` and `y` are
// valid for writes and properly aligned. `tmp` cannot be
// overlapping either `x` or `y` because `tmp` was just allocated
// on the stack as a separate allocated object.
unsafe {
copy_nonoverlapping(x, tmp.as_mut_ptr(), 1);
copy(y, x, 1); // `x` and `y` may overlap
copy_nonoverlapping(tmp.as_ptr(), y, 1);
}
} |
|
Ah, I misunderstood (I thought you meant the values at a and b, rather than the indexes themselves). Still, |
|
@clubby789 I believe the snippet I sent you above shows that it is guaranteed, specially this part: copy_nonoverlapping(x, tmp.as_mut_ptr(), 1);
copy(y, x, 1); // `x` and `y` may overlap
copy_nonoverlapping(tmp.as_ptr(), y, 1);You can interpret them as *tmp = *x; // No overlap
*x = *y; // Might overlap
*y = *tmp; // No overlapNow assume *tmp = *x; // (1)
*x = *x; // (2)
*x = *tmp; // (3)Assignment in *tmp = *x; // (1)
*x = *tmp; // (3)Which is guaranteed to work because |
|
This is not a no-op though - it's 3 moves (which can panic if OOB). |
|
Oh, I'm sorry! By Do you suggest I change the comment like so? -/// If `a` equals to `b`, this is a no-op.
+/// If `a` equals to `b`, no swaps are done. |
|
I don't think the implementation guarantees that no swaps are done. In debug mode we swap even with equal arguments: https://rust.godbolt.org/z/z174eW9eh I think in most cases this is fine, I wouldn't add a check to the swap code to skip the check as it's probably true that the vast majority of swaps have non-equal arguments and the extra check is a wasted computation. |
Add a note telling that no elements change when arguments are equal
5ebf984 to
30c61ee
Compare
| /// If `a` equals to `b`, it's guaranteed that elements won't change value. | ||
| /// |
There was a problem hiding this comment.
I edited the text accordingly, now it should make more sense?
|
@bors r+ rollup I guess this seems OK, though I'm not sure that it's very meaningful to add this to the docs. |
…cs, r=Mark-Simulacrum std: edit [T]::swap docs Add a note about what happens when index arguments are equal.
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#113005 (Don't call `query_normalize` when reporting similar impls) - rust-lang#113064 (std: edit [T]::swap docs) - rust-lang#113138 (Add release notes for 1.71.0) - rust-lang#113217 (resolve typerelative ctors to adt) - rust-lang#113254 (Use consistent formatting in Readme) - rust-lang#113482 (Migrate GUI colors test to original CSS color format) r? `@ghost` `@rustbot` modify labels: rollup
Add a note about what happens when index arguments are equal.