Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add array lang item and [T; N]::map(f: FnMut(T) -> S) #75212

Merged
merged 6 commits into from
Aug 13, 2020

Conversation

JulianKnodt
Copy link
Contributor

This introduces an array lang item so functions can be defined on top of [T; N]. This was previously not done because const-generics was not complete enough to allow for this. Now it is in a state that is usable enough to start adding functions.

The function added is a monadic (I think?) map from [T; N] -> [S; N]. Until transmute can function on arrays, it also allocates an extra temporary array, but this can be removed at some point.

r? @lcnr

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 6, 2020
library/core/src/array/mod.rs Outdated Show resolved Hide resolved
library/core/src/array/mod.rs Outdated Show resolved Hide resolved
@JulianKnodt JulianKnodt force-pushed the array_map branch 2 times, most recently from 6c237b8 to e961a6e Compare August 7, 2020 00:55
library/core/src/array/mod.rs Outdated Show resolved Hide resolved
library/core/src/array/mod.rs Outdated Show resolved Hide resolved
@JulianKnodt JulianKnodt force-pushed the array_map branch 4 times, most recently from 53846a6 to 664e456 Compare August 7, 2020 05:40
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation looks good to me, leaving this to:

  • @rust-lang/lang for the new lang item array which allows us to implement methods for [T; N].
  • @rust-lang/libs for the new unstable method fn array::map.

@JulianKnodt JulianKnodt mentioned this pull request Aug 7, 2020
4 tasks
@pickfire
Copy link
Contributor

pickfire commented Aug 7, 2020

Looks like this may lead to impl Iterator for [T; N].

Copy link
Contributor

@pickfire pickfire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I wonder if the doc changes requires doc team to review. CC @jyn514

@lcnr
Copy link
Contributor

lcnr commented Aug 7, 2020

Looks like this may lead to impl Iterator for [T; N].

I don't think we want to implement Iterator of arrays, but I am not really sure what you mean with "lead to" here. This seems more closely related to Option::map than Iterator.

We do want to implement IntoIterator(#65819) and FromIterator(#69985) for arrays but this method is still a lot more concise than arr.into_iter().map(func).collect::<Result<_, _>>().unwrap().

@JulianKnodt
Copy link
Contributor Author

@pickfire ah I'm not sure if impl Iterator for [T; N] makes sense and it's really only map that is useful. fold doesn't require outputting to a fixed size(vector -> scalar sort of thing) so it can just be used with an into_iter(), and other things such as filter cannot know the output size. map is one of the few that makes a lot of sense because if we use a normal iterator we have to try_convert or other shenanigans to fixed size, which is annoying, so I think the whole point of map is to get around that.

@bors
Copy link
Contributor

bors commented Aug 13, 2020

⌛ Testing commit af32db2 with merge a0c290f...

@bors
Copy link
Contributor

bors commented Aug 13, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: LukasKalbertodt
Pushing a0c290f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 13, 2020
@bors bors merged commit a0c290f into rust-lang:master Aug 13, 2020
@leonardo-m
Copy link

I can't find one use case for this in my codebase :-(

@JulianKnodt JulianKnodt deleted the array_map branch August 15, 2020 08:39
@JulianKnodt
Copy link
Contributor Author

Mm, one case that is particularly useful to me is in numerical computing and performing vectorized operations with fixed size arrays. In this case, it's nice to have a method that retains the size of the array but allows performing some operation on each item in the inner type.

I would keep it in mind when looking to operate on something that is a known size at compile that must be transmuted into something that is the same size. With that said, it's not useful for a lot of cases, because I imagine that if the code is not performance sensitive and/or must be dynamically allocated, it'll just use Vec, which this wouldn't apply to.

@scottmcm
Copy link
Member

@leonardo-m Given how annoying to use arrays have historically been, I'm not that surprised.

This is one of those things that gets more useful as new things that produce arrays start showing up -- for example, from the array_chunks(_mut) iterators introduced in #74373 and #75021.

@leonardo-m
Copy link

In my Nightly-based codebase I have probably few thousand arrays, tiny ones, small ones, and larger boxed ones. And currently in about 100% of the case I don't want to create a new array from another one, but read the contents of an array or but mutate an existing array.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 3, 2020
…-to-array, r=Mark-Simulacrum

Add `[T; N]::as_[mut_]slice`

Part of me trying to populate arrays with a couple of basic useful methods, like slices already have. The ability to add methods to arrays were added in rust-lang#75212.  Tracking issue: rust-lang#76118

This adds:

```rust
impl<T, const N: usize> [T; N] {
    pub fn as_slice(&self) -> &[T];
    pub fn as_mut_slice(&mut self) -> &mut [T];
}
```

These methods are like the ones on `std::array::FixedSizeArray` and in the crate `arraytools`.
@electroCutie
Copy link

The function added is a monadic (I think?)

It is not Monadic. That would be more like mapping from elements intro arrays and then concatenating them as the result
This appears to be a Functor

@lcnr lcnr added A-const-generics Area: const generics (parameters and arguments) F-const_generics `#![feature(const_generics)]` labels Nov 14, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 21, 2020
Added [T; N]::zip()

This is my first PR to rust so I hope I have done everything right, or at least close :)

