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

Make the PyArray::new method unsafe and document what can and cannot be done with its return value. #220

Merged
merged 2 commits into from
Nov 13, 2021
Merged

Conversation

adamreichold
Copy link
Member

Luckily, NumPy will always zero-initialize object pointers, c.f. https://github.com/numpy/numpy/blob/84e0707afa587e7655410561324ac36085db2b95/numpy/core/src/multiarray/ctors.c#L826-L835, so that even without invoking Self::zeros, the return values of Self::new will always be safe to drop.

However their elements will be invalid (null pointers are not valid instances of PyObject as Py<...> is built on NonNull<...>). Hence the method is marked unsafe as referencing its elements will invoke undefined behaviour. To actually initialise the elements, the new uget_raw method is provided (and used internally in from_iter, from_exact_iter, from_vec2 and from_vec3 where we currently invoke UB ourselves).

Fixes #217

Copy link
Member

@kngwyu kngwyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well written, thanks!
I just found a grammar error.

src/array.rs Outdated Show resolved Hide resolved
for (i, item) in iter.enumerate() {
*array.uget_mut([i]) = item;
array.uget_raw([i]).write(item);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

@kngwyu kngwyu merged commit 131bb5f into PyO3:main Nov 13, 2021
@adamreichold adamreichold deleted the pyarray-uninit branch November 13, 2021 07:35
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.

Fix signature of PyArray::new or better yet replace it by PyArray::uninit
2 participants