Conversation
|
@zeenix Do you see these comments? UPDATE: Sorry - it turns out "pending" means pending MY approval to send review comments. |
|
BTW, why explicit |
|
Also, I see in some places |
|
|
|
You should use |
|
OK. Then everything good as it is. |
src/vec/mod.rs
Outdated
| v | ||
| } | ||
| // MaybeUninit::deref is non-const, so we just cast it through. | ||
| // Casting to internal value of MaybeUninit<T> is safe since it is transparent. |
There was a problem hiding this comment.
Casting is always safe. Explaining why it is safe to dereference should go into the SAFETY comment below.
There was a problem hiding this comment.
Also, I think src.as_ptr() followed by mem::forget(src) would be easier to understand.
There was a problem hiding this comment.
Also, I think
src.as_ptr()followed bymem::forget(src)would be easier to understand.
I somehow "feel" uncomfortable about that. First, wouldn't this make code more "brittle" to maintain? Someone later could put a call that could panic, between copy and mem::forget. Second, doesn't ManuallyDrop + copy makes intention more clear for optimization compiler, then copy + forget? Intention that this is actually "move", not "copy".
| let src_ptr: *const T = ptr::from_ref(&src).cast(); | ||
|
|
||
| // Cast from [MaybeUninit] to [T] is safe since it is transparent. | ||
| let dst_ptr: *mut T = v.buffer.buffer.as_mut_ptr().cast(); |
There was a problem hiding this comment.
Does this actually need a cast? Doesn't .as_mut_ptr() already return *mut T?
There was a problem hiding this comment.
No, it is MaybeUninit<T>.
Because
pub type Vec<T, const N: usize, LenT = usize> = VecInner<T, LenT, OwnedVecStorage<T, N>>;
pub type OwnedVecStorage<T, const N: usize> = VecStorageInner<[MaybeUninit<T>; N]>;
Co-authored-by: Markus Reiter <me@reitermark.us>
|
So, do you need something from my side to move this forward? |
4 participants
Implements #643