Merge pull request #51 from aldanor/no-error-chain

Remove error-chain dependency + various cleanups

Author: aldanor

CHANGELOG.md

@@ -14,7 +14,7 @@
   - `File::open(path, "r")` is now `File::open(path)`
   - `File::open(path, "r+")` is now `File::open_rw(path)`
   - `File::open(path, "w")` is now `File::create(path)`
-  - `File::open(path, "x" | "w-")` is now `File::create_exl(path)`
+  - `File::open(path, "x" | "w-")` is now `File::create_excl(path)`
   - `File::open(path, "a")` is now `File::append(path)`
 - Also added `File::open_as(path, mode)` which accepts the mode enum.
 - Rewritten `FileBuilder`: it no longer accepts userblock, driver etc;
@@ -33,6 +33,14 @@
 - It's no longer prohibited to set FCPL options when opening a file
   and not creating it -- it will simply be silently ignored (this
   simplifies the behavior and allows using a single file builder).
+- Added an explicit `hdf5_types::string::StringError` error type,
+  as a result `error-chain` dependency has been dropped.
+- `hdf5::Error` is now convertible from `ndarray::ShapeError`;
+  `hdf5::ResultExt` trait has been removed.
+  
+### Fixed
+
+- Replaced deprecated `std::mem::uninitialized` with `std::mem::MaybeUninit`.
 
 ## 0.5.2
 

hdf5-types/Cargo.toml

@@ -15,7 +15,3 @@ libc = "0.2"
 
 [dev-dependencies]
 quickcheck = "0.8"
-
-[dependencies.error-chain]
-version = "0.12"
-default-features = false

hdf5-types/src/error.rs

@@ -1,9 +1,6 @@
 #![recursion_limit = "1024"]
 #![cfg_attr(feature = "cargo-clippy", allow(clippy::transmute_bytes_to_str))]
 
-#[macro_use]
-extern crate error_chain;
-
 #[cfg(test)]
 #[macro_use]
 extern crate quickcheck;
@@ -12,17 +9,8 @@ mod array;
 mod h5type;
 mod string;
 
-mod errors {
-    error_chain! {
-        foreign_links {
-            Ascii(::ascii::AsAsciiStrError);
-        }
-    }
-}
-
 pub use self::array::{Array, VarLenArray};
-pub use self::errors::Error;
 pub use self::h5type::{
     CompoundField, CompoundType, EnumMember, EnumType, FloatSize, H5Type, IntSize, TypeDescriptor,
 };
-pub use self::string::{FixedAscii, FixedUnicode, VarLenAscii, VarLenUnicode};
+pub use self::string::{FixedAscii, FixedUnicode, StringError, VarLenAscii, VarLenUnicode};

hdf5-types/src/lib.rs

@@ -1,4 +1,5 @@
 use std::borrow::{Borrow, Cow};
+use std::error::Error as StdError;
 use std::fmt;
 use std::hash::{Hash, Hasher};
 use std::mem;
@@ -10,7 +11,38 @@ use std::str::{self, FromStr};
 use ascii::{AsAsciiStr, AsAsciiStrError, AsciiStr};
 
 use crate::array::Array;
-use crate::errors::Result;
+
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
+pub enum StringError {
+    InternalNull,
+    InsufficientCapacity,
+    AsciiError(AsAsciiStrError),
+    #[doc(hidden)]
+    __Incomplete,
+}
+
+impl From<AsAsciiStrError> for StringError {
+    fn from(err: AsAsciiStrError) -> Self {
+        StringError::AsciiError(err)
+    }
+}
+
+impl StdError for StringError {
+    fn description(&self) -> &str {
+        match self {
+            StringError::InternalNull => "variable length string with internal null",
+            StringError::InsufficientCapacity => "insufficient capacity for fixed sized string",
+            StringError::AsciiError(err) => err.description(),
+            _ => "",
+        }
+    }
+}
+
+impl fmt::Display for StringError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "string error: {}", self.description())
+    }
+}
 
 // ================================================================================
 
@@ -221,12 +253,10 @@ impl VarLenAscii {
         Self::from_bytes(bytes.as_ref())
     }
 
