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

Tracking Issue for slice_flatten #95629

Closed
4 of 5 tasks
Cyborus04 opened this issue Apr 4, 2022 · 39 comments · Fixed by #134995
Closed
4 of 5 tasks

Tracking Issue for slice_flatten #95629

Cyborus04 opened this issue Apr 4, 2022 · 39 comments · Fixed by #134995
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Cyborus04
Copy link
Contributor

Cyborus04 commented Apr 4, 2022

These methods stabilized in #125561 for rust 1.80 🎉

This issue is now tracking their const-stability.


Feature gate: #![feature(slice_flatten)]

This is a tracking issue for the methods flatten and flatten_mut on [[T; N]], and into_flattened on Vec<[T; N], A>.

Public API

// core::slice

impl<T, const N: usize> [[T; N]] {
    pub fn as_flattened(&self) -> &[T];
    pub fn as_flattened_mut(&mut self) -> &mut [T];
}

// alloc::vec

impl<T, A: Allocator, const N: usize> Vec<[T; N], A> {
    pub fn into_flattened(self) -> Vec<T, A>;
}

Steps / History

Unresolved Questions

  • Are these the best possible names?
@Cyborus04 Cyborus04 added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Apr 4, 2022
@CryZe
Copy link
Contributor

CryZe commented Apr 4, 2022

This might pose a similar problem to array/slice into_iter in that array might get a flatten as well, so we may need to be careful when stabilizing this as slice::flatten.

@Cyborus04
Copy link
Contributor Author

Cyborus04 commented Apr 4, 2022

Adding &[[T; M]; N] -> &[T; N * M] would change the type [[1, 2, 3], [4, 5, 6]].flatten() returns, but would that actually be a problem? Anywhere it was expected to be a slice, it could simply deref/coerce into one.

@wwylele
Copy link
Contributor

wwylele commented Apr 4, 2022

I don't think we should rely on coercion for this. In more complicated code that relies on type inference, it might not be able to correctly coerce from array to slice.

I propose we rename this to flatten_slice, and have flatten_array for array

@Cyborus04
Copy link
Contributor Author

Cyborus04 commented Apr 4, 2022

Hmm, true.

Another point: the case [[(); usize::MAX] usize::MAX] would likely turn into a compile-time error rather than a panic, which would technically be a breaking change, if a rather niche one.

I like those names, the added explicitness is nice. For the mutable variants, flatten_slice_mut &flatten_array_mut, or flatten_mut_slice & flatten_mut_array? I prefer the flatten_*_mut ones.

@scottmcm
Copy link
Member

scottmcm commented Apr 8, 2022

We could also use into_flattened for the array version, like we currently have for the Vec version. That way it'd be distinct from the slice versions. Looking at https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv , maybe as_flattened(), as_flattened_mut(), and .into_flattened().

That'd also address feedback like #61680 (comment) that calling this flatten is confusing since it's not exactly the same as Iterator::flatten (though I'm not convinced by that concern, personally). And it'd fit with .as_chunks(), which is the inverse method on slices.

@Cyborus04
Copy link
Contributor Author

We could also use into_flattened for the array version, like we currently have for the Vec version.

That would work for [[T; N]; M] -> [T; N * M] (indeed, I would like to include that), would it for &[[T; N]; M] -> &[T; N * M]?

maybe as_flattened(), as_flattened_mut(), and .into_flattened().

I like that idea, seems more consistent that way

@leonardo-m
Copy link

A sufficiently common use case I've found for flatten_mut it to reset 2D matrices:

let mut m = [[1, 2, 3], [4, 5, 6]];
m.flatten_mut().fill(0);

@Elarcis
Copy link
Contributor

Elarcis commented Nov 17, 2022

In the case of generating a flattened slice from an array/vector, I will add a case for .flat_slice() and .flat_slice_mut().

  • The name starts with the keyword one would expect while trying to flatten their type
  • It’s short enough to write without losing meaning
  • It mentions that it creates a slice so no ambiguity
  • It doesn’t look too much like .flatten() which is something I’d expect on iterators, and which works differently

Edit: or .as_flat_slice() and .as_flat_mut_slice() for coherence with .as_slice() and .as_mut_slice().

@Cyborus04
Copy link
Contributor Author

That would make the names for equivalent array methods easier too: .{as_}flat_array(), .{as_}flat_array_ref(), and .{as_}flat_array_mut()

@mina86
Copy link
Contributor

mina86 commented Dec 17, 2022

Just to stir things up a little, ;) was an alternative/additional From<&[[T; N]]> for &[T] considered? I think it would solve array problem in the sense that once From<&[[T; N]; M]> for &[T; N*M] and From<[[T; N]; M]> for [T; N*M] become possible they could be added as well without having to invent new function name or affecting existing code. Though admittedly usage might be more convoluted in places where type inference cannot help.

@scottmcm
Copy link
Member

@mina86 Personally, the thing I like most about .flatten() is that it's not so generic. When I call .flatten() I don't need to turbo-fish it, and I know exactly what I'm going to get.

With .into() I might just get the &[[T; N]] back out again unless I'm extra careful. And I also really don't want to type <&[T]>::from(x) most of the time.

There's also the potential issue that it can panic when T is a ZST. It's kinda contrived, but generally Froms try not to do that. (The obvious exception is allocation failure, but that feels pretty different from this O(1) thing.)

@nvzqz
Copy link
Contributor

nvzqz commented Aug 9, 2023

I created #114676 but just realized this feature also exists. Perhaps it would make sense to rename this to flatten_ref or flatten_slice?

@Cyborus04
Copy link
Contributor Author

Perhaps it would make sense to rename this to flatten_ref or flatten_slice?

I'm not sure I see why? If to not conflict with [[T; N]; M] -> [T; N * M], that feature could be named array_flatten.

@nvzqz
Copy link
Contributor

nvzqz commented Aug 9, 2023

It would conflict in code that has x: [[T; N]; M] where x.flatten() goes from returning an IntoIterator of references to an IntoIterator of values.

@nvzqz
Copy link
Contributor

nvzqz commented Aug 9, 2023

Name conflicts aside, in a future where we have [[T; N]; M] -> [T; N * M], I think we would also want:

  • &[[T; N]; M] -> &[T; N * M]
  • &mut [[T; N]; M] -> &mut [T; N * M]

(which appears to already have been discussed)

@scottmcm
Copy link
Member

scottmcm commented Aug 9, 2023

Vec has the same "I deref to a slice but also want an owned version of this" problem as arrays, which currently is solving that with the name into_flattened.

@Cyborus04
Copy link
Contributor Author

Cyborus04 commented Aug 9, 2023

Oh, did you mean rename the method? I thought you meant the unstable feature name. If that's the case, then yes, I would like to rename the method as discussed before. I haven't yet as I haven't been sure what to rename to exactly.

@nvzqz
Copy link
Contributor

nvzqz commented Aug 10, 2023

I'm in favor of the following names:

impl<T, A: Allocator, const N: usize> Vec<[T; N], A> {
    pub fn into_flattened(self) -> Vec<T, A>;
}

impl<T, const N: usize, const M: usize> [[T; N]; M] {
    pub fn into_flattened(self) -> [T; N * M];
    pub fn as_flattened(&self) -> &[T; N * M];
    pub fn as_flattened_mut(&mut self) -> &mut [T; N * M];
}

impl<T, const N: usize> [[T; N]] {
    pub fn as_flattened(&self) -> &[T];
    pub fn as_flattened_mut(&mut self) -> &mut [T];
}

@Cyborus04
Copy link
Contributor Author

I like those names too, but adding the array methods later would change the behavior of [[1, 2], [3, 4]].as_flattened().

@nvzqz
Copy link
Contributor

nvzqz commented Aug 11, 2023

Given that arrays deref to slices, I don't think adding array-reference flattening later would break any existing code that uses slice flattening. The only case I can think of would be out-of-bounds indexing becoming a compile error.

@mina86
Copy link
Contributor

mina86 commented Aug 11, 2023

There's also the potential issue that it can panic when T is a ZST. It's kinda contrived, but generally Froms try not to do that. (The obvious exception is allocation failure, but that feels pretty different from this O(1) thing.)

Why would ZST cause panics?

@scottmcm
Copy link
Member

Why would ZST cause panics?

See "Panics" section in https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.flatten.

@Xiretza
Copy link
Contributor

Xiretza commented Aug 13, 2023

I like those names too, but adding the array methods later would change the behavior of [[1, 2], [3, 4]].as_flattened().

The array methods can be added now with an incorrect signature, which allows the slice methods to be stabilised first. Once const generics are good enough to allow the actual array methods, the signatures can be fixed and the array methods stabilized.

@scottmcm scottmcm added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label May 4, 2024
@scottmcm
Copy link
Member

scottmcm commented May 4, 2024

Hello libs-api folks! Looking at "wants no unsafe code (outside the TCB)" https://github.com/QuState/PhastFT/ reminded me of this issue and as_chunks.

Since this doesn't have the "but N == 0 is a divide-by-zero" problems that as_chunks (#74985) does, would you be willing to stabilize any or all of these as-is? That means three functions:
https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.flatten for &[[T;N]] -> &[T]
https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.flatten_mut for &mut [[T;N]] -> &mut [T]
https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#method.into_flattened for Vec<[T; N]> -> Vec<T>

These are all fallible except for the edge case of ZSTs, where [[(); 2]; usize::MAX].flatten() (and similar) has to panic for capacity overflow. Personally I think that's fine, because I suspect only contrived examples will ever hit it. For all the normal cases -- like [u8; 3] or [f32; 4] -- it's impossible to hit the panic because the input existing means the output will fit in isize::MAX bytes.

Naming feedback welcome as well. It looks like this issue predates ACPs by a couple months, so I don't know if you've ever seen it.

One thing that will probably come up is "but what about safe transmute?" I think (elaborated in #95629 (comment) ) that this would be nice to have even with safe-transmute because it gets only the natural output type, and does so automatically. Whereas safe-transmute will let me [[f32; 2]] -> [u16] -- it's perfectly safe -- even though that's almost certainly not what I wanted.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 18, 2025
…en, r=Amanieu

Stabilize const_slice_flatten

Const-stabilizes `slice::as_flattened{,_mut}`:
```rust
// core::slice
impl<T, const N: usize> [[T; N]] {
    pub const fn as_flattened(&self) -> &[T];
    pub const fn as_flattened_mut(&mut self) -> &mut [T];
}
```

Tracking issue: rust-lang#95629

Requires separate FCP, as per rust-lang#95629 (comment).

Closes rust-lang#95629.

``@rustbot`` modify labels: +T-libs-api

Happy new year!
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 18, 2025
…en, r=Amanieu

Stabilize const_slice_flatten

Const-stabilizes `slice::as_flattened{,_mut}`:
```rust
// core::slice
impl<T, const N: usize> [[T; N]] {
    pub const fn as_flattened(&self) -> &[T];
    pub const fn as_flattened_mut(&mut self) -> &mut [T];
}
```

Tracking issue: rust-lang#95629

Requires separate FCP, as per rust-lang#95629 (comment).

Closes rust-lang#95629.

```@rustbot``` modify labels: +T-libs-api

Happy new year!
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 18, 2025
…en, r=Amanieu

Stabilize const_slice_flatten

Const-stabilizes `slice::as_flattened{,_mut}`:
```rust
// core::slice
impl<T, const N: usize> [[T; N]] {
    pub const fn as_flattened(&self) -> &[T];
    pub const fn as_flattened_mut(&mut self) -> &mut [T];
}
```

Tracking issue: rust-lang#95629

Requires separate FCP, as per rust-lang#95629 (comment).

Closes rust-lang#95629.

````@rustbot```` modify labels: +T-libs-api

Happy new year!
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 19, 2025
…en, r=Amanieu

Stabilize const_slice_flatten

Const-stabilizes `slice::as_flattened{,_mut}`:
```rust
// core::slice
impl<T, const N: usize> [[T; N]] {
    pub const fn as_flattened(&self) -> &[T];
    pub const fn as_flattened_mut(&mut self) -> &mut [T];
}
```

Tracking issue: rust-lang#95629

Requires separate FCP, as per rust-lang#95629 (comment).

Closes rust-lang#95629.

`````@rustbot````` modify labels: +T-libs-api

Happy new year!
@bors bors closed this as completed in 7de250f Feb 19, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 19, 2025
Rollup merge of rust-lang#134995 - DaniPopes:stable-const_slice_flatten, r=Amanieu

Stabilize const_slice_flatten

Const-stabilizes `slice::as_flattened{,_mut}`:
```rust
// core::slice
impl<T, const N: usize> [[T; N]] {
    pub const fn as_flattened(&self) -> &[T];
    pub const fn as_flattened_mut(&mut self) -> &mut [T];
}
```

Tracking issue: rust-lang#95629

Requires separate FCP, as per rust-lang#95629 (comment).

Closes rust-lang#95629.

`````@rustbot````` modify labels: +T-libs-api

Happy new year!
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this issue Feb 20, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this issue Feb 20, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to carolynzech/rust that referenced this issue Feb 20, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this issue Feb 20, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Feb 21, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Feb 21, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this issue Feb 22, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this issue Feb 22, 2025
Const-stabilizes `slice::as_flattened{,_mut}`:
```rust
// core::slice
impl<T, const N: usize> [[T; N]] {
    pub const fn as_flattened(&self) -> &[T];
    pub const fn as_flattened_mut(&mut self) -> &mut [T];
}
```

Tracking issue: rust-lang#95629

Requires separate libs-api FCP, as per rust-lang#95629 (comment).

Closes rust-lang#95629.
github-actions bot pushed a commit to carolynzech/rust that referenced this issue Feb 22, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to carolynzech/rust that referenced this issue Feb 22, 2025
Const-stabilizes `slice::as_flattened{,_mut}`:
```rust
// core::slice
impl<T, const N: usize> [[T; N]] {
    pub const fn as_flattened(&self) -> &[T];
    pub const fn as_flattened_mut(&mut self) -> &mut [T];
}
```

Tracking issue: rust-lang#95629

Requires separate libs-api FCP, as per rust-lang#95629 (comment).

Closes rust-lang#95629.
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this issue Feb 22, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this issue Feb 22, 2025
Const-stabilizes `slice::as_flattened{,_mut}`:
```rust
// core::slice
impl<T, const N: usize> [[T; N]] {
    pub const fn as_flattened(&self) -> &[T];
    pub const fn as_flattened_mut(&mut self) -> &mut [T];
}
```

Tracking issue: rust-lang#95629

Requires separate libs-api FCP, as per rust-lang#95629 (comment).

Closes rust-lang#95629.
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Feb 22, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Feb 22, 2025
Const-stabilizes `slice::as_flattened{,_mut}`:
```rust
// core::slice
impl<T, const N: usize> [[T; N]] {
    pub const fn as_flattened(&self) -> &[T];
    pub const fn as_flattened_mut(&mut self) -> &mut [T];
}
```

Tracking issue: rust-lang#95629

Requires separate libs-api FCP, as per rust-lang#95629 (comment).

Closes rust-lang#95629.
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Mar 3, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Mar 3, 2025
Const-stabilizes `slice::as_flattened{,_mut}`:
```rust
// core::slice
impl<T, const N: usize> [[T; N]] {
    pub const fn as_flattened(&self) -> &[T];
    pub const fn as_flattened_mut(&mut self) -> &mut [T];
}
```

Tracking issue: rust-lang#95629

Requires separate libs-api FCP, as per rust-lang#95629 (comment).

Closes rust-lang#95629.
github-actions bot pushed a commit to carolynzech/rust that referenced this issue Mar 4, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to carolynzech/rust that referenced this issue Mar 4, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Mar 4, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Mar 4, 2025
Const-stabilizes `slice::as_flattened{,_mut}`:
```rust
// core::slice
impl<T, const N: usize> [[T; N]] {
    pub const fn as_flattened(&self) -> &[T];
    pub const fn as_flattened_mut(&mut self) -> &mut [T];
}
```

Tracking issue: rust-lang#95629

Requires separate libs-api FCP, as per rust-lang#95629 (comment).

Closes rust-lang#95629.
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this issue Mar 6, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this issue Mar 6, 2025
Const-stabilizes `slice::as_flattened{,_mut}`:
```rust
// core::slice
impl<T, const N: usize> [[T; N]] {
    pub const fn as_flattened(&self) -> &[T];
    pub const fn as_flattened_mut(&mut self) -> &mut [T];
}
```

Tracking issue: rust-lang#95629

Requires separate libs-api FCP, as per rust-lang#95629 (comment).

Closes rust-lang#95629.
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Mar 6, 2025
Tracking issue: rust-lang#95629

Unblocked by const_mut_refs being stabilized: rust-lang#129195
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Mar 6, 2025
Const-stabilizes `slice::as_flattened{,_mut}`:
```rust
// core::slice
impl<T, const N: usize> [[T; N]] {
    pub const fn as_flattened(&self) -> &[T];
    pub const fn as_flattened_mut(&mut self) -> &mut [T];
}
```

Tracking issue: rust-lang#95629

Requires separate libs-api FCP, as per rust-lang#95629 (comment).

Closes rust-lang#95629.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.