Skip to content

tech-debt: replace unsafe raw-pointer iterator_mut in DenseMatrix (basic/matrix.rs) with safe split_at_mut approach #368

@Mec-iS

Description

@Mec-iS

Context

DenseMatrix::iterator_mut in src/linalg/basic/matrix.rs uses raw pointer arithmetic with unsafe blocks to implement both row-major (axis-0) and column-major (axis-1) mutable iteration:

// Current implementation (DenseMatrix and DenseMatrixMutView)
let ptr = self.values.as_mut_ptr();
// ...
Box::new((0..nrows).flat_map(move |r| {
    (0..ncols).map(move |c| unsafe {
        &mut *ptr.add(if column_major { r + c * nrows } else { r * ncols + c })
    })
}))

This is a pre-existing pattern that was not introduced by any recent PR. It is guarded by a #[cfg(debug_assertions)] HashSet-based aliasing check, and a // Safety: comment documents the invariant. However, per the project design principles:

No use of unsafe blocks.

What Should Be Done

Replace the raw pointer approach with a safe Rust alternative. Two viable approaches:

Option A — split_at_mut chain (zero-copy, no allocation)

For row-major (axis-0), the flat values Vec is already in the correct order for DenseMatrix with column_major = false, so values.iter_mut() works directly. For column-major storage or axis-1 traversal, use recursive split_at_mut to produce non-aliasing &mut T slices:

// Conceptual sketch for column-major axis-0 on a column_major=true DenseMatrix
// Stride = nrows; each column lives at values[col*nrows .. (col+1)*nrows]
let chunks: Vec<&mut T> = self.values
    .chunks_mut(nrows)              // each chunk = one column
    .flat_map(|col| col.iter_mut()) // row-major within each column
    .collect();
Box::new(chunks.into_iter())

For axis-1 on a row-major layout the chunks would be of size ncols.

Option B — Restructure to avoid axis-1 mutable iteration

Audit all call sites of iterator_mut(1). If they can be replaced by reading with iterator(0) and writing back via set(), the mutable column-major iterator can be removed from the trait entirely, simplifying the implementation surface.

Acceptance Criteria

  • iterator_mut for DenseMatrix contains no unsafe block
  • iterator_mut for DenseMatrixMutView contains no unsafe block
  • All existing test_iter_mut tests continue to pass
  • No new allocations introduced for the hot path (axis-0 row-major case)
  • cargo clippy --all-features -- -Drust-2018-idioms -Dwarnings passes clean

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions