feat: impl Into for Span/Array pair

[tdelabro]

fixes #5631

---

This change is 

[orizi]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @tdelabro)

---

corelib/src/array.cairo line 151 at r1 (raw file):

        self.span()
    }
}

there's not equivalent for making a vec into a span using into in rust.
please find out why before we decide on adding such a thing.

Code quote:

impl ArrayIntoSpan<T, +Drop<T>> of Into<Array<T>, Span<T>> {
    fn into(self: Array<T>) -> Span<T> {
        self.span()
    }
}

---

corelib/src/array.cairo line 161 at r1 (raw file):

        arr
    }
}

Suggestion:

impl SpanIntoArray<T, +Drop<T>, +Clone<T>> of Into<Span<T>, Array<T>> {
    fn into(mut self: Span<T>) -> Array<T> {
        let mut arr = array![];
        arr.append_span(self);
        arr
    }
}

[tdelabro]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @orizi)

---

corelib/src/array.cairo line 151 at r1 (raw file):

Previously, orizi wrote…

there's not equivalent for making a vec into a span using into in rust.
please find out why before we decide on adding such a thing.

tldr: there is not an exact equivalence between rust &[] and Cairo snapshots.

In Rust the implementation would not compile:

// input is passed by value
fn into(input: Vec<T>) -> &[T] {
   &input

  // `input` is dropped here
  // making any reference to it invalid due to lifetime issue
}

So there is no way to implement From<Vec<T>> for &[] in Rust.
The way to go from one to the other is through reference. Because Vec<T> implements AsRef<[T]>, &Vec<T> can be coerced into &[T].

In Cairo, things are different because a snapshot can outlive the value it was created from.
Meaning you can do the following:

let snap = my_vec.snap();
drop(my_vec);

// snap is still valid here because, due to the immutable memory, his lifetime is `'static`
snap.pop_next();

So an Into implementation is totally valid. It will create a snapshot of the array and drop the initial array in the same call. Nothing prohibited there. It's a valid type conversion, doing exactly what it advertises "consume an array and return a snapshot of it"

[tdelabro]

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @orizi)

---

corelib/src/array.cairo line 161 at r1 (raw file):

        arr
    }
}

Done.

[orizi]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @tdelabro)

---

corelib/src/array.cairo line 151 at r1 (raw file):

Previously, tdelabro (Timothée Delabrouille) wrote…

tldr: there is not an exact equivalence between rust &[] and Cairo snapshots.

In Rust the implementation would not compile:

// input is passed by value
fn into(input: Vec<T>) -> &[T] {
   &input

  // `input` is dropped here
  // making any reference to it invalid due to lifetime issue
}

So there is no way to implement From<Vec<T>> for &[] in Rust.
The way to go from one to the other is through reference. Because Vec<T> implements AsRef<[T]>, &Vec<T> can be coerced into &[T].

In Cairo, things are different because a snapshot can outlive the value it was created from.
Meaning you can do the following:

let snap = my_vec.snap();
drop(my_vec);

// snap is still valid here because, due to the immutable memory, his lifetime is `'static`
snap.pop_next();

So an Into implementation is totally valid. It will create a snapshot of the array and drop the initial array in the same call. Nothing prohibited there. It's a valid type conversion, doing exactly what it advertises "consume an array and return a snapshot of it"

So conversion into a span should be done through something closer to the deref mechanism (currently being added, so probably not here)

[tdelabro]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @orizi)

---

corelib/src/array.cairo line 151 at r1 (raw file):

Previously, orizi wrote…

So conversion into a span should be done through something closer to the deref mechanism (currently being added, so probably not here)

The fact that this is a totally valid way to create the object makes me think that it should be implemented.

Otherwise, it hurts the ability of the programmers to write useful generic traits.

Any trait that has a +Into<Span<T>> bound should accept an Array<T> as a generic type, just for the sake of completeness. The opposite should be true too.

In Rust it doesn't make sense because references cannot outlive the value they are referencing. Cairo is entirely different in this regard and there is no reason to impair programmers for the sake of respecting constraints that only exist in another language.

Rust references and Cairo snapshots are just two different things, with different limitations, leading to different possibilities of usage.

[orizi]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tdelabro)

a discussion (no related file):
Add tests in array_test.cairo

