Lifetime issues with `slice` methods

[LeBoucEtMistere]

Hello folks,

I think I found an issue in the way lifetimes are inferred when using slice or slice_axis on array views.

Here is a minimal example with three functions, only bar and baz compile, while foo doesn't. I would argue that foo should be able to compile.

use ndarray; // 0.15.6

use ndarray::{ArrayBase, ViewRepr, Axis, Dimension, Slice, ArrayView};

fn main() {
    let a = ndarray::array![[1, 2, 3], [4, 5, 6]];
    foo(a.view(), ndarray::Axis(1));
}

fn foo<T, D>(arrayview: ArrayView<T, D>, axis: ndarray::Axis) -> ArrayView<T, D>
where
    D: Dimension,
{
    arrayview.slice_axis(axis, Slice::from(1..2))
}

fn bar<T, D>(arrayview: &mut ArrayView<T, D>, axis: Axis)
where
    D: Dimension,
{
    arrayview.slice_axis_inplace(axis, Slice::from(1..2));
}

fn baz<'a, 'data: 'a, T, D>(
    arrayview: &'a ArrayBase<ViewRepr<&'data T>, D>,
    axis: Axis,
) -> ArrayBase<ViewRepr<&'a T>, D>
where
    D: Dimension,
{
    arrayview.slice_axis(axis, Slice::from(1..2))
}

(playground link: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7d62b20eec45d257e629dae07840544b)

The error is the following:

error[[E0311]](https://doc.rust-lang.org/stable/error-index.html#E0311): the parameter type `D` may not live long enough
  --> src/main.rs:14:5
   |
14 |     arrayview.slice_axis(axis, Slice::from(1..2))
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: the parameter type `D` must be valid for the anonymous lifetime defined here...
  --> src/main.rs:10:25
   |
10 | fn foo<T, D>(arrayview: ArrayView<T, D>, axis: ndarray::Axis) -> ArrayView<T, D>
   |                         ^^^^^^^^^^^^^^^
note: ...so that the type `D` will meet its required lifetime bounds
  --> src/main.rs:14:5
   |
14 |     arrayview.slice_axis(axis, Slice::from(1..2))
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: consider adding an explicit lifetime bound...
   |
12 |     D: Dimension + 'a,
   |                  ++++

error[[E0515]](https://doc.rust-lang.org/stable/error-index.html#E0515): cannot return reference to function parameter `arrayview`
  --> src/main.rs:14:5
   |
14 |     arrayview.slice_axis(axis, Slice::from(1..2))
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returns a reference to data owned by the current function

For more information about this error, try `rustc --explain E0515`.
error: could not compile `playground` due to 2 previous errors

My understanding of the problem is the following: the lifetime of the data represented by the array view passed in as argument to foo is not propagated correctly to the one obtained by slicing it, and the output of the slice function is actually depending on the lifetime of the view passed as argument and not the lifetime of the data referenced by the view.
In bar I show how to circumnavigate the issue by slicing in place. Given that views are cheap to create and clone, this is the alternative I'm going with for now.
In baz, I show a version where I explicitly annotate lifetimes to make it work, note how I have to constrain 'a to 'data and how I need to make the argument a reference to avoid having the function drop it while the returned value still references it.

Also, I'm not sure why the error message is angry about the lifetime of D in the first place, I don't see where it's being defined as a reference or a pointer.

I'm not super good at understanding lifetimes but it feels like it should be possible to slice a view and obtain something that doesn't depend on the view itself but on the underlying data only. Or am I missing something here ?

[jturner314]

The existing slice_axis method is defined like this:

impl<A, S, D> ArrayBase<S, D>
where
    S: RawData<Elem = A>,
    D: Dimension,
{
    pub fn slice_axis(&self, axis: Axis, indices: Slice) -> ArrayView<'_, A, D>
    where
        S: Data,
    { ... }
}

With explicit lifetimes, that's equivalent to:

impl<A, S, D> ArrayBase<S, D>
where
    S: RawData<Elem = A>,
    D: Dimension,
{
    pub fn slice_axis<'b>(&'b self, axis: Axis, indices: Slice) -> ArrayView<'b, A, D>
    where
        S: Data,
    { ... }
}

It's defined this way in order to be generic over all storage types (owned arrays, views, mutable views, etc.).

ArrayView<'a, A, D> is defined as:

pub type ArrayView<'a, A, D> = ArrayBase<ViewRepr<&'a A>, D>;

Let's call 'a in this definition the "data" lifetime of the view.

In the case of ArrayView, observe that the slice_axis method is effectively:

impl<'a, A, D> ArrayView<'a, A, D>
where
    D: Dimension,
{
    pub fn slice_axis<'b>(&'b self, axis: Axis, indices: Slice) -> ArrayView<'b, A, D> { ... }
}

