Skip to content

Commit

Permalink
Fix clippy errors in nalgebra base
Browse files Browse the repository at this point in the history
  • Loading branch information
JulianKnodt committed Sep 25, 2023
1 parent bc452b4 commit 127ddf7
Show file tree
Hide file tree
Showing 18 changed files with 108 additions and 23 deletions.
13 changes: 13 additions & 0 deletions src/base/construction_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ macro_rules! impl_constructors(
}

/// Creates, without bound checking, a new matrix view from the given data array.
/// # Safety
/// `data[start..start+rstride * cstride]` must be within bounds.
#[inline]
pub unsafe fn from_slice_unchecked(data: &'a [T], start: usize, $($args: usize),*) -> Self {
Self::from_slice_generic_unchecked(data, start, $($gargs),*)
Expand All @@ -113,6 +115,11 @@ macro_rules! impl_constructors(
}

/// Creates, without bound checking, a new matrix view with the specified strides from the given data array.
///
/// # Safety
///
/// `start`, `rstride`, and `cstride`, with the given matrix size will not index
/// outside of `data`.
#[inline]
pub unsafe fn from_slice_with_strides_unchecked(data: &'a [T], start: usize, $($args: usize,)* rstride: usize, cstride: usize) -> Self {
Self::from_slice_with_strides_generic_unchecked(data, start, $($gargs,)* Dyn(rstride), Dyn(cstride))
Expand Down Expand Up @@ -257,6 +264,10 @@ macro_rules! impl_constructors_mut(
}

/// Creates, without bound checking, a new mutable matrix view from the given data array.
///
/// # Safety
///
/// `data[start..start+(R * C)]` must be within bounds.
#[inline]
pub unsafe fn from_slice_unchecked(data: &'a mut [T], start: usize, $($args: usize),*) -> Self {
Self::from_slice_generic_unchecked(data, start, $($gargs),*)
Expand All @@ -274,6 +285,8 @@ macro_rules! impl_constructors_mut(
}

/// Creates, without bound checking, a new mutable matrix view with the specified strides from the given data array.
/// # Safety
/// `data[start..start+rstride * cstride]` must be within bounds.
#[inline]
pub unsafe fn from_slice_with_strides_unchecked(data: &'a mut [T], start: usize, $($args: usize,)* rstride: usize, cstride: usize) -> Self {
Self::from_slice_with_strides_generic_unchecked(
Expand Down
4 changes: 4 additions & 0 deletions src/base/dimension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ impl IsNotStaticOne for Dyn {}

/// Trait implemented by any type that can be used as a dimension. This includes type-level
/// integers and `Dyn` (for dimensions not known at compile-time).
///
/// # Safety
///
/// Hoists integers to the type level, including binary operations.
pub unsafe trait Dim: Any + Debug + Copy + PartialEq + Send + Sync {
#[inline(always)]
fn is<D: Dim>() -> bool {
Expand Down
8 changes: 4 additions & 4 deletions src/base/edition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ impl<T: Scalar, R: Dim, C: Dim, S: Storage<T, R, C>> Matrix<T, R, C, S> {
if nremove.value() != 0 {
unsafe {
compress_rows(
&mut m.as_mut_slice(),
m.as_mut_slice(),
nrows.value(),
ncols.value(),
i,
Expand Down Expand Up @@ -796,7 +796,7 @@ impl<T: Scalar, R: Dim, C: Dim, S: Storage<T, R, C>> Matrix<T, R, C, S> {

if ninsert.value() != 0 {
extend_rows(
&mut res.as_mut_slice(),
res.as_mut_slice(),
nrows.value(),
ncols.value(),
i,
Expand Down Expand Up @@ -909,7 +909,7 @@ impl<T: Scalar, R: Dim, C: Dim, S: Storage<T, R, C>> Matrix<T, R, C, S> {
unsafe {
if new_nrows.value() < nrows {
compress_rows(
&mut data.as_mut_slice(),
data.as_mut_slice(),
nrows,
ncols,
new_nrows.value(),
Expand All @@ -923,7 +923,7 @@ impl<T: Scalar, R: Dim, C: Dim, S: Storage<T, R, C>> Matrix<T, R, C, S> {
new_nrows, new_ncols, data.data,
));
extend_rows(
&mut res.as_mut_slice(),
res.as_mut_slice(),
nrows,
new_ncols.value(),
nrows,
Expand Down
7 changes: 7 additions & 0 deletions src/base/indexing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,10 @@ impl<T, R: Dim, C: Dim, S: RawStorage<T, R, C>> Matrix<T, R, C, S> {

/// Produces a view of the data at the given index, without doing
/// any bounds checking.
///
/// # Safety
///
/// `index` must within bounds of the array.
#[inline]
#[must_use]
pub unsafe fn get_unchecked<'a, I>(&'a self, index: I) -> I::Output
Expand All @@ -530,6 +534,9 @@ impl<T, R: Dim, C: Dim, S: RawStorage<T, R, C>> Matrix<T, R, C, S> {

/// Returns a mutable view of the data at the given index, without doing
/// any bounds checking.
/// # Safety
///
/// `index` must within bounds of the array.
#[inline]
#[must_use]
pub unsafe fn get_unchecked_mut<'a, I>(&'a mut self, index: I) -> I::OutputMut
Expand Down
12 changes: 12 additions & 0 deletions src/base/matrix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,10 @@ where
impl<T, R, C, S> Matrix<T, R, C, S> {
/// Creates a new matrix with the given data without statically checking that the matrix
/// dimension matches the storage dimension.
///
/// # Safety
///
/// The storage dimension must match the given dimensions.
#[inline(always)]
pub const unsafe fn from_data_statically_unchecked(data: S) -> Matrix<T, R, C, S> {
Matrix {
Expand Down Expand Up @@ -1194,6 +1198,10 @@ impl<T, R: Dim, C: Dim, S: RawStorageMut<T, R, C>> Matrix<T, R, C, S> {
}

/// Swaps two entries without bound-checking.
///
/// # Safety
///
/// Both `(r, c)` must have `r < nrows(), c < ncols()`.
#[inline]
pub unsafe fn swap_unchecked(&mut self, row_cols1: (usize, usize), row_cols2: (usize, usize)) {
debug_assert!(row_cols1.0 < self.nrows() && row_cols1.1 < self.ncols());
Expand Down Expand Up @@ -1300,6 +1308,8 @@ impl<T, R: Dim, C: Dim, S: RawStorageMut<T, R, C>> Matrix<T, R, C, S> {

impl<T, D: Dim, S: RawStorage<T, D>> Vector<T, D, S> {
/// Gets a reference to the i-th element of this column vector without bound checking.
/// # Safety
/// `i` must be less than `D`.
#[inline]
#[must_use]
pub unsafe fn vget_unchecked(&self, i: usize) -> &T {
Expand All @@ -1311,6 +1321,8 @@ impl<T, D: Dim, S: RawStorage<T, D>> Vector<T, D, S> {

impl<T, D: Dim, S: RawStorageMut<T, D>> Vector<T, D, S> {
/// Gets a mutable reference to the i-th element of this column vector without bound checking.
/// # Safety
/// `i` must be less than `D`.
#[inline]
#[must_use]
pub unsafe fn vget_unchecked_mut(&mut self, i: usize) -> &mut T {
Expand Down
20 changes: 14 additions & 6 deletions src/base/matrix_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ macro_rules! view_storage_impl (

impl<'a, T, R: Dim, C: Dim, RStride: Dim, CStride: Dim> $T<'a, T, R, C, RStride, CStride> {
/// Create a new matrix view without bounds checking and from a raw pointer.
///
/// # Safety
///
/// `*ptr` must point to memory that is valid `[T; R * C]`.
#[inline]
pub unsafe fn from_raw_parts(ptr: $Ptr,
shape: (R, C),
Expand All @@ -63,6 +67,11 @@ macro_rules! view_storage_impl (
// Dyn is arbitrary. It's just to be able to call the constructors with `Slice::`
impl<'a, T, R: Dim, C: Dim> $T<'a, T, R, C, Dyn, Dyn> {
/// Create a new matrix view without bounds checking.
///
/// # Safety
///
/// `storage` contains sufficient elements beyond `start + R * C` such that all
/// accesses are within bounds.
#[inline]
pub unsafe fn new_unchecked<RStor, CStor, S>(storage: $SRef, start: (usize, usize), shape: (R, C))
-> $T<'a, T, R, C, S::RStride, S::CStride>
Expand All @@ -75,6 +84,10 @@ macro_rules! view_storage_impl (
}

/// Create a new matrix view without bounds checking.
///
/// # Safety
///
/// `strides` must be a valid stride indexing.
#[inline]
pub unsafe fn new_with_strides_unchecked<S, RStor, CStor, RStride, CStride>(storage: $SRef,
start: (usize, usize),
Expand Down Expand Up @@ -128,12 +141,7 @@ impl<'a, T: Scalar, R: Dim, C: Dim, RStride: Dim, CStride: Dim> Clone
{
#[inline]
fn clone(&self) -> Self {
Self {
ptr: self.ptr,
shape: self.shape,
strides: self.strides,
_phantoms: PhantomData,
}
*self
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/base/norm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ where
let (elt, basis) = vs[..i + 1].split_last_mut().unwrap();

for basis_element in &basis[..nbasis_elements] {
*elt -= &*basis_element * elt.dot(basis_element)
*elt -= basis_element * elt.dot(basis_element)
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/base/statistics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ impl<T: Scalar, R: Dim, C: Dim, S: RawStorage<T, R, C>> Matrix<T, R, C, S> {
let mean = self.mean();

self.iter().cloned().fold(T::zero(), |acc, x| {
acc + (x.clone() - mean.clone()) * (x.clone() - mean.clone())
acc + (x.clone() - mean.clone()) * (x - mean.clone())
}) / n_elements
}
}
Expand Down
19 changes: 19 additions & 0 deletions src/base/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ pub type CStride<T, R, C = U1> =
/// The trait shared by all matrix data storage.
///
/// TODO: doc
/// # Safety
///
/// In generic code, it is recommended use the `Storage` trait bound instead. The `RawStorage`
/// trait bound is generally used by code that needs to work with storages that contains
/// `MaybeUninit<T>` elements.
Expand All @@ -41,6 +43,7 @@ pub type CStride<T, R, C = U1> =
/// should **not** allow the user to modify the size of the underlying buffer with safe methods
/// (for example the `VecStorage::data_mut` method is unsafe because the user could change the
/// vector's size so that it no longer contains enough elements: this will lead to UB.
///
pub unsafe trait RawStorage<T, R: Dim, C: Dim = U1>: Sized {
/// The static stride of this storage's rows.
type RStride: Dim;
Expand Down Expand Up @@ -129,6 +132,14 @@ pub unsafe trait RawStorage<T, R: Dim, C: Dim = U1>: Sized {
}

/// Trait shared by all matrix data storage that don’t contain any uninitialized elements.
///
/// # Safety
///
/// Note that `Self` must always have a number of elements compatible with the matrix length (given
/// by `R` and `C` if they are known at compile-time). For example, implementors of this trait
/// should **not** allow the user to modify the size of the underlying buffer with safe methods
/// (for example the `VecStorage::data_mut` method is unsafe because the user could change the
/// vector's size so that it no longer contains enough elements: this will lead to UB.
pub unsafe trait Storage<T, R: Dim, C: Dim = U1>: RawStorage<T, R, C> {
/// Builds a matrix data storage that does not contain any reference.
fn into_owned(self) -> Owned<T, R, C>
Expand All @@ -143,6 +154,8 @@ pub unsafe trait Storage<T, R: Dim, C: Dim = U1>: RawStorage<T, R, C> {

/// Trait implemented by matrix data storage that can provide a mutable access to its elements.
///
/// # Safety
///
/// In generic code, it is recommended use the `StorageMut` trait bound instead. The
/// `RawStorageMut` trait bound is generally used by code that needs to work with storages that
/// contains `MaybeUninit<T>` elements.
Expand Down Expand Up @@ -226,6 +239,10 @@ pub unsafe trait RawStorageMut<T, R: Dim, C: Dim = U1>: RawStorage<T, R, C> {
}

/// Trait shared by all mutable matrix data storage that don’t contain any uninitialized elements.
///
/// # Safety
///
/// See safety note for `Storage`, `RawStorageMut`.
pub unsafe trait StorageMut<T, R: Dim, C: Dim = U1>:
Storage<T, R, C> + RawStorageMut<T, R, C>
{
Expand All @@ -241,6 +258,8 @@ where

/// Marker trait indicating that a storage is stored contiguously in memory.
///
/// # Safety
///
/// The storage requirement means that for any value of `i` in `[0, nrows * ncols - 1]`, the value
/// `.get_unchecked_linear` returns one of the matrix component. This trait is unsafe because
/// failing to comply to this may cause Undefined Behaviors.
Expand Down
1 change: 1 addition & 0 deletions src/geometry/dual_quaternion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ where
}

impl<T: RealField> DualQuaternion<T> {
#[allow(clippy::wrong_self_convention)]
fn to_vector(self) -> OVector<T, U8> {
self.as_ref().clone().into()
}
Expand Down
12 changes: 12 additions & 0 deletions src/geometry/point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,10 @@ where
}

/// Gets a reference to i-th element of this point without bound-checking.
///
/// # Safety
///
/// `i` must be less than `self.len()`.
#[inline]
#[must_use]
pub unsafe fn get_unchecked(&self, i: usize) -> &T {
Expand Down Expand Up @@ -344,13 +348,21 @@ where
}

/// Gets a mutable reference to i-th element of this point without bound-checking.
///
/// # Safety
///
/// `i` must be less than `self.len()`.
#[inline]
#[must_use]
pub unsafe fn get_unchecked_mut(&mut self, i: usize) -> &mut T {
self.coords.vget_unchecked_mut(i)
}

/// Swaps two entries without bound-checking.
///
/// # Safety
///
/// `i1` and `i2` must be less than `self.len()`.
#[inline]
pub unsafe fn swap_unchecked(&mut self, i1: usize, i2: usize) {
self.coords.swap_unchecked((i1, 0), (i2, 0))
Expand Down
5 changes: 5 additions & 0 deletions src/geometry/rotation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,11 @@ impl<T: Scalar, const D: usize> Rotation<T, D> {
}

/// A mutable reference to the underlying matrix representation of this rotation.
///
/// # Safety
///
/// Invariants of the rotation matrix should not be violated.
///
#[inline]
#[deprecated(note = "Use `.matrix_mut_unchecked()` instead.")]
pub unsafe fn matrix_mut(&mut self) -> &mut SMatrix<T, D, D> {
Expand Down
6 changes: 3 additions & 3 deletions src/geometry/rotation_specialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1058,7 +1058,7 @@ impl<T: SimdRealField> Rotation3<T> {
{
let mut angles = [T::zero(); 3];
let eps = T::from_subset(&1e-7);
let _2 = T::from_subset(&2.0);
let two = T::from_subset(&2.0);

if extrinsic {
seq.reverse();
Expand Down Expand Up @@ -1090,7 +1090,7 @@ impl<T: SimdRealField> Rotation3<T> {
-s1,
c1,
);
let o_t = &c * self.matrix() * (c.transpose() * r1l);
let o_t = c * self.matrix() * (c.transpose() * r1l);
angles[1] = o_t.m33.acos();

let safe1 = angles[1].abs() >= eps;
Expand Down Expand Up @@ -1131,7 +1131,7 @@ impl<T: SimdRealField> Rotation3<T> {
// dont adjust gimbal locked rotation
if adjust && observable {
angles[0] += T::pi();
angles[1] = _2 * lambda - angles[1];
angles[1] = two * lambda - angles[1];
angles[2] -= T::pi();
}

Expand Down
4 changes: 4 additions & 0 deletions src/geometry/scale.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ impl<T: Scalar, const D: usize> Scale<T, D> {
/// assert_eq!(t.inverse_unchecked() * t, Scale2::identity());
/// }
/// ```
///
/// # Safety
///
/// Should only be used if all scaling is known to be non-zero.
#[inline]
#[must_use]
pub unsafe fn inverse_unchecked(&self) -> Scale<T, D>
Expand Down
8 changes: 4 additions & 4 deletions src/geometry/scale_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,28 +83,28 @@ add_sub_impl!(Mul, mul, ClosedMul;
(Const<D>, U1), (Const<D>, U1) -> (Const<D>, U1)
const D; for; where;
self: &'a Scale<T, D>, right: &'b SVector<T, D>, Output = SVector<T, D>;
SVector::from(self.vector.component_mul(&right));
self.vector.component_mul(right);
'a, 'b);

add_sub_impl!(Mul, mul, ClosedMul;
(Const<D>, U1), (Const<D>, U1) -> (Const<D>, U1)
const D; for; where;
self: &'a Scale<T, D>, right: SVector<T, D>, Output = SVector<T, D>;
SVector::from(self.vector.component_mul(&right));
self.vector.component_mul(&right);
'a);

add_sub_impl!(Mul, mul, ClosedMul;
(Const<D>, U1), (Const<D>, U1) -> (Const<D>, U1)
const D; for; where;
self: Scale<T, D>, right: &'b SVector<T, D>, Output = SVector<T, D>;
SVector::from(self.vector.component_mul(&right));
self.vector.component_mul(right);
'b);

add_sub_impl!(Mul, mul, ClosedMul;
(Const<D>, U1), (Const<D>, U1) -> (Const<D>, U1)
const D; for; where;
self: Scale<T, D>, right: SVector<T, D>, Output = SVector<T, D>;
SVector::from(self.vector.component_mul(&right)); );
self.vector.component_mul(&right); );

// Scale *= Scale
add_sub_assign_impl!(MulAssign, mul_assign, ClosedMul;
Expand Down
Loading

0 comments on commit 127ddf7

Please sign in to comment.