ARROW-3088: [Rust] Use internal Result<T> type instead of Result<T, ArrowError>

paddyhoran · andygrove · commit 94e8196ff925 · 2018-08-20T07:29:52.000-06:00
Updates internal code to use existing arrow `Result` type. Also, includes the following minor updates that do not deserve their own PR: - Removed `println!` statements that had been commented out but left in the code - Updated to use short-hand syntax where using identifiers that match the argument names when creating objects (i.e. `{field}` vs `{field: field}`) - Updated some `assert!` macros to more specific assert versions (`assert_eq!` and `assert_ne!`) to improve debug output Author: Paddy Horan <paddyhoran@hotmail.com> Closes apache#2448 from paddyhoran/ARROW-3088 and squashes the following commits: 0d923bb <Paddy Horan> Fixed lint issues. 21d8974 <Paddy Horan> Provide better debug info with more specific asserts 23f033a <Paddy Horan> Removed commented out `println!` calls 2caeb65 <Paddy Horan> Uses less verbose syntax for object creation 6d171f9 <Paddy Horan> Replaces standard `Result` with internal arrow `Result`
diff --git a/rust/src/array.rs b/rust/src/array.rs
@@ -106,7 +106,7 @@ struct RawPtrBox<T> {
 
 impl<T> RawPtrBox<T> {
     fn new(inner: *const T) -> Self {
-        Self { inner: inner }
+        Self { inner }
     }
 
     fn get(&self) -> *const T {
@@ -240,11 +240,11 @@ macro_rules! def_primitive_array {
 /// Constructs a `PrimitiveArray` from an array data reference.
 impl<T: ArrowPrimitiveType> From<ArrayDataRef> for PrimitiveArray<T> {
     fn from(data: ArrayDataRef) -> Self {
-        assert!(data.buffers().len() == 1);
+        assert_eq!(data.buffers().len(), 1);
         let raw_values = data.buffers()[0].raw_data();
         assert!(memory::is_aligned::<u8>(raw_values, mem::align_of::<T>()));
         Self {
-            data: data,
+            data,
             raw_values: RawPtrBox::new(raw_values as *const T),
         }
     }
@@ -321,8 +321,8 @@ impl ListArray {
 /// Constructs a `ListArray` from an array data reference.
 impl From<ArrayDataRef> for ListArray {
     fn from(data: ArrayDataRef) -> Self {
-        assert!(data.buffers().len() == 1);
-        assert!(data.child_data().len() == 1);
+        assert_eq!(data.buffers().len(), 1);
+        assert_eq!(data.child_data().len(), 1);
         let values = make_array(data.child_data()[0].clone());
         let raw_value_offsets = data.buffers()[0].raw_data();
         assert!(memory::is_aligned(
@@ -331,12 +331,15 @@ impl From<ArrayDataRef> for ListArray {
         ));
         let value_offsets = raw_value_offsets as *const i32;
         unsafe {
-            assert!(*value_offsets.offset(0) == 0);
-            assert!(*value_offsets.offset(data.len() as isize) == values.data().len() as i32);
+            assert_eq!(*value_offsets.offset(0), 0);
+            assert_eq!(
+                *value_offsets.offset(data.len() as isize),
+                values.data().len() as i32
+            );
         }
         Self {
             data: data.clone(),
-            values: values,
+            values,
             value_offsets: RawPtrBox::new(value_offsets),
         }
     }
@@ -410,7 +413,7 @@ impl BinaryArray {
 
 impl From<ArrayDataRef> for BinaryArray {
     fn from(data: ArrayDataRef) -> Self {
-        assert!(data.buffers().len() == 2);
+        assert_eq!(data.buffers().len(), 2);
         let raw_value_offsets = data.buffers()[0].raw_data();
         assert!(memory::is_aligned(
             raw_value_offsets,
@@ -478,10 +481,7 @@ impl From<ArrayDataRef> for StructArray {
         for cd in data.child_data() {
             boxed_fields.push(make_array(cd.clone()));
         }
-        Self {
-            data: data,
-            boxed_fields: boxed_fields,
-        }
+        Self { data, boxed_fields }
     }
 }
 
diff --git a/rust/src/array_data.rs b/rust/src/array_data.rs
@@ -156,7 +156,7 @@ pub struct ArrayDataBuilder {
 impl ArrayDataBuilder {
     pub fn new(data_type: DataType) -> Self {
         Self {
-            data_type: data_type,
+            data_type,
             len: 0,
             null_count: UNKNOWN_NULL_COUNT,
             null_bit_buffer: None,
diff --git a/rust/src/buffer.rs b/rust/src/buffer.rs
@@ -60,7 +60,7 @@ impl Buffer {
     /// Creates a buffer from an existing memory region (must already be byte-aligned)
     pub fn from_raw_parts(ptr: *const u8, len: usize) -> Self {
         assert!(memory::is_aligned(ptr, 64));
-        let buf_data = BufferData { ptr: ptr, len: len };
+        let buf_data = BufferData { ptr, len };
         Buffer {
             data: Arc::new(buf_data),
             offset: 0,
@@ -147,17 +147,17 @@ mod tests {
 
         // slice with same offset should still preserve equality
         let buf3 = buf1.slice(2);
-        assert!(buf1 != buf3);
+        assert_ne!(buf1, buf3);
         let buf4 = buf2.slice(2);
         assert_eq!(buf3, buf4);
 
         // unequal because of different elements
         buf2 = Buffer::from(&[0, 0, 2, 3, 4]);
-        assert!(buf1 != buf2);
+        assert_ne!(buf1, buf2);
 
         // unequal because of different length
         buf2 = Buffer::from(&[0, 1, 2, 3]);
-        assert!(buf1 != buf2);
+        assert_ne!(buf1, buf2);
     }
 
     #[test]
diff --git a/rust/src/datatypes.rs b/rust/src/datatypes.rs
@@ -19,7 +19,7 @@ use std::fmt;
 use std::mem::size_of;
 use std::slice::from_raw_parts;
 
-use error::ArrowError;
+use error::{ArrowError, Result};
 use serde_json::Value;
 
 /// Arrow data type
@@ -92,8 +92,7 @@ where
 
 impl DataType {
     /// Parse a data type from a JSON representation
-    fn from(json: &Value) -> Result<DataType, ArrowError> {
-        //println!("DataType::from({:?})", json);
+    fn from(json: &Value) -> Result<DataType> {
         match *json {
             Value::Object(ref map) => match map.get("name") {
                 Some(s) if s == "bool" => Ok(DataType::Boolean),
@@ -148,7 +147,7 @@ impl DataType {
                         let fields = fields_array
                             .iter()
                             .map(|f| Field::from(f))
-                            .collect::<Result<Vec<Field>, ArrowError>>();
+                            .collect::<Result<Vec<Field>>>();
                         Ok(DataType::Struct(fields?))
                     }
                     _ => Err(ArrowError::ParseError("empty type".to_string())),
@@ -193,8 +192,8 @@ impl Field {
     pub fn new(name: &str, data_type: DataType, nullable: bool) -> Self {
         Field {
             name: name.to_string(),
-            data_type: data_type,
-            nullable: nullable,
+            data_type,
+            nullable,
         }
     }
 
@@ -211,8 +210,7 @@ impl Field {
     }
 
     /// Parse a field definition from a JSON representation
-    pub fn from(json: &Value) -> Result<Self, ArrowError> {
-        //println!("Field::from({:?}", json);
+    pub fn from(json: &Value) -> Result<Self> {
         match *json {
             Value::Object(ref map) => {
                 let name = match map.get("name") {
@@ -284,7 +282,7 @@ impl Schema {
     }
 
     pub fn new(columns: Vec<Field>) -> Self {
-        Schema { columns: columns }
+        Schema { columns }
     }
 
     pub fn columns(&self) -> &Vec<Field> {
diff --git a/rust/src/memory.rs b/rust/src/memory.rs
@@ -18,7 +18,7 @@
 use libc;
 use std::mem;
 
-use super::error::ArrowError;
+use super::error::{ArrowError, Result};
 
 const ALIGNMENT: usize = 64;
 
@@ -30,7 +30,7 @@ extern "C" {
 }
 
 #[cfg(windows)]
-pub fn allocate_aligned(size: i64) -> Result<*mut u8, ArrowError> {
+pub fn allocate_aligned(size: i64) -> Result<*mut u8> {
     let page = unsafe { _aligned_malloc(size as libc::size_t, ALIGNMENT as libc::size_t) };
     match page {
         0 => Err(ArrowError::MemoryError(
@@ -41,7 +41,7 @@ pub fn allocate_aligned(size: i64) -> Result<*mut u8, ArrowError> {
 }
 
 #[cfg(not(windows))]
-pub fn allocate_aligned(size: i64) -> Result<*mut u8, ArrowError> {
+pub fn allocate_aligned(size: i64) -> Result<*mut u8> {
     unsafe {
         let mut page: *mut libc::c_void = mem::uninitialized();
         let result = libc::posix_memalign(&mut page, ALIGNMENT, size as usize);
diff --git a/rust/src/memory_pool.rs b/rust/src/memory_pool.rs
@@ -19,24 +19,20 @@ use libc;
 use std::cmp;
 use std::mem;
 
-use super::error::ArrowError;
+use super::error::Result;
 use super::memory::{allocate_aligned, free_aligned};
 
 /// Memory pool for allocating memory. It's also responsible for tracking memory usage.
 pub trait MemoryPool {
     /// Allocate memory.
     /// The implementation should ensures that allocated memory is aligned.
-    fn allocate(&self, size: usize) -> Result<*mut u8, ArrowError>;
+    fn allocate(&self, size: usize) -> Result<*mut u8>;
 
     /// Reallocate memory.
     /// If the implementation doesn't support reallocating aligned memory, it allocates new memory
     /// and copied old memory to it.
-    fn reallocate(
-        &self,
-        old_size: usize,
-        new_size: usize,
-        pointer: *const u8,
-    ) -> Result<*const u8, ArrowError>;
+    fn reallocate(&self, old_size: usize, new_size: usize, pointer: *const u8)
+        -> Result<*const u8>;
 
     /// Free memory.
     fn free(&self, ptr: *const u8);
@@ -47,7 +43,7 @@ pub trait MemoryPool {
 struct LibcMemoryPool;
 
 impl MemoryPool for LibcMemoryPool {
-    fn allocate(&self, size: usize) -> Result<*mut u8, ArrowError> {
+    fn allocate(&self, size: usize) -> Result<*mut u8> {
         allocate_aligned(size as i64)
     }
 
@@ -56,7 +52,7 @@ impl MemoryPool for LibcMemoryPool {
         old_size: usize,
         new_size: usize,
         pointer: *const u8,
-    ) -> Result<*const u8, ArrowError> {
+    ) -> Result<*const u8> {
         unsafe {
             let old_src = mem::transmute::<*const u8, *mut libc::c_void>(pointer);
             let result = self.allocate(new_size)?;
@@ -75,6 +71,7 @@ impl MemoryPool for LibcMemoryPool {
 #[cfg(test)]
 mod tests {
     use super::*;
+
     const ALIGNMENT: usize = 64;
 
     #[test]
