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

Change uninitialized to return an array of MaybeUninit #685

Closed
jturner314 opened this issue Aug 15, 2019 · 4 comments · Fixed by #803
Closed

Change uninitialized to return an array of MaybeUninit #685

jturner314 opened this issue Aug 15, 2019 · 4 comments · Fixed by #803
Milestone

Comments

@jturner314
Copy link
Member

The .uninitialized() method on ArrayBase has some issues. For example, Array1::<bool>::uninitialized(2) is undefined behavior.

Rust 1.36 added std::mem::MaybeUninit for safer handling of uninitialized data. We should replace the existing .uninitialized() method on ArrayBase with one that returns an array of MaybeUninit instances. We then need to add the necessary methods to cleanly work with arrays like this (e.g. an array-level equivalent of assume_init).

@bluss
Copy link
Member

bluss commented Aug 21, 2019

Internally I'd kind of just like a Zip with ndproducers that would give out raw pointers to the elements (so that we avoid references and iteration is sound). That way we can use uninitialized or similar correctly using some raw pointer code, at least.

@bluss
Copy link
Member

bluss commented Sep 1, 2019

I'm not sure the recipe in the title of this issue is what we should do - there might be better ways to do it, for example to provide raw pointer traversals

@bluss
Copy link
Member

bluss commented Apr 13, 2020

Can we explain why Array1::<bool>::uninitialized(2) is UB? What's the difference to Vec::<bool>::with_capacity(2)?

To me it looks like it can't be UB, on its own, but further method calls would be needed.

Maybe this comment can back me up rust-lang/rust-clippy#4483 (comment)

Edit: I have a tentative conclusion, Array1::<bool>::uninitialized(2) in itself is ok. However, if this array drops without being initialized, the &mut [bool] created in the method Vec::drop is not valid, and that's the reason this is a very hard API to work with (and it is not really panic safe, as was intended, even though it does not seem to have ill effects today. Meanwhile, I've even proposed a PR to rust to fix Vec::drop to not make a mutable slice in drop.)

@bluss
Copy link
Member

bluss commented Apr 14, 2020

Internal implementation of this is added in #797, that's a start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants