Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use MaybeUninit #72

Closed
wants to merge 5 commits into from
Closed

Use MaybeUninit #72

wants to merge 5 commits into from

Conversation

pitdicker
Copy link
Contributor

Replace UnsafeCell<Option<T>> with UnsafeCell<MaybeUninit<T>>.

I included a terrible imitation of MaybeUninit based on Option, so there is no change in behavior right now. The real implementation that uses std::mem::MaybeUninit is behind a feature flag maybe_uninit and only usable with nightly.

I did find one complexity: OnceCell now has to implement Drop itself. Therefore it has to check is_initialized. into_inner now has to use the mem::replace trick to move the value field out of the OnceCell, and then free the cell without dropping (because initialized is not reset).

Conclusion: a bit messy. But I think it makes sense, as a preparation for the future. And it is a plus that is_initialized is now the single source of truth.

@pitdicker
Copy link
Contributor Author

It probably works better to swap the types to MaybeUninit<UnsafeCell<T>>. Then the decision whether to get a mutable reference or not can be made later. This helps making a nicer abstraction for a consume operation, and the maybe_uninit feature wouldn't depend on nightly. Have to think it through a bit more.

@pitdicker
Copy link
Contributor Author

pitdicker commented Nov 3, 2019

Late night ideas don't always work 😄.
To initialize a MaybeUninit<UnsafeCell<T>> you either have to do:

  • let slot = (&*self.value.as_ptr()).get() as *mut T;
    slot.write(value);
    which is UB, because it is creating a shared reference to an uninitialized type.
  • let slot = self.value.as_mut_ptr();
    slot.write(UnsafeCell::new(value));
    which is not possible, because we don't have a mutable reference to the MaybeUninit.

The mention of UnsafeCell in the description of MaybeUninit::as_ptr threw me off.

@matklad
Copy link
Owner

matklad commented Nov 11, 2020

Turns out we can't use MaybeUninit -- we'll need #[may_dangle] for dropcheck, and it only is avable in nightly.

@matklad matklad closed this Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants