[incertia]

the rust-version is 1.85, and transmute_copy is const since 1.74.

we also move self into ManuallyDrop to prevent dropping self at the end of assume_init, which should be a bug in the previous version i think (in the case where dropping N::ArrayType<MaybeUninit<T>> does something crazy).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (09310c5) to head (d52656f).

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #239   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          562       564    +2     
=========================================
+ Hits           562       564    +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tarcieri]

There's already a complex story around just this mem::transmute_copy and whatever approach is being used to satisfy the compiler in this case needs an explanation for what it's doing and I don't think it should just get unceremoniously shoved into the existing text.

How about instead of using ManuallyDrop, what if you just did mem::forget(self) with its own separate comment like // SAFETY: ownership has been transferred to [return_var_name]. Consume the MaybeUninit array to satisfy const drop checking or thereabouts.

[newpavlov]

Maybe it's better to use pointer casting here, something like core::ptr::read((&raw const self).cast())?

[incertia]

i think the argument for the existing impl (for why we need to use transmute_copy) is enough.

functionally, there is only one reason we cannot upgrade this function from fn to const fn, and that is that self is dropped.

from my point of view, wrapping in ManuallyDrop finishes telling the story that we want

1. the compiler is not convinced that N::ArrayType<T> and N::ArrayType<MaybeUninit<T>> have the same size, so we cannot use core::mem::transmute
2. by using core::mem::transmute_copy, we interpret &N::ArrayType<MaybeUninit<T>> as &N::ArrayType<T> and fake dereference/create a copy to arrive at N::ArrayType<T>, but importantly this does not move out of the original ref
3. since the original ref used to be &self, self would be dropped at the end of the function. by wrapping this in ManuallyDrop, my interpretation is that the process of transmuting also takes the responsibility of drop.

changing this to mem::forget also works, but i think transmute with ManuallyDrop more accurately reflects that something else is taking ownership of the underlying memory.

i don't think i like using pointer casting here.

regardless maybe this warrants a comment cleanup once some decision has been reached on whether or not to use ManuallyDrop or forget

[incertia]

i've pushed an updated comment to better document the use of ManuallyDrop, but i can turn this into a mem::forget if that is the preference.

[tarcieri]

@incertia the thing I thought would be nice about splitting them up is being able to separate the documentation for what is effectively a workaround because the compiler can't prove !Drop for an inner type which is always [MaybeDrop<T>; N] but the compiler can't figure that out, rather than having one giant documentation comment for everything, even if it isn't quite as pure a semantic mapping to what's happening

[incertia]

sounds good, i'll adjust the impl and the doc

[tarcieri]

Is that a general comment? It doesn't resolve the const drop issue

[newpavlov]

Ah, I missed note in the OP. IIRC it should be impossible to impl Drop for MaybeUninit<MyType>, so it's not a bug. Technically, std may add a Drop impl, but I think we can safely disregard such scenario.

[tarcieri]

This is the problem that const fn introduces:

error[E0493]: destructor of `Array<MaybeUninit<T>, U>` cannot be evaluated at compile-time
   --> src/lib.rs:560:37
    |
560 |     pub const unsafe fn assume_init(self) -> Array<T, U> {
    |                                     ^^^^ the destructor for this type cannot be evaluated in constant functions

That happens even with core::ptr::read((&raw const self).cast()). I think it's having trouble reasoning about the generic inner type this method is effectively erasing, and can't figure out it will always be [MaybeUninit<T>; N].

That's why either ManuallyDrop or mem::forget are necessary.

Edit: as a general comment, it does seem a bit more straightforward

[newpavlov]

I see. Then I would prefer to use mem::forget(self); instead of ManuallyDrop.

[incertia]

can't figure out it will always be [MaybeUninit<T>; N].

i think there can be some dubious implementations of ArraySize where ArrayType<T> = Box<[T]> which would cause a buggy drop, although this violates the safety guidelines of ArraySize, which is why the compiler is unaware of it

[tarcieri]

i think there can be some dubious implementations of ArraySize where ArrayType = Box<[T]> which would cause a buggy drop, although this violates the safety guidelines of ArraySize, which is why the compiler is unaware of it

It's ultimately bounded on typenum::sealed::Sealed (by way of typenum::Unsigned), and therefore only hybrid-array can add impls of it due to orphan rules.

The compiler has all the information it needs to deduce there is a fixed set of impls since no other crate can add them, and that all of them are !Drop (because they're all [MaybeUninit<T>; N]). But that would probably be slow and have weird effects on inference.