-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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::array_chunks #74985
Comments
Add `slice::array_chunks_mut` This follows `array_chunks` from rust-lang#74373 with a mutable version, `array_chunks_mut`. The implementation is identical apart from mutability. The new tests are adaptations of the `chunks_exact_mut` tests, plus an inference test like the one for `array_chunks`. I reused the unstable feature `array_chunks` and tracking issue rust-lang#74985, but I can separate that if desired. r? `@withoutboats` cc `@lcnr`
Add array_windows fn This mimicks the functionality added by array_chunks, and implements a const-generic form of `windows`. It makes egregious use of `unsafe`, but by necessity because the array must be re-interpreted as a slice of arrays, and unlike array_chunks this cannot be done by casting the original array once, since each time the index is advanced it needs to move one element, not `N`. I'm planning on adding more tests, but this should be good enough as a premise for the functionality. Notably: should there be more functions overwritten for the iterator implementation/in general? ~~I've marked the issue as rust-lang#74985 as there is no corresponding exact issue for `array_windows`, but it's based of off `array_chunks`.~~ Edit: See Issue rust-lang#75027 created by @lcnr for tracking issue ~~Do not merge until I add more tests, please.~~ r? @lcnr
…lbertodt Add [T]::as_chunks(_mut) Allows getting the slices directly, rather than just through an iterator as in `array_chunks(_mut)`. The constructors for those iterators are then written in terms of these methods, so the iterator constructors no longer have any `unsafe` of their own. Unstable, of course. rust-lang#74985
Is there any intention of providing the reverse versions of these functions? |
What would that look like? |
Same API, but iterating backwards. I have a use case in which I need to convert either a prefix or a suffix of a slice to an array reference. This API is perfect for the prefix use case, and I'd love to be able to use it for the suffix use case as well. |
@joshlf |
That's only true if |
Ah, I misunderstood your use case, you want |
Yep, exactly 😃 |
Yeah, array_rchunks is definitely worth having. |
Also useful would be the inverse operation: |
Do we need to wait until |
Hi, is there any alternatives before this is stable? Thanks! |
Yeah |
Thanks! |
Looks like that checking of
In other words, it seems that officially runtime checks are preferable over compile-time checks so can we proceed with stabilization? |
Permitting something to compile knowing with 100% certainty that it will panic seems like an odd decision to make, given that we have the capability to prevent it. Post-monomorphization errors aren't ideal, but surely it's better than an unconditional runtime panic? |
The problem is that it's not "100% certainty that it will panic". There was an example in another thread (#99471 (comment)) of code that is perfectly legal, but adding post-monomorphization error makes it fail to compile: fn f<const N: usize>() {
if N == 0 {
...
} else {
foo.as_chunks::<N>()...
}
}
fn main() {
f::<0>();
} |
Ah, that makes sense. So unless we can solve the halting problem, a runtime error it is… |
If the choice is between stabilising it as is with a runtime error, and keeping it experimental until |
I haven't found the time to work on it for the last few weeks, but I think my pre-RFC for pattern types has a good solution for this when combined with rust-lang/project-const-generics#26. |
This comment was marked as spam.
This comment was marked as spam.
Now that inline const is stabilized, we should be able to make compilation fail when |
I think the lang team has made it clear repeatedly that they prefer runtime panics over post-mono errors. Personally I don't agree, but I don't think it's worth arguing over anymore. |
I don't agree either, but it's also not up to me. This issue has it crossed out, but that doesn't appear to be a formal decision in either direction. The docs currently say
So it seems to remain an open question? |
A reduced example that should compile according to the responsible team. pub fn foo<const N: usize>() -> i32 {
const {
assert!(N != 0);
}
1
}
pub fn bar<const N: usize>() -> i32 {
if N == 0 {
2
} else {
foo::<N>()
}
}
fn main() {
assert_eq!(bar::<0>(), 2);
} Maybe something about the PR that unblocked inline constants (#122568) could help here? There are then 4 scenarios for stabilization:
cc rust-lang/rfcs#3582 |
Thanks for that example. I've added it to the top comment. |
I haven't seen it discussed here yet, but another way to tackle the case of |
That would mean the iterator can't be If people want |
On the subject of the "what should happen if the const argument is 0" debate, I'd like to present what I think is an analogous situation: fn divide<const D: u32>(n: u32) -> u32 {
n / D
}
fn divide_checked<const D: u32>(n: u32) -> Option<u32> {
if D == 0 {
None
} else {
Some(n / D)
}
}
fn main() {
dbg!(divide_checked::<5>(99));
dbg!(divide_checked::<0>(99));
dbg!(divide::<5>(99));
dbg!(divide::<0>(99));
} The current behaviour of Rust in this situation is to compile with no errors or warnings (not even on the call to Meanwhile, if you just write I don't think the situation of, e.g., It's also worth noting an edge case: let fn_pointer = <[u32]>::as_chunks::<0>; Do we want to allow this, or not? With the explicit |
The remaining issue here is what to do on #74985 (comment) it is mentioned above that apparently the lang team prefers runtime panics over post-mono errors, but I don't see any links to discussion about this or justification for why. Runtime errors do have the advantage that they won't be triggered in dead code. The arguments for making it a post-mono error are clear: getting errors earlier (and at all if the code is not tested). Post mono errors won't error in Other comments above make additional arguments in favor of post-mono errors, and the documentation of the methods also intended to make it a post-mono error eventually. |
@rustbot labels +I-lang-nominated I'm not sure whether what was mentioned about our lang preferences is actually true, so let's nominate to discuss. cc @rust-lang/lang |
I don't remember what exactly the argument against post-mono errors was but I do remember thinking "that does makes sense", even though I prefer an earlier error. I want to say @BurntSushi may have originally expressed the concern? If we do go with the runtime panic, we should document that a future Rust version is allowed to reject |
I think #74985 (comment) is a good example why a runtime check is desirable, so users can be flexible in their own APIs around this. If I write |
That’s current limitation in expressiveness of the language, no? I can easily imagine: impl [T] {
fn as_chunks<const N: NonZeroUsize>() -> (&[[T; N]], &[T]);
}
fn foo<const N: usize>(slice: &[T]) -> Option<i32> {
let chunks = const if let Some(N) = NonZeroUsize::new(N) {
slice.as_chunks::<N>().0;
} else {
return None;
};
todo!()
} That would require a few things from the language:
To me it seems that the question is whether we want to wait for all that or stabilise. |
Maybe neither here nor there, but note that the following works: const fn divide<const D: u64>(n: u64) -> u64 {
const { assert!(D > 0) }; // <--- Post-mono assertion.
n / D
}
const fn divide_checked<const D: u64>(n: u64) -> Option<u64> {
match D {
0 => None,
1.. => Some(divide::<D>(n)),
}
}
fn main() {
dbg!({const { divide_checked::<1>(1) }});
dbg!({const { divide_checked::<0>(1) }}); // <--- Look here.
dbg!(divide_checked::<1>(1));
} That is, if the function within which we want to compile-time assert is parametric over a type or const generic, is const, and we're always in a const context when we'd see the "bad" generic argument for it, then something like what we want here does work currently. (The question of whether we'll guarantee to not later disallow this is #122301.) Interestingly, this is the odd case of a call that we allow in a const context but disallow outside of one. |
The feature gate for the issue is
#![feature(array_chunks)]
.Also tracks
as_(r)chunks(_mut)
in#![feature(slice_as_chunks)]
.About tracking issues
Tracking issues are used to record the overall progress of implementation.
They are also uses as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.
Steps
RestrictMakeN
to values greater than 0 at compile time instead of using a runtime panic.<[T]>::array_*
methods fail to compile on 0 len arrays #99471N == 0
is unreachable due to conditionals Tracking Issue for slice::array_chunks #74985 (comment)Unresolved Questions
[T]::array_chunks
really need to be an iterator? #76354Implementation history
slice::array_chunks
to std #74373slice::array_chunks_mut
#75021as_chunks{_mut}
in Add [T]::as_chunks(_mut) #76635as_rchunks{_mut}
and unchecked versions in Addas_rchunks
(and friends) to slices #78818as_(r)chunk
in constifyslice_as_chunks
(unstable) #111453The text was updated successfully, but these errors were encountered: