Remove FromFn trait
#35
Conversation
src/from_fn.rs
Outdated
| let mut array: [MaybeUninit<T>; N] = unsafe { MaybeUninit::uninit().assume_init() }; | ||
| try_from_fn_erased(&mut array, cb)?; | ||
| let mut array: Array<MaybeUninit<T>, U> = unsafe { MaybeUninit::uninit().assume_init() }; | ||
| try_from_fn_erased(array.0.as_mut(), f)?; |
There was a problem hiding this comment.
Aah joy, this again:
error: this call for this type may be undefined behavior
--> src/from_fn.rs:34:60
|
34 | let mut array: Array<MaybeUninit<T>, U> = unsafe { MaybeUninit::uninit().assume_init() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
It would probably be good to leverage Array::uninit to construct this, but that would currently require adding the ArraySize<ArrayType = [T; N]> bound, which gets viral quickly and would impact several other APIs which call this, so I was trying to avoid it.
There was a problem hiding this comment.
Is it possible add this bound on the method itself?
There was a problem hiding this comment.
If the bound exists on the method then it needs to be added to everything which calls the method, e.g. the FromIterator impl.
I think I'd like to try a PR which finds a sane way to remove the bound from Array::uninit so we can use that, which also avoids triggering this clippy lint.
There was a problem hiding this comment.
I wasn't able to come up with a solution that satisfies the clippy lint, unfortunately.
7deb1a3 removes the bounds on ArrayType and adds allow(clippy::uninit_assumed_init).
It's not ideal, but allows Array::try_from_fn to use Array::uninit and Array::assume_init, so at least we only need to get things right in those two places.
src/lib.rs
Outdated
| @@ -416,7 +396,7 @@ where | |||
| /// state. | |||
| pub unsafe fn assume_init(self) -> Array<T, U> { | |||
| // TODO(tarcieri): use `MaybeUninit::array_assume_init` when stable | |||
| Array(ptr::read(self.0.as_ptr().cast())) | |||
| mem::transmute_copy(&self) | |||
There was a problem hiding this comment.
I'm a little unclear if this is actually correct and it could probably use a better explanatory comment
|
Gonna roll back to what @newpavlov approved and PR the other changes separately |
This trait was a sort of crutch to thunk to constructing the inner array type, but isn't strictly necessary and just adds implementation complexity. Instead, we can have `try_from_fn` use the `AsMut` bound on the inner type to provide access to the inner array, and compose `Array::from_fn` in terms of `Array::try_from_fn`. The result is less complexity and API surface, without subtracting any functionality.
tarcieri
force-pushed
the
remove-fromfn-trait
branch
from
ad3736c
to
7aedc59
Compare
This trait was a sort of crutch to thunk to constructing the inner array type, but isn't strictly necessary and just adds implementation complexity.
Instead, we can have
try_from_fnuse theAsMutbound on the inner type to provide access to the inner array, and composeArray::from_fnin terms ofArray::try_from_fn.The result is less complexity and API surface, without subtracting any functionality.