Fixing UB in construction code

[bazhenov]

As stated in #16 miri complains about undefined behavior in eytzinger_walk(). As far as I can tell this is indeed UB because get_unchecked_mut() is UB when accessing out-of-bounds index (even without dereferencing). Out-of-bounds I interpret as "after length", not "after capacity".

The issue goes away if appropriate resize() call has been done on a Vec before writing elements to it. This agrees with the hypothesis that uninitialized vector is the issue.

I rewrite construction code using MaybeUninit, resize_with (which is zero cost in conjunction with MaybeUninit) and mem::transmute to mitigate this issue. mem::transmute may seems scary but it is the analog of set_len() in old code.

The alternative would be to fill the Vec with default values, but it does have performance impact.

[jonhoo]

Hmm, given that we only ever write to this value, wouldn't it be easier to just use

v.as_mut_ptr().offset(i).write(iter.next().unwrap())

?

[codecov-commenter]

Codecov Report

Merging #17 (7636bda) into main (ec4cbab) will increase coverage by 0.0%.
The diff coverage is 100.0%.

Files Changed	Coverage
src/lib.rs	100.0%

[bazhenov]

I'm somewhat skeptical about the contract between eytzinger_walk() and from_sorted_iter(). One need to remember how exactly those two functions need to behave to stay sound. My thought was to be more explicit with MaybeUninit to prevent possible problems in the future.

But in terms of simplicity, yes I agree – pointer arithmetic is much simpler. I've implemented your approach.

[jonhoo]

You're not wrong, but at the same time I worry that the code with MaybeUninit is also more complex, and so easier to accidentally get subtly wrong. We also now have miri to keep us honest, which helps!