mbutrovich commented on PR #11174: URL: https://github.com/apache/arrow-rs/pull/11174#issuecomment-6303792376
> Specifically I think the API is `unsafe` in the sense that it could cause memory violations using only safe APIs and thus I don't think we can merge it as is. Thanks @alamb. Sorry I didn't push harder on this. I asked for `unsafe fn` in the [first round](https://github.com/apache/arrow-rs/pull/11174#discussion_r4318098381), then accepted that the trait is private and that `copy_rows` already relies on caller-computed ranges, and I suggested `debug_assert!`s instead. The precedent I saw was `InProgressArray::copy_rows`. It is a safe trait method with no documented precondition, and its impls call `get_unchecked` on `offset..offset + len` behind a `SAFETY` comment that relies on the caller ([primitive.rs](https://github.com/apache/arrow-rs/blob/d58a0fce35b67836ac9b2e20a7efe47a584e0a86/arrow-select/src/coalesce/primitive.rs#L167-L169), [byte_view.rs](https://github.com/apache/arrow-rs/blob/d58a0fce35b67836ac9b2e20a7efe47a584e0a86/arrow-select/src/coalesce/byte_view.rs#L411-L412)). Should I file an issue to make it `unsafe` or have it check the range? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