-    pub fn from_ascii<B: ?Sized + AsRef<[u8]>>(bytes: &B) -> Result<Self> {
+    pub fn from_ascii<B: ?Sized + AsRef<[u8]>>(bytes: &B) -> Result<Self, StringError> {
         let bytes = bytes.as_ref();
         if !bytes.iter().all(|&c| c != 0) {
-            bail!("Variable length ASCII string with internal null: {:?}", unsafe {
-                str::from_utf8_unchecked(bytes)
-            });
+            return Err(StringError::InternalNull);
         }
         let s = AsciiStr::from_ascii(bytes)?;
         unsafe { Ok(Self::from_bytes(s.as_bytes())) }
@@ -239,7 +269,7 @@ impl AsAsciiStr for VarLenAscii {
         AsciiStr::from_ascii_unchecked(self.as_bytes())
     }
 
-    fn as_ascii_str(&self) -> ::std::result::Result<&AsciiStr, AsAsciiStrError> {
+    fn as_ascii_str(&self) -> Result<&AsciiStr, AsAsciiStrError> {
         AsciiStr::from_ascii(self.as_bytes())
     }
 }
@@ -322,11 +352,14 @@ impl VarLenUnicode {
 }
 
 impl FromStr for VarLenUnicode {
-    type Err = crate::errors::Error;
+    type Err = StringError;
 
-    fn from_str(s: &str) -> Result<Self> {
-        ensure!(s.chars().all(|c| c != '\0'), "Variable length unicode string with internal null");
-        unsafe { Ok(Self::from_bytes(s.as_bytes())) }
+    fn from_str(s: &str) -> Result<Self, <Self as FromStr>::Err> {
+        if s.chars().all(|c| c != '\0') {
+            unsafe { Ok(Self::from_bytes(s.as_bytes())) }
+        } else {
+            Err(StringError::InternalNull)
+        }
     }
 }
 
@@ -342,9 +375,9 @@ impl<A: Array<Item = u8>> Clone for FixedAscii<A> {
     #[inline]
     fn clone(&self) -> Self {
         unsafe {
-            let mut buf: A = mem::uninitialized();
-            ptr::copy_nonoverlapping(self.buf.as_ptr(), buf.as_mut_ptr(), A::capacity());
-            FixedAscii { buf }
+            let mut buf = mem::MaybeUninit::<A>::uninit();
+            ptr::copy_nonoverlapping(self.buf.as_ptr(), buf.as_mut_ptr() as *mut _, A::capacity());
+            FixedAscii { buf: buf.assume_init() }
         }
     }
 }
@@ -403,12 +436,12 @@ impl<A: Array<Item = u8>> FixedAscii<A> {
         Self::from_bytes(bytes.as_ref())
     }
 
-    pub fn from_ascii<B: ?Sized + AsRef<[u8]>>(bytes: &B) -> Result<Self> {
+    pub fn from_ascii<B: ?Sized + AsRef<[u8]>>(bytes: &B) -> Result<Self, StringError> {
         let bytes = bytes.as_ref();
-        ensure!(bytes.len() <= A::capacity(), "Insufficient capacity for fixed-size ASCII string");
-        let s = AsciiStr::from_ascii(bytes).map_err(|_| {
-            format!("Invalid ASCII string: `{}`", unsafe { str::from_utf8_unchecked(bytes) })
-        })?;
+        if bytes.len() > A::capacity() {
+            return Err(StringError::InsufficientCapacity);
+        }
+        let s = AsciiStr::from_ascii(bytes)?;
         unsafe { Ok(Self::from_bytes(s.as_bytes())) }
     }
 }
@@ -419,7 +452,7 @@ impl<A: Array<Item = u8>> AsAsciiStr for FixedAscii<A> {
         AsciiStr::from_ascii_unchecked(self.as_bytes())
     }
 
-    fn as_ascii_str(&self) -> ::std::result::Result<&AsciiStr, AsAsciiStrError> {
+    fn as_ascii_str(&self) -> Result<&AsciiStr, AsAsciiStrError> {
         AsciiStr::from_ascii(self.as_bytes())
     }
 }
@@ -436,9 +469,9 @@ impl<A: Array<Item = u8>> Clone for FixedUnicode<A> {
     #[inline]
     fn clone(&self) -> Self {
         unsafe {
-            let mut buf: A = mem::uninitialized();
-            ptr::copy_nonoverlapping(self.buf.as_ptr(), buf.as_mut_ptr(), A::capacity());
-            FixedUnicode { buf }
+            let mut buf = mem::MaybeUninit::<A>::uninit();
+            ptr::copy_nonoverlapping(self.buf.as_ptr(), buf.as_mut_ptr() as *mut _, A::capacity());
+            FixedUnicode { buf: buf.assume_init() }
         }
     }
 }
@@ -507,14 +540,14 @@ impl<A> FromStr for FixedUnicode<A>
 where
     A: Array<Item = u8>,
 {
-    type Err = crate::errors::Error;
+    type Err = StringError;
 
-    fn from_str(s: &str) -> Result<Self> {
-        ensure!(
-            s.as_bytes().len() <= A::capacity(),
-            "Insufficient capacity for fixed-size unicode string"
-        );
-        unsafe { Ok(Self::from_bytes(s.as_bytes())) }
+    fn from_str(s: &str) -> Result<Self, <Self as FromStr>::Err> {
+        if s.as_bytes().len() <= A::capacity() {
+            unsafe { Ok(Self::from_bytes(s.as_bytes())) }
+        } else {
+            Err(StringError::InsufficientCapacity)
+        }
     }
 }
 

hdf5-types/src/string.rs

@@ -1,9 +1,11 @@
 use std::cell::RefCell;
+use std::error::Error as StdError;
 use std::fmt;
 use std::ops::Index;
 use std::ptr;
 
 use lazy_static::lazy_static;
+use ndarray::ShapeError;
 use num_integer::Integer;
 use num_traits::{Bounded, Zero};
 use parking_lot::Mutex;
@@ -52,6 +54,12 @@ impl ErrorFrame {
 #[doc(hidden)]
 pub struct SilenceErrors;
 
+impl Default for SilenceErrors {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 lazy_static! {
     static ref ERROR_HANDLER: Mutex<RefCell<usize>> = Mutex::default();
 }
@@ -63,7 +71,7 @@ extern "C" fn default_error_handler(estack: hid_t, _cdata: *mut c_void) -> herr_
 impl SilenceErrors {
     pub fn new() -> Self {
         Self::silence(true);
-        SilenceErrors
+        Self
     }
 
     fn silence(on: bool) {
@@ -140,13 +148,12 @@ impl ErrorStack {
                 match closure(*err_desc) {
                     Ok(frame) => {
                         data.stack.push(frame);
-                        0
                     }
                     Err(err) => {
                         data.err = Some(err);
-                        0
                     }
                 }
+                0
             }
         }
 
@@ -209,7 +216,7 @@ impl ErrorStack {
     }
 
     pub fn detail(&self) -> Option<String> {
-        self.top().and_then(|frame| frame.detail())
+        self.top().and_then(ErrorFrame::detail)
     }
 }
 
@@ -219,7 +226,7 @@ pub enum Error {
     /// An error occurred in the C API of the HDF5 library. Full error stack is captured.
     HDF5(ErrorStack),
     /// A user error occurred in the high-level Rust API (e.g., invalid user input).
-    Internal(String), // TODO: add error kinds etc, handle errors properly
+    Internal(String),
 }
 
 /// A type for results generated by HDF5-related functions where the `Err` type is
@@ -243,15 +250,18 @@ impl Error {
     }
 }
 
-impl<S> From<S> for Error
-where
-    S: Into<String>,
-{
-    fn from(desc: S) -> Self {
+impl From<&str> for Error {
+    fn from(desc: &str) -> Self {
         Error::Internal(desc.into())
     }
 }
 
+impl From<String> for Error {
+    fn from(desc: String) -> Self {
+        Error::Internal(desc)
+    }
+}
+
 impl fmt::Debug for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
@@ -267,24 +277,15 @@ impl fmt::Display for Error {
     }
 }
 
-impl ::std::error::Error for Error {
+impl StdError for Error {
     fn description(&self) -> &str {
         self.description()
     }
 }
 
-// TODO: this is a temporary shim
-#[doc(hidden)]
-pub trait ResultExt<T, E> {
-    fn str_err(self) -> Result<T>;
-}
-
-impl<T, E> ResultExt<T, E> for ::std::result::Result<T, E>
-where
-    E: ::std::error::Error,
-{
-    fn str_err(self) -> Result<T> {
-        self.map_err(|e| Error::from(e.description()))
+impl From<ShapeError> for Error {
+    fn from(err: ShapeError) -> Self {
+        format!("shape error: {}", err.description()).into()
     }
 }
 

src/error.rs

@@ -133,8 +133,8 @@ impl<'a> Reader<'a> {
 
             self.read_into_buf(vec.as_mut_ptr(), Some(&fspace), Some(&mspace))?;
 
-            let arr = ArrayD::from_shape_vec(reduced_shape, vec).str_err()?;
-            arr.into_dimensionality().str_err()
+            let arr = ArrayD::from_shape_vec(reduced_shape, vec)?;
+            Ok(arr.into_dimensionality()?)
         }
     }
 
@@ -149,8 +149,8 @@ impl<'a> Reader<'a> {
             ensure!(obj_ndim == ndim, "ndim mismatch: expected {}, got {}", ndim, obj_ndim);
         }
         let vec = self.read_raw()?;
-        let arr = ArrayD::from_shape_vec(shape, vec).str_err()?;
-        arr.into_dimensionality().str_err()
+        let arr = ArrayD::from_shape_vec(shape, vec)?;
+        Ok(arr.into_dimensionality()?)
     }
 
     /// Reads a dataset/attribute into a vector in memory order.
@@ -206,8 +206,8 @@ impl<'a> Reader<'a> {
     pub fn read_scalar<T: H5Type>(&self) -> Result<T> {
         let obj_ndim = self.obj.get_shape()?.ndim();
         ensure!(obj_ndim == 0, "ndim mismatch: expected scalar, got {}", obj_ndim);
-        let mut val: T = unsafe { mem::uninitialized() };
-        self.read_into_buf(&mut val as *mut _, None, None).map(|_| val)
+        let mut val = mem::MaybeUninit::<T>::uninit();
+        self.read_into_buf(val.as_mut_ptr(), None, None).map(|_| unsafe { val.assume_init() })
     }
 }
 
@@ -394,7 +394,7 @@ impl ObjectClass for Container {
     const VALID_TYPES: &'static [H5I_type_t] = &[H5I_DATASET, H5I_ATTR];
 
     fn from_handle(handle: Handle) -> Self {
-        Container(handle)
+        Self(handle)
     }
 
     fn handle(&self) -> &Handle {