Skip to content

Commit

Permalink
Use Result for fallible Vec*::from_* calls
Browse files Browse the repository at this point in the history
The various `Vec*` and `Vector*` constructor functions for `from_*_slice`
were previously returning `None` if the input slice was not the correct
length. This is not idiomatic Rust, and the functions should instead
return a `Result` with a reasonable error message in such cases instead.
`Result`s are also convertible into `Option` easily.
  • Loading branch information
bitwizeshift committed Sep 28, 2024
1 parent edbc542 commit 13d87e4
Show file tree
Hide file tree
Showing 10 changed files with 138 additions and 148 deletions.
5 changes: 5 additions & 0 deletions alloy/src/math/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ mod vec2;
mod vec2i;
mod vec2u;

pub(crate) mod errors;

Check failure on line 50 in alloy/src/math/vec.rs

View workflow job for this annotation

GitHub Actions / Verify build integrity / Build and Test / Build and Test (ubuntu-latest)

file not found for module `errors`

#[doc(inline)]
pub use errors::VecError;

Check failure on line 53 in alloy/src/math/vec.rs

View workflow job for this annotation

GitHub Actions / Verify build integrity / Build and Test / Build and Test (ubuntu-latest)

unresolved import `errors::VecError`

#[doc(inline)]
pub use vec2::*;
#[doc(inline)]
Expand Down
32 changes: 15 additions & 17 deletions alloy/src/math/vec/vec2.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use super::errors::VecError;

Check failure on line 1 in alloy/src/math/vec/vec2.rs

View workflow job for this annotation

GitHub Actions / Verify build integrity / Build and Test / Build and Test (ubuntu-latest)

unresolved import `super::errors::VecError`
use crate::math::vec::{Vec2i, Vec2u};