The "data" lifetime of the returned view corresponds to the borrow of self ('b), not the "data" lifetime of the original view ('a). Your foo function looks like this, after de-sugaring:

fn foo<'a, T, D>(arrayview: ArrayView<'a, T, D>, axis: ndarray::Axis) -> ArrayView<'a, T, D>
where
    D: Dimension,
{
    ArrayBase::slice_axis(&arrayview, axis, Slice::from(1..2))
}

With this de-sugared source, it's clearer why it fails to compile: the slice_axis call returns a view with a "data" lifetime which matches the &arrayview borrow within the body of the function, not the "data" lifetime ('a) of arrayview. I agree that the compiler's first error message (E0311) is unnecessarily confusing in this case. The second error message (E0515) is the more useful message. It's saying that the "data" lifetime of the view returned by the slice_axis call matches the &arrayview borrow. Since arrayview is "data owned by the current function" (and arrayview will be dropped at the end of the function), the function cannot return a view with a "data" lifetime matching a borrow of arrayview. (The lifetime of the borrow of arrayview cannot be longer than the lifetime of the arrayview instance itself, which ends at the end of the function body.)

You are correct that we could define an additional method for ArrayView like this:

impl<'a, T, D> ArrayView<'a, T, D>
where
    D: Dimension,
{
    pub fn slice_axis_preserve_lifetime<'b>(&'b self, axis: Axis, indices: Slice) -> ArrayView<'a, T, D> { ... }
}

This differs from the existing slice_axis method in the lifetime of the returned view.

However, an ArrayBase::slice_axis_move method similar to the existing slice_move method would provide (almost) the same functionality as an ArrayView::slice_axis_preserve_lifetime method for views, and it would be useful for owned arrays too. The only disadvantage is that for dynamic- or generic-dimensional array views, the caller would have to .clone() the view before calling .slice_axis_move(). I've created PR #1211 to add a slice_axis_move method.

Does my explanation above answer your question?

[LeBoucEtMistere]

Woah thanks a lot for the comprehensive explanation, it's crystal clear ! Ultimately I ended up doing the same thing as you did in your implementation of slice_axis_move by leveraging a clone of the view (obtained with my_view.view()) passed by value and a call to slice_axis_inplace, so I guess it's reasonable to go the move route to implement this to stay in the spirit of genericity over data storage backend.

However, I've been tinkering more with ndarray in the past week, and I think I would like to generalize the topic of this post a little bit. I feel like ArrayViews are not as easy to use as I would like. Conceptually, I was hoping to use them in the same way as when I use a &[] derived from an owned representation like Vec<T>. However, most ArrayBase generic functions that work on them propagate the lifetime of the container (ArrayBase) instead of the contained data. Because of this, it's harder to work with array views and to write functions with clear contract for wether or not they copy data.

Usage of arrayviews also feel clunky when you look at ArrayBase.clone, in opposition to ArrayView.view, the first one, when called on an ArrayView, will copy the data, whereas one could expect it to only derive a new view on top of the already owned data. It makes it unclear whether ArrayView is really a "cheap" data type, reference-like.

I have another example of why this approach feels clunky to me: I had to write this extension trait to be able to write functions automatically accepting references to arrays or array views:

