Skip to content

Conversation

@adamreichold
Copy link
Member

No description provided.

@adamreichold adamreichold requested a review from mejrs September 16, 2022 14:38
@adamreichold adamreichold linked an issue Sep 16, 2022 that may be closed by this pull request
@adamreichold adamreichold force-pushed the fix-unsound-box-aliasing branch from aaf686d to 7e3bac4 Compare September 16, 2022 14:44
@adamreichold adamreichold changed the title Fix unsound alasing into Box<[T]> when converting them into NumPy arrays Fix unsound aliasing into Box<[T]> when converting them into NumPy arrays Sep 16, 2022
@adamreichold adamreichold force-pushed the fix-unsound-box-aliasing branch from 7e3bac4 to b1c1e4f Compare September 16, 2022 14:46
@adamreichold
Copy link
Member Author

adamreichold commented Sep 16, 2022

I would like to ignore the coverage failure here as we do have a doctest for the offending method, it is just that doctests are currently not supported by cargo-llvm-cov and thereby not counted against our coverage. Duplicating the test into a unit or integration test just to increase the patch coverage seems like a maintainability loss to me.

Copy link
Member

@mejrs mejrs left a comment

Choose a reason for hiding this comment

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

Thanks, you're quick 👍 We're still taking pointers and then moving vecs/arrays, but that should be fine, because those aren't noalias. :)

@adamreichold
Copy link
Member Author

adamreichold commented Sep 16, 2022

We're still taking pointers and then moving vecs/arrays, but that should be fine, because those aren't noalias. :)

I considered doing the same change for Vec and Array for consistency, but the problem is that the pointer owning the allocating is not necessarily equals the pointer to the first element for arrays, so deriving the pointer from PySliceContainer does not work for Array (which is backed by Vec) that easily. Hence, I decided for the minimum required change.

@adamreichold adamreichold merged commit c6508b5 into release-0.17 Sep 16, 2022
@adamreichold adamreichold deleted the fix-unsound-box-aliasing branch September 16, 2022 17:44
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.

PySliceContainer is unsound

4 participants