You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/12/23 06:09:02 UTC
[GitHub] [arrow] jorgecarleitao commented on a change in pull request #8973: ARROW-10989: [Rust] Iterate primitive buffers by slice
jorgecarleitao commented on a change in pull request #8973:
URL: https://github.com/apache/arrow/pull/8973#discussion_r547687817
##########
File path: rust/arrow/src/array/array_primitive.rs
##########
@@ -63,17 +63,28 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
self.data.is_empty()
}
- /// Returns a raw pointer to the values of this array.
- pub fn raw_values(&self) -> *const T::Native {
- unsafe { self.raw_values.get().add(self.data.offset()) }
- }
-
/// Returns a slice for the given offset and length
///
/// Note this doesn't do any bound checking, for performance reason.
- pub fn value_slice(&self, offset: usize, len: usize) -> &[T::Native] {
- let raw =
- unsafe { std::slice::from_raw_parts(self.raw_values().add(offset), len) };
+ /// # Safety
+ /// caller must ensure that the passed in offset + len are less than the array len()
+ #[deprecated(note = "Please use values() instead")]
+ pub unsafe fn value_slice(&self, offset: usize, len: usize) -> &[T::Native] {
+ let raw = std::slice::from_raw_parts(
+ self.raw_values.get().add(self.data.offset()).add(offset),
+ len,
+ );
+ &raw[..]
+ }
+
+ /// Returns a slice of the values of this array
+ pub fn values(&self) -> &[T::Native] {
Review comment:
`inline`?
##########
File path: rust/arrow/src/array/array_primitive.rs
##########
@@ -63,17 +63,28 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
self.data.is_empty()
}
- /// Returns a raw pointer to the values of this array.
- pub fn raw_values(&self) -> *const T::Native {
- unsafe { self.raw_values.get().add(self.data.offset()) }
- }
-
/// Returns a slice for the given offset and length
///
/// Note this doesn't do any bound checking, for performance reason.
- pub fn value_slice(&self, offset: usize, len: usize) -> &[T::Native] {
- let raw =
- unsafe { std::slice::from_raw_parts(self.raw_values().add(offset), len) };
+ /// # Safety
+ /// caller must ensure that the passed in offset + len are less than the array len()
+ #[deprecated(note = "Please use values() instead")]
+ pub unsafe fn value_slice(&self, offset: usize, len: usize) -> &[T::Native] {
+ let raw = std::slice::from_raw_parts(
+ self.raw_values.get().add(self.data.offset()).add(offset),
+ len,
+ );
+ &raw[..]
+ }
+
+ /// Returns a slice of the values of this array
+ pub fn values(&self) -> &[T::Native] {
+ let raw = unsafe {
Review comment:
IMO we should justify that this is safe because `from(ArrayDataRef)` asserts that the pointer is aligned with T, etc, as we describe in the `README`.
##########
File path: rust/arrow/src/array/array_primitive.rs
##########
@@ -63,17 +63,28 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
self.data.is_empty()
}
- /// Returns a raw pointer to the values of this array.
- pub fn raw_values(&self) -> *const T::Native {
- unsafe { self.raw_values.get().add(self.data.offset()) }
- }
-
/// Returns a slice for the given offset and length
///
/// Note this doesn't do any bound checking, for performance reason.
- pub fn value_slice(&self, offset: usize, len: usize) -> &[T::Native] {
- let raw =
- unsafe { std::slice::from_raw_parts(self.raw_values().add(offset), len) };
+ /// # Safety
+ /// caller must ensure that the passed in offset + len are less than the array len()
+ #[deprecated(note = "Please use values() instead")]
+ pub unsafe fn value_slice(&self, offset: usize, len: usize) -> &[T::Native] {
+ let raw = std::slice::from_raw_parts(
+ self.raw_values.get().add(self.data.offset()).add(offset),
+ len,
+ );
+ &raw[..]
+ }
+
+ /// Returns a slice of the values of this array
+ pub fn values(&self) -> &[T::Native] {
+ let raw = unsafe {
+ std::slice::from_raw_parts(
+ self.raw_values.get().add(self.data.offset()),
+ self.len(),
+ )
+ };
&raw[..]
Review comment:
Is this needed here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org