use crate::cmp::{AlmostEq, Near};
Expand Down Expand Up @@ -99,7 +100,7 @@ impl Vec2 {

/// Forms a reference to a [`Vec2`] from a slice of [`f32`].
///
/// This requires that `slice.len() == 2`, otherwise this returns [`None`].
/// This requires that `slice.len() == 2`, otherwise this returns [`VecError`].
///
/// # Parameters
///
Expand Down Expand Up @@ -128,23 +129,22 @@ impl Vec2 {
///
/// let vec = Vec2::from_slice(slice);
///
/// assert_eq!(vec, None);
/// assert!(vec.is_err());
/// ```
#[must_use]
pub const fn from_slice(slice: &[f32]) -> Option<&Self> {
pub const fn from_slice(slice: &[f32]) -> Result<&Self, VecError> {
if slice.len() == 2 {
// SAFETY: Vec2 is transparent, and implemented directly in terms of a
// slice of f32s. The representation is the same, and thus valid.
// This is implemented symmetrically to `OsStr`.
Some(unsafe { Self::from_slice_unchecked(slice) })
Ok(unsafe { Self::from_slice_unchecked(slice) })
} else {
None
Err(VecError::new(2, slice.len()))
}
}

/// Forms a mutable reference to a [`Vec2`] from a mutable slice of [`f32`].
///
/// This requires that `slice.len() == 2`, otherwise this returns [`None`].
/// This requires that `slice.len() == 2`, otherwise this returns [`VecError`].
///
/// # Parameters
///
Expand All @@ -171,17 +171,16 @@ impl Vec2 {
///
/// let vec = Vec2::from_mut_slice(slice);
///
/// assert_eq!(vec, None);
/// assert!(vec.is_err());
/// ```
#[must_use]
pub fn from_mut_slice(slice: &mut [f32]) -> Option<&mut Self> {
pub fn from_mut_slice(slice: &mut [f32]) -> Result<&mut Self, VecError> {
if slice.len() == 2 {
// SAFETY: Vec2 is transparent, and implemented directly in terms of a
// slice of f32s. The representation is the same, and thus valid.
// This is implemented symmetrically to `OsStr`.
Some(unsafe { Self::from_mut_slice_unchecked(slice) })
Ok(unsafe { Self::from_mut_slice_unchecked(slice) })
} else {
None
Err(VecError::new(2, slice.len()))
}
}

Expand Down Expand Up @@ -883,17 +882,16 @@ impl Vector2 {

/// Constructs this vector from a slice of floats.
///
/// This will return [`None`] if `slice.len()` is not equal to 2.
/// This will return [`VecError`] if `slice.len()` is not equal to 2.
///
/// # Parameters
///
/// * `slice` - the slice to read from
#[must_use]
pub const fn from_slice(slice: &[f32]) -> Option<Self> {
pub const fn from_slice(slice: &[f32]) -> Result<Self, VecError> {
if slice.len() != 2 {
None
Err(VecError::new(2, slice.len()))
} else {
Some(Self {
Ok(Self {
x: slice[0],
y: slice[1],
})
Expand Down
33 changes: 16 additions & 17 deletions alloy/src/math/vec/vec2i.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use super::errors::VecError;

Check failure on line 1 in alloy/src/math/vec/vec2i.rs

View workflow job for this annotation

GitHub Actions / Verify build integrity / Build and Test / Build and Test (ubuntu-latest)

unresolved import `super::errors::VecError`
use crate::ops::Dot;

use std::borrow::{Borrow, BorrowMut};
use std::fmt;
use std::ops::{
Expand Down Expand Up @@ -91,7 +93,7 @@ impl Vec2i {

/// Forms a reference to a [`Vec2`] from a slice of [`i32`].
///
/// This requires that `slice.len() == 2`, otherwise this returns [`None`].
/// This requires that `slice.len() == 2`, otherwise this returns [`VecError`].
///
/// # Parameters
///
Expand Down Expand Up @@ -120,23 +122,22 @@ impl Vec2i {
///
/// let vec = Vec2i::from_slice(slice);
///
/// assert_eq!(vec, None);
/// assert!(vec.is_err());
/// ```
#[must_use]
pub const fn from_slice(slice: &[i32]) -> Option<&Self> {
pub const fn from_slice(slice: &[i32]) -> Result<&Self, VecError> {
if slice.len() == 2 {
// SAFETY: Vec2 is transparent, and implemented directly in terms of a
// slice of i32s. The representation is the same, and thus valid.
// This is implemented symmetrically to `OsStr`.
Some(unsafe { Self::from_slice_unchecked(slice) })
Ok(unsafe { Self::from_slice_unchecked(slice) })
} else {
None
Err(VecError::new(2, slice.len()))
}
}

/// Forms a mutable reference to a [`Vec2`] from a mutable slice of [`i32`].
///
/// This requires that `slice.len() == 2`, otherwise this returns [`None`].
/// This requires that `slice.len() == 2`, otherwise this returns [`VecError`].
///
/// # Parameters
///
Expand All @@ -163,17 +164,16 @@ impl Vec2i {
///
/// let vec = Vec2i::from_mut_slice(slice);
///
/// assert_eq!(vec, None);
/// assert!(vec.is_err());
/// ```
#[must_use]
pub fn from_mut_slice(slice: &mut [i32]) -> Option<&mut Self> {
pub fn from_mut_slice(slice: &mut [i32]) -> Result<&mut Self, VecError> {
if slice.len() == 2 {
// SAFETY: Vec2 is transparent, and implemented directly in terms of a
// slice of i32s. The representation is the same, and thus valid.
// This is implemented symmetrically to `OsStr`.
Some(unsafe { Self::from_mut_slice_unchecked(slice) })
Ok(unsafe { Self::from_mut_slice_unchecked(slice) })
} else {
None
Err(VecError::new(2, slice.len()))
}
}

Expand Down Expand Up @@ -692,17 +692,16 @@ impl Vector2i {

/// Constructs this vector from a slice of floats.
///
/// This will return [`None`] if `slice.len()` is not equal to 2.
/// This will return [`VecError`] if `slice.len()` is not equal to 2.
///
/// # Parameters
///
/// * `slice` - the slice to read from
#[must_use]
pub const fn from_slice(slice: &[i32]) -> Option<Self> {
pub const fn from_slice(slice: &[i32]) -> Result<Self, VecError> {
if slice.len() != 2 {
None
Err(VecError::new(2, slice.len()))
} else {
Some(Self {
Ok(Self {
x: slice[0],
y: slice[1],
})
Expand Down
33 changes: 16 additions & 17 deletions alloy/src/math/vec/vec2u.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use super::errors::VecError;

Check failure on line 1 in alloy/src/math/vec/vec2u.rs

View workflow job for this annotation

GitHub Actions / Verify build integrity / Build and Test / Build and Test (ubuntu-latest)

unresolved import `super::errors::VecError`
use crate::ops::Dot;

use std::borrow::{Borrow, BorrowMut};
use std::fmt;
use std::ops::{
Expand Down Expand Up @@ -91,7 +93,7 @@ impl Vec2u {

/// Forms a reference to a [`Vec2`] from a slice of [`u32`].
///
/// This requires that `slice.len() == 2`, otherwise this returns [`None`].
/// This requires that `slice.len() == 2`, otherwise this returns [`VecError`].
///
/// # Parameters
///
Expand Down Expand Up @@ -120,23 +122,22 @@ impl Vec2u {
///
/// let vec = Vec2u::from_slice(slice);
///
/// assert_eq!(vec, None);
/// assert!(vec.is_err());
/// ```
#[must_use]
pub const fn from_slice(slice: &[u32]) -> Option<&Self> {
pub const fn from_slice(slice: &[u32]) -> Result<&Self, VecError> {
if slice.len() == 2 {
// SAFETY: Vec2 is transparent, and implemented directly in terms of a
// slice of u32s. The representation is the same, and thus valid.
// This is implemented symmetrically to `OsStr`.
Some(unsafe { Self::from_slice_unchecked(slice) })
Ok(unsafe { Self::from_slice_unchecked(slice) })
} else {
None
Err(VecError::new(2, slice.len()))
}
}

/// Forms a mutable reference to a [`Vec2`] from a mutable slice of [`u32`].
///
/// This requires that `slice.len() == 2`, otherwise this returns [`None`].
/// This requires that `slice.len() == 2`, otherwise this returns [`VecError`].
///
/// # Parameters
///
Expand All @@ -163,17 +164,16 @@ impl Vec2u {
///
/// let vec = Vec2u::from_mut_slice(slice);
///
/// assert_eq!(vec, None);
/// assert!(vec.is_err());
/// ```
#[must_use]
pub fn from_mut_slice(slice: &mut [u32]) -> Option<&mut Self> {
pub fn from_mut_slice(slice: &mut [u32]) -> Result<&mut Self, VecError> {
if slice.len() == 2 {
// SAFETY: Vec2 is transparent, and implemented directly in terms of a
// slice of u32s. The representation is the same, and thus valid.
// This is implemented symmetrically to `OsStr`.
Some(unsafe { Self::from_mut_slice_unchecked(slice) })
Ok(unsafe { Self::from_mut_slice_unchecked(slice) })
} else {
None
Err(VecError::new(2, slice.len()))
}
}

Expand Down Expand Up @@ -662,17 +662,16 @@ impl Vector2u {

/// Constructs this vector from a slice of floats.
///
/// This will return [`None`] if `slice.len()` is not equal to 2.
/// This will return [`VecError`] if `slice.len()` is not equal to 2.
///
/// # Parameters
///
/// * `slice` - the slice to read from
#[must_use]
pub const fn from_slice(slice: &[u32]) -> Option<Self> {
pub const fn from_slice(slice: &[u32]) -> Result<Self, VecError> {
if slice.len() != 2 {
None
Err(VecError::new(2, slice.len()))
} else {
Some(Self {
Ok(Self {
x: slice[0],
y: slice[1],
})
Expand Down
30 changes: 14 additions & 16 deletions alloy/src/math/vec/vec3.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use super::errors::VecError;

Check notice on line 1 in alloy/src/math/vec/vec3.rs

View workflow job for this annotation

GitHub Actions / Verify build integrity / Build and Test / Build and Test (ubuntu-latest)

consider importing this unresolved item through its public re-export instead

Check failure on line 1 in alloy/src/math/vec/vec3.rs

View workflow job for this annotation

GitHub Actions / Verify build integrity / Build and Test / Build and Test (ubuntu-latest)

unresolved import `super::errors::VecError`
use crate::math::vec::{Vec2, Vec3i, Vec3u};

use crate::cmp::{AlmostEq, Near};
Expand Down Expand Up @@ -100,7 +101,7 @@ impl Vec3 {

/// Forms a reference to a [`Vec3`] from a slice of [`f32`].
///
/// This requires that `slice.len() == 3`, otherwise this returns [`None`].
/// This requires that `slice.len() == 3`, otherwise this returns [`VecError`].
///
/// # Parameters
///
Expand Down Expand Up @@ -130,23 +131,22 @@ impl Vec3 {
///
/// let vec = Vec3::from_slice(slice);
///
/// assert_eq!(vec, None);
/// assert!(vec.is_err());
/// ```
#[must_use]
pub const fn from_slice(slice: &[f32]) -> Option<&Self> {
pub const fn from_slice(slice: &[f32]) -> Result<&Self, VecError> {
if slice.len() == 3 {
// SAFETY: Vec3 is transparent, and implemented directly in terms of a
// slice of f32s. The representation is the same, and thus valid.
// This is implemented symmetrically to `OsStr`.
Some(unsafe { Self::from_slice_unchecked(slice) })
Ok(unsafe { Self::from_slice_unchecked(slice) })
} else {
None
Err(VecError::new(3, slice.len()))
}
}

/// Forms a mutable reference to a [`Vec3`] from a mutable slice of [`f32`].
///
/// This requires that `slice.len() == 3`, otherwise this returns [`None`].
/// This requires that `slice.len() == 3`, otherwise this returns [`VecError`].
///
/// # Parameters
///
Expand Down Expand Up @@ -175,15 +175,14 @@ impl Vec3 {
///
/// assert_eq!(vec, None);
/// ```
#[must_use]
pub fn from_mut_slice(slice: &mut [f32]) -> Option<&mut Self> {
pub fn from_mut_slice(slice: &mut [f32]) -> Result<&mut Self, VecError> {
if slice.len() == 3 {
// SAFETY: Vec3 is transparent, and implemented directly in terms of a
// slice of f32s. The representation is the same, and thus valid.
// This is implemented symmetrically to `OsStr`.
Some(unsafe { Self::from_mut_slice_unchecked(slice) })
Ok(unsafe { Self::from_mut_slice_unchecked(slice) })
} else {
None
Err(VecError::new(3, slice.len()))
}
}

Expand Down Expand Up @@ -974,17 +973,16 @@ impl Vector3 {

/// Constructs this vector from a slice of floats.
///
/// This will return [`None`] if `slice.len()` is not equal to 2.
/// This will return [`VecError`] if `slice.len()` is not equal to 2.
///
/// # Parameters
///
/// * `slice` - the slice to read from
#[must_use]
pub const fn from_slice(slice: &[f32]) -> Option<Self> {
pub const fn from_slice(slice: &[f32]) -> Result<Self, VecError> {
if slice.len() != 3 {
None
Err(VecError::new(3, slice.len()))
} else {
Some(Self {
Ok(Self {
x: slice[0],
y: slice[1],
z: slice[3],
Expand Down
Loading

0 comments on commit 13d87e4

Please sign in to comment.