Override Box::<[T]>::clone_from #72499
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
dtolnay
added
the
T-libs
src/liballoc/tests/boxed.rs
Outdated
| let mut copy = vec![Dummy { _data: 84 }; size + 1].into_boxed_slice(); | ||
| let copy_raw = copy.as_ptr() as usize; | ||
| copy.clone_from(&control); | ||
| assert_ne!(copy.as_ptr() as usize, copy_raw); |
There was a problem hiding this comment.
Seems like this would be flaky. The allocator can put these at the same address if the compiler decides to drop the old one before allocating the new one.
What sort of regression is this test hoping to catch?
There was a problem hiding this comment.
Fair enough, I made this test while I was trying to write them without UB or unsafe. It doesn't make much sense now that you point that out
There was a problem hiding this comment.
The other test on the other hand might not catch a regression for the same reason 🤔 but I can't think of a another way to test this. Basically it might give a false positive but never a false negative
|
@bors r+ |
|
📌 Commit dbf32e2 has been approved by |
bors
added
S-waiting-on-bors
Rollup of 9 pull requests Successful merges: - rust-lang#72299 (more `LocalDefId`s) - rust-lang#72368 (Resolve overflow behavior for RangeFrom) - rust-lang#72441 (Fix ICE with explicit late-bound lifetimes) - rust-lang#72499 (Override Box::<[T]>::clone_from) - rust-lang#72521 (Properly handle InlineAsmOperand::SymFn when collecting monomorphized items) - rust-lang#72540 (mir: adjust conditional in recursion limit check) - rust-lang#72563 (multiple Return terminators are possible) - rust-lang#72585 (Only capture tokens for items with outer attributes) - rust-lang#72607 (Eagerly lower asm sub-expressions to HIR even if there is an error) Failed merges: r? @ghost
Avoid dropping and reallocating when cloning to an existing box if the lengths are the same.
It would be nice if this could also be specialized for
Copybut I don't know how that works since it's not on stable. Will gladly look into it if it's deemed as a good idea.This is my first PR with code, hope I did everything right 😄