---

This is PR adds the array method `[T; N]::zip()` which, in my mind, is a natural extension to rust-lang#75212.

My implementation of `zip()` is mostly just a modified copy-paste of `map()`. Should I keep the comments? Also am I right in assuming there should be no way for the `for`-loop to panic, thus no need for the dropguard seen in the `map()`-function?

The doc comment is in a similar way a slightly modified copy paste of [`Iterator::zip()`](https://doc.rust-lang.org/beta/std/iter/trait.Iterator.html#method.zip)

`@jplatte` mentioned in [rust-lang#75490](rust-lang#75490 (comment)) `zip_with()`,
> zip and zip_with seem like they would be useful :)

is this something I should add (assuming there is interest for this PR at all :))
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 22, 2020
Added [T; N]::zip()

This is my first PR to rust so I hope I have done everything right, or at least close :)

---

This is PR adds the array method `[T; N]::zip()` which, in my mind, is a natural extension to rust-lang#75212.

My implementation of `zip()` is mostly just a modified copy-paste of `map()`. Should I keep the comments? Also am I right in assuming there should be no way for the `for`-loop to panic, thus no need for the dropguard seen in the `map()`-function?

The doc comment is in a similar way a slightly modified copy paste of [`Iterator::zip()`](https://doc.rust-lang.org/beta/std/iter/trait.Iterator.html#method.zip)

``@jplatte`` mentioned in [rust-lang#75490](rust-lang#75490 (comment)) `zip_with()`,
> zip and zip_with seem like they would be useful :)

is this something I should add (assuming there is interest for this PR at all :))
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 22, 2020
Added [T; N]::zip()

This is my first PR to rust so I hope I have done everything right, or at least close :)

---

This is PR adds the array method `[T; N]::zip()` which, in my mind, is a natural extension to rust-lang#75212.

My implementation of `zip()` is mostly just a modified copy-paste of `map()`. Should I keep the comments? Also am I right in assuming there should be no way for the `for`-loop to panic, thus no need for the dropguard seen in the `map()`-function?

The doc comment is in a similar way a slightly modified copy paste of [`Iterator::zip()`](https://doc.rust-lang.org/beta/std/iter/trait.Iterator.html#method.zip)

```@jplatte``` mentioned in [rust-lang#75490](rust-lang#75490 (comment)) `zip_with()`,
> zip and zip_with seem like they would be useful :)

is this something I should add (assuming there is interest for this PR at all :))
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 22, 2020
Added [T; N]::zip()

This is my first PR to rust so I hope I have done everything right, or at least close :)

---

This is PR adds the array method `[T; N]::zip()` which, in my mind, is a natural extension to rust-lang#75212.

My implementation of `zip()` is mostly just a modified copy-paste of `map()`. Should I keep the comments? Also am I right in assuming there should be no way for the `for`-loop to panic, thus no need for the dropguard seen in the `map()`-function?

The doc comment is in a similar way a slightly modified copy paste of [`Iterator::zip()`](https://doc.rust-lang.org/beta/std/iter/trait.Iterator.html#method.zip)

`@jplatte` mentioned in [rust-lang#75490](rust-lang#75490 (comment)) `zip_with()`,
> zip and zip_with seem like they would be useful :)

is this something I should add (assuming there is interest for this PR at all :))
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 11, 2021
…ds, r=dtolnay

Add `[T; N]::each_ref` and `[T; N]::each_mut`

This PR adds the methods `each_ref` and `each_mut` to `[T; N]`. The ability to add methods to arrays was added in rust-lang#75212. These two methods are particularly useful with `map` which was also added in that PR. Tracking issue: rust-lang#76118

```rust
impl<T, const N: usize> [T; N] {
    pub fn each_ref(&self) -> [&T; N];
    pub fn each_mut(&mut self) -> [&mut T; N];
}
```
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) F-const_generics `#![feature(const_generics)]` merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.