/// Let's you pass either an owned ArrayView or an immutable reference to an array to a function that needs only an array view
    pub trait AsArrayView<'data, T, D>
    where
        D: Dimension,
    {
        fn view(self: Self) -> ArrayView<'data, T, D>;
    }

    impl<'data, T, D> AsArrayView<'data, T, D> for ArrayView<'data, T, D>
    where
        D: Dimension,
    {
        fn view(self: ArrayView<'data, T, D>) -> ArrayView<'data, T, D> {
            self
        }
    }

    impl<'data, T, D> AsArrayView<'data, T, D> for &'data Array<T, D>
    where
        D: Dimension,
    {
        fn view(self: &'data Array<T, D>) -> ArrayView<'data, T, D> {
            self.view()
        }
    }

This made it easier for me to develop code where &arrays are used interchangeably with array views when calling code that only requires an array view. (Btw I was surprised such a trait wasn't already existing in the library, maybe it's worth upsourcing something like that ?)

After having worked with ndarray for a week or so, this is definitely my main pain point, ArrayViews don't feel like pointer types enough. I'm honestly not sure how this could be addressed however, I do understand the appeal of having methods being declared on ArrayBase instead, but maybe they could be specialized on the ArrayView type, if they were defined in a separate trait instead ? (mostly food for thoughts, I probably doesn't have a clue of what this would imply in terms of library architectural changes ahah)

[jturner314]

Usage of arrayviews also feel clunky when you look at ArrayBase.clone, in opposition to ArrayView.view, the first one, when called on an ArrayView, will copy the data, whereas one could expect it to only derive a new view on top of the already owned data.

Fwiw, this isn't true. Calling .clone() on an ArrayView does not copy the array data; it just copies the pointer, shape, and strides into a new view. This is nearly free, except for dynamic-dimensional views with ndim > 4. (Dynamic-dimensional views with ndim > 4 use heap allocations for the shape and strides.) Also, fwiw, ArrayView actually implements Copy for ArrayView0, …, ArrayView6.

I agree that views are much more awkward to use than &[]. We're unfortunately somewhat limited by the language, which has limited ability to define custom fat pointers. Short of language changes, one thing that would really help is something like #879. I'd also be interested in exploring a trait-based API for generalization instead of using ArrayBase generic over the storage type. For something like slice_axis, we could have trait implementations for arrays of all storage types, and we could have an inherent method of the same name for ArrayViews which would be equivalent to the slice_axis_preserve_lifetime method suggested above. (In the case of an inherent method and trait method of the same name, object.method() calls the inherent method.)

Regarding the existing version of ndarray, I'd suggest looking at the "Motivation" section of #879, which has some discussion of e.g. different ways to handle function parameters and the limitations of each way.

[LeBoucEtMistere]

Fwiw, this isn't true. Calling .clone() on an ArrayView does not copy the array data; it just copies the pointer, shape, and strides into a new view. This is nearly free, except for dynamic-dimensional views with ndim > 4. (Dynamic-dimensional views with ndim > 4 use heap allocations for the shape and strides.) Also, fwiw, ArrayView actually implements Copy for ArrayView0, …, ArrayView6.

Oh I definitely missed this part, thanks for correcting me then.

Thanks for linking the RFC, I wasn't aware of all this work you had going on. The thoughts you laid out seem really interesting, I probably don't know the project enough to have a meaningful opinion, but it does look like a step in a better direction to me. I noted that the discussions on this date back to 2021, is there a technical reason that blocked implementation or is this only linked with dev bandwidth/logistics (which are perfectly fine reasons ofc)? Is the project lacking contributors?

I'm also very interested in this idea of Trait-based implementations, this looks like a much more flexible approach to adding behaviours on top of data storages. I'm currently writing a naive implementation of 1-dim masking in a separate trait to mimic numpy's capabilities here, if this proves mature enough at some point I might open a PR to share it.

Anyway, thanks for all the answers Jim 🙇

[jturner314]

I noted that the discussions on this date back to 2021, is there a technical reason that blocked implementation or is this only linked with dev bandwidth/logistics (which are perfectly fine reasons ofc)? Is the project lacking contributors?

Yes, it's basically due to dev bandwidth. @bluss and I are the primary contributors, but I don't think @bluss has much time to work on ndarray any more, and I have less time that I used to. I make an effort to answer questions on the issue tracker and fix any bugs that users encounter, but I haven't done much design work in a while. To some extent, I've been waiting on Generic Associated Types (GATs) (rust-lang/rust#96709) before making significant changes to the design, since they'll make the final design cleaner. GATs should be in the next stable version of Rust, I think, so that'll no longer be a blocker. Mostly, though, I (or someone else) just needs to find the time to work on ndarray.