Remove UB in the heap_string function

[rylev]

After some discussion it was concluded that there is UB in the implementation of heap_string. This removes that UB.

In particular, std::slice::from_raw_parts_mut requires that the pointer points to initialized member which we do not get from heap_alloc. The solution to initializing uninitialized memory is to use std::ptr::write which does not read or drop values it writes to (and is thus documented specifically as being appropriate for writing to uninitialized memory).

cc @Lokathor

[rylev]

@kennykerr @Lokathor thanks for the tip on using copy_nonoverlapping. I'm using the method copy_from_nonoverlapping instead. I also added a check to make sure the allocation is properly aligned since it's also UB if the source or destination are not aligned. This might incur a very small runtime cost for now but align_offset should become const soon and then this check will be optimized away.

[Lokathor]

You could probably lower that alignment assert to a debug_assert, since if the allocator gives you a badly aligned pointer then a million other places will all have similar problems anyway. That much of the allocator you usually have to just trust.

Though now that you mention it, checking for a non-null buffer pointer is probably a good idea. The allocation cam fail, and in that case the pointer would be null.

[rylev]

@Lokathor We already check for null inside of heap_alloc so heap_alloc is guaranteed to return a non-null value. It's also guaranteed to return a pointer that is aligned to either 8 or 16 byte offsets. The issue is that T is not guaranteed to fall within that alignment (it could be 32 byte aligned for example). It would be preferable if we could put some constraint on T such that it was guaranteed that T is properly aligned on 8 bit or 16 byte offsets, but I don't believe that's possible.

[kennykerr]

This is fine for now - we may eventually replace the generic function with a pair of non-generic functions since this almost seems like more trouble than it's worth.

[rylev]

@kennykerr moving to two functions would only really allow removing the trait constraints on the argument type and the alignment check which might not be worth the duplicated code, but we can see 😊