---

[tdelabro]

Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @orizi)

a discussion (no related file):

Previously, orizi wrote…

Add tests in array_test.cairo

Done.

---

[orizi]

Reviewed all commit messages.
Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @tdelabro)

---

corelib/src/test/array_test.cairo line 180 at r3 (raw file):

}

mod into_trait_impls {

remove the outer module.
prefix all test names with test_.

---

corelib/src/test/array_test.cairo line 217 at r3 (raw file):

        assert_eq!(array_snap, @array![1, 2, 3]); 
    }

Suggestion:

    #[test]
    fn array_into_span() {
        assert_eq!(array![1, 2, 3].into(), array![1, 2, 3].span())
    }


    #[test]
    fn span_into_array() {
        assert_eq!(array![1, 2, 3].span().into(), array![1, 2, 3]);
    }

    #[test]
    fn array_snap_into_span() {
        assert_eq!((@array![1, 2, 3]).into(), array![1, 2, 3].span())
    }

    #[test]
    fn span_into_array_snap() {
        assert_eq!(array![1, 2, 3].span().into(), @array![1, 2, 3]);
    }

[tdelabro]

Now the question is about, how relevant it is to have both Into<_, Span> and ToSpanTrait in the corelib.

https://github.com/starkware-libs/cairo/blob/main/corelib/src/array.cairo#L241-L252

ToSpanTrait just sounds like a specialized subset of Into. Is it really relevant to keep it?

From a developer POV I would favorite keeping Into<_, Span> because it is coherent with the rest of the language design.
I'm not supposed to have a specialized trait for each type I want to convert into, I just use Into for all of them. Or we have to explain why Span is so special, but right now it's just a struct, not a primitive of the language, so I would prioritize global coherence and go with Into.
What was the initial reason to introduce ToSpanTrait instead of using Into in the first place @orizi?

In the meantime, I added this:

impl SnapIntoSpanWhereToSpanTrait<C, T, +ToSpanTrait<C, T>> of Into<@C, Span<T>> {
    fn into(self: @C) -> Span<T> {
        self.span()
    }
} 

This provides us with more generic SnapIntoSpan Impl, which provides one for Array too.

[orizi]

Reviewed 1 of 2 files at r4, all commit messages.
Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @tdelabro)

---

corelib/src/test/array_test.cairo line 219 at r4 (raw file):

fn tets_span_into_array_snap() {
    assert_eq!(@array![1, 2, 3], array![1, 2, 3].span().into()); 
}

Suggestion:

fn test_array_into_span() {
    assert_eq!(array![1, 2, 3].span(), array![1, 2, 3].into())
}

#[test]
fn test_span_into_array() {
    assert_eq!(array![1, 2, 3], array![1, 2, 3].span().into());
}

#[test]
fn test_array_snap_into_span() {
    assert_eq!(array![1, 2, 3].span(), (@array![1, 2, 3]).into());
}

#[test]
fn test_span_into_array_snap() {
    assert_eq!(@array![1, 2, 3], array![1, 2, 3].span().into());
}

[tdelabro]

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @orizi)

---

corelib/src/test/array_test.cairo line 219 at r4 (raw file):

fn tets_span_into_array_snap() {
    assert_eq!(@array![1, 2, 3], array![1, 2, 3].span().into()); 
}

Done.

[orizi]

Reviewed 1 of 2 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @tdelabro)

---

corelib/src/array.cairo line 161 at r5 (raw file):

        arr
    }
}

Suggestion:

impl SpanIntoArray<T, +Drop<T>, +Clone<T>> of Into<Span<T>, Array<T>> {
    fn into(self: Span<T>) -> Array<T> {
        let mut arr = array![];
        arr.append_span(self);
        arr
    }
}

[orizi]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @tdelabro)

a discussion (no related file):
@gilbens-starkware for 2nd eye.

---

[tdelabro]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @orizi)

---

corelib/src/array.cairo line 161 at r5 (raw file):

        arr
    }
}

Done.

[orizi]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tdelabro)

[orizi]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tdelabro)

[gilbens-starkware]

Reviewed 1 of 1 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tdelabro)

[orizi]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tdelabro)

[tdelabro]

@orizi fmt done, ready to merge

[orizi]

Reviewed 2 of 2 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tdelabro)