`.as_slice_memory_order_mut()` is very difficult to use correctly in generic code, and there are a couple of bugs as a result

[jturner314]

I just realized that .as_slice_memory_order_mut() is very difficult to use correctly in code with generic S: DataMut for any purpose other than iterating over the elements in arbitrary order. The issue is that .as_slice_memory_order_mut() unshares the data, which may change its layout, so strides obtained before calling .as_slice_memory_order_mut() may not be correct for accessing elements in the returned slice. As a result, the implementation of .map_mut() and the implementation of .zip_mut_with_same_shape() produce incorrect results in some cases for ArcArray inputs. Here's an example:

use ndarray::prelude::*;
use ndarray::ArcArray2;

fn main() {
    // Fortran-layout `ArcArray`.
    let a: ArcArray2<u8> = {
        let orig = array![[0, 1, 2, 3, 4], [5, 6, 7, 8, 9]];
        let mut orig_f = Array2::zeros(orig.raw_dim().f());
        orig_f.assign(&orig);
        orig_f.into_shared()
    };
    assert_eq!(a, array![[0, 1, 2, 3, 4], [5, 6, 7, 8, 9]]);
    assert_eq!(
        a.as_slice_memory_order(),
        Some(&[0, 5, 1, 6, 2, 7, 3, 8, 4, 9][..])
    );

    // Shared reference of a portion of `a`.
    let mut b = a.clone().slice_move(s![.., ..2]);
    assert_eq!(b, array![[0, 1], [5, 6]]);
    assert_eq!(b.as_slice_memory_order(), Some(&[0, 5, 1, 6][..]));

    // `.map_mut()` is broken in this case, because the data is contiguous and
    // unsharing the data changes the layout. The current implementation of
    // `.map_mut()` uses the strides of `b` from *before* unsharing to construct
    // the new array, but these are no longer correct *after* the unsharing.
    //
    // As a result, this assertion fails using the current implementation.
    assert_eq!(b.map_mut(|&mut x| x + 10), array![[10, 11], [15, 16]]);
}

It's also worth noting that the current implementation of try_as_slice_memory_order_mut is questionable, because it checks for contiguity before unsharing with ensure_unique, and DataMut doesn't guarantee that the array after calling ensure_unique will still be contiguous (although this is currently the case for all existing storage types).

I think the best solution is to do the following:

1. Add this constraint to RawDataMut: "The implementer must ensure that if the input to try_ensure_unique is contiguous, then the output will have the same strides as the input."
2. Change the implementation of RawDataMut for OwnedArcRepr to uphold this constraint.
3. Document that .as_slice_memory_order_mut() unshares the data if necessary, but the strides are preserved.

[bluss]

Oh good that you found this. I guess it's a gotcha we've sort of grown into. Unsharing cow arrays is a problematic operation in this sense.