Skip to content

fix(matrix): respect the documented bounds in set_slice, slice and transpose - #845

Merged
samueltardieu merged 1 commit into
evenfurther:mainfrom
tachsin:fix/matrix-bounds
Sep 24, 2026
Merged

samueltardieu merged 1 commit into
evenfurther:mainfrom
tachsin:fix/matrix-bounds

Conversation

@tachsin

@tachsin tachsin commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Fixes #844.

Four places where Matrix panicked on inputs its documentation covers.

  • set_slice is documented to copy only the cells that fit when the slice goes outside the matrix, but a position past the edge underflowed self.rows - row and panicked. It now uses saturating_sub, and copies nothing when nothing fits.
  • slice already returns WrongIndex for a range running past the matrix. A range ending before it starts underflowed rows.end - rows.start and panicked with a capacity overflow; it now gets WrongIndex too, and the documentation says so.
  • transpose, in place, now refuses a matrix without rows with exactly the check and message transposed already uses, and documents it under # Panics. Before, debug builds underflowed and release builds left a matrix with rows of zero columns, which Matrix otherwise never allows. Nothing inside the crate calls transpose; the internal callers use transposed, which already had this check.
  • utils::constrain, behind Matrix::constrain and Grid::constrain, negated negative values, and -isize::MIN does not fit in an isize. It now uses unsigned_abs, which gives the same result for every other value.

Tests

In tests/matrix.rs and tests/utils.rs:

  • set_slice_outside_the_matrix_changes_nothing, including a position partly inside to check that clipping still copies what fits
  • slice_with_a_reversed_range_is_an_error, for both rows and columns
  • transpose_in_place_refuses_what_transposed_refuses, matching the panic message
  • constrain_the_most_negative_value, against rem_euclid

Each fails on current main; the constrain one in a debug build, since release happened to wrap to the right answer.

For the rest of Matrix I compared rotations in both directions for every count, both flips, transposed and the in-place transpose (including the non-square cycle-following path), slice, neighbours, in_direction and constrain against straightforward reimplementations on a few hundred random matrices up to 6x6. They all agreed, so these fixes are limited to the edges listed above.

…anspose

set_slice is documented to copy only what fits when the slice goes
outside the matrix, but a position past the edge underflowed and
panicked. It now copies nothing in that case.

slice returns WrongIndex for ranges running past the matrix, but a
range ending before it starts underflowed and panicked with a capacity
overflow. It now returns WrongIndex for that as well.

transpose had no check for a matrix without rows, which transposed
refuses because the result would have empty rows. Debug builds
underflowed, and release builds produced such a matrix. It now panics
with the same message as transposed.

constrain negated negative values, and -isize::MIN does not fit in an
isize, so debug builds panicked on it. It now uses unsigned_abs.

Fixes evenfurther#844
@samueltardieu

Copy link
Copy Markdown
Member

Good catch

@samueltardieu
samueltardieu added this pull request to the merge queue Sep 24, 2026
Merged via the queue into evenfurther:main with commit 71bfdcb Sep 24, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Matrix::set_slice, slice and transpose panic on inputs their documentation covers

2 participants