hzuo commented on code in PR #5554:
URL: https://github.com/apache/arrow-rs/pull/5554#discussion_r1540157385
##########
arrow-buffer/src/buffer/scalar.rs:
##########
@@ -63,9 +63,15 @@ impl<T: ArrowNativeType> ScalarBuffer<T> {
/// This method will panic if
///
/// * `offset` or `len` would result in overflow
- /// * `buffer` is not aligned to a multiple of `std::mem::size_of::<T>`
+ /// * `buffer` is not aligned to a multiple of `std::mem::align_of::<T>`
/// * `bytes` is not large enough for the requested slice
pub fn new(buffer: Buffer, offset: usize, len: usize) -> Self {
+ assert_eq!(
+ buffer.as_ptr().align_offset(std::mem::align_of::<T>()),
+ 0,
+ "buffer is not aligned"
+ );
Review Comment:
but a user could call `new` without going through the From conversion,
right? and the docs are specifically talking about `new`?
also looks like `From<Buffer> for ScalarBuffer<T>` doesn't go through `new`
so this wouldn't add any additional overhead
but not opinionated about this at all - happy to remove if you'd like
--
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]
