diff --git a/src/matrix.rs b/src/matrix.rs index 16585256..5a484cf0 100644 --- a/src/matrix.rs +++ b/src/matrix.rs @@ -85,13 +85,17 @@ impl Matrix { /// # Errors /// /// [`MatrixFormatError::WrongIndex`] if the ranges - /// are outside the original matrix. + /// are outside the original matrix, or if a range ends before it starts. pub fn slice( &self, rows: Range, columns: Range, ) -> Result { - if rows.end > self.rows || columns.end > self.columns { + if rows.end > self.rows + || columns.end > self.columns + || rows.start > rows.end + || columns.start > columns.end + { return Err(MatrixFormatError::WrongIndex); } let height = rows.end - rows.start; @@ -244,8 +248,12 @@ impl Matrix { /// original matrix. pub fn set_slice(&mut self, pos: (usize, usize), slice: &Self) { let (row, column) = pos; - let height = (self.rows - row).min(slice.rows); - let width = (self.columns - column).min(slice.columns); + // A position at or beyond the edge leaves nothing to copy, rather than underflowing. + let height = self.rows.saturating_sub(row).min(slice.rows); + let width = self.columns.saturating_sub(column).min(slice.columns); + if height == 0 || width == 0 { + return; + } for r in 0..height { self.data[(row + r) * self.columns + column..(row + r) * self.columns + column + width] .copy_from_slice(&slice.data[r * slice.columns..r * slice.columns + width]); @@ -778,7 +786,16 @@ impl Matrix { /// /// For more information refer to /// [In-place matrix transposition](https://en.wikipedia.org/wiki/In-place_matrix_transposition). + /// + /// # Panics + /// + /// This function will panic if the transposed matrix would end + /// up with empty rows, like [`Matrix::transposed`]. pub fn transpose(&mut self) { + assert!( + self.rows != 0 || self.columns == 0, + "this operation would create a matrix with empty rows" + ); // Transposing square matrices in place is significantly more efficient than non- // square matrices, so we handle that special case separately. if self.rows == self.columns { diff --git a/src/utils.rs b/src/utils.rs index b43ed774..98c1bdf4 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -95,6 +95,7 @@ pub const fn constrain(value: isize, upper: usize) -> usize { if value > 0 { value as usize % upper } else { - (upper - (-value) as usize % upper) % upper + // `unsigned_abs` rather than negation: `-isize::MIN` does not fit in an `isize`. + (upper - value.unsigned_abs() % upper) % upper } } diff --git a/tests/matrix.rs b/tests/matrix.rs index 16ce5209..03ff2708 100644 --- a/tests/matrix.rs +++ b/tests/matrix.rs @@ -841,3 +841,42 @@ fn is_empty() { let m: Matrix = matrix![]; assert!(m.is_empty()); } + +#[test] +fn set_slice_outside_the_matrix_changes_nothing() { + let mut m = Matrix::new(3, 3, 0u32); + let patch = Matrix::new(2, 2, 1u32); + // Documented as clipping to the matrix; a position past the edge leaves nothing to copy. + m.set_slice((5, 5), &patch); + m.set_slice((0, 5), &patch); + m.set_slice((5, 0), &patch); + m.set_slice((3, 3), &patch); + assert!(m.values().all(|&v| v == 0)); + // Partially outside still copies the part that fits. + m.set_slice((2, 2), &patch); + assert_eq!(m.values().filter(|&&v| v == 1).count(), 1); +} + +#[test] +#[expect( + clippy::reversed_empty_ranges, + reason = "a reversed range is exactly the input under test" +)] +fn slice_with_a_reversed_range_is_an_error() { + let m = Matrix::new(3, 3, 0u8); + assert!(matches!( + m.slice(2..1, 0..2), + Err(MatrixFormatError::WrongIndex) + )); + assert!(matches!( + m.slice(0..2, 2..1), + Err(MatrixFormatError::WrongIndex) + )); +} + +#[test] +#[should_panic(expected = "this operation would create a matrix with empty rows")] +fn transpose_in_place_refuses_what_transposed_refuses() { + let mut m = Matrix::::new_empty(3); + m.transpose(); +} diff --git a/tests/utils.rs b/tests/utils.rs index 0c08e237..4558228f 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -29,3 +29,16 @@ fn in_direction_valid() { vec![(2, 4), (3, 7)] ); } + +#[test] +fn constrain_the_most_negative_value() { + // -isize::MIN does not fit in an isize, so this cannot go through negation. + assert_eq!( + constrain(isize::MIN, 3), + usize::try_from(isize::MIN.rem_euclid(3)).unwrap() + ); + assert_eq!( + constrain(isize::MIN, 7), + usize::try_from(isize::MIN.rem_euclid(7)).unwrap() + ); +}