-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Comments
This might pose a similar problem to |
Adding |
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 |
Hmm, true. Another point: the case I like those names, the added explicitness is nice. For the mutable variants, |
We could also use That'd also address feedback like #61680 (comment) that calling this |
That would work for
I like that idea, seems more consistent that way |
A sufficiently common use case I've found for flatten_mut it to reset 2D matrices:
|
In the case of generating a flattened slice from an array/vector, I will add a case for
Edit: or |
That would make the names for equivalent array methods easier too: |
Just to stir things up a little, ;) was an alternative/additional |
@mina86 Personally, the thing I like most about With There's also the potential issue that it can panic when T is a ZST. It's kinda contrived, but generally |
I created #114676 but just realized this feature also exists. Perhaps it would make sense to rename this to |
I'm not sure I see why? If to not conflict with |
It would conflict in code that has |
Name conflicts aside, in a future where we have
(which appears to already have been discussed) |
|
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. |
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];
} |
I like those names too, but adding the array methods later would change the behavior of |
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. |
Why would ZST cause panics? |
See "Panics" section in https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.flatten. |
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. |
Hello libs-api folks! Looking at "wants no unsafe code (outside the TCB)" https://github.com/QuState/PhastFT/ reminded me of this issue and Since this doesn't have the "but These are all fallible except for the edge case of ZSTs, where 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 |
We discussed this in the libs-api meeting and we are mostly happy with these functions, except that we would like an @rfcbot fcp merge |
Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
The |
Right, the names would be |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
Rename `flatten(_mut)` → `as_flattened(_mut)` As requested by libs-api in rust-lang#95629 (comment) (This is just the rename, not the stabilization, so can land without waiting on the FCP in that other issue.)
Rollup merge of rust-lang#125171 - scottmcm:rename-flatten, r=jhpratt Rename `flatten(_mut)` → `as_flattened(_mut)` As requested by libs-api in rust-lang#95629 (comment) (This is just the rename, not the stabilization, so can land without waiting on the FCP in that other issue.)
Rename has landed: #125171 |
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
Yay! |
Want to make the stabilization PR, @Cyborus04 ? |
Is there a reason why |
@nickmertin Functions taking |
Yes I understand that, but it is possible on nightly with a feature. Other APIs taking |
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
andflatten_mut
on[[T; N]]
, andinto_flattened
onVec<[T; N], A>
.Public API
Steps / History
<[[T; N]]>::flatten{_mut}
#95579flatten(_mut)
→as_flattened(_mut)
#125171slice_flatten
#125561<[T]>::as_flattened
constUnresolved Questions
Are these the best possible names?The text was updated successfully, but these errors were encountered: