-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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 slice::array_chunks_mut
#75021
Add slice::array_chunks_mut
#75021
Conversation
I'm willing to do reverse versions too -- I also want to try |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't say much about the aliasing stuff in array_chunks_mut
, but the rest looks good.
Iirc @JulianKnodt wanted to look into implementing fn array_windows
I think rchunks might be better with a different feature gate and in a separate PR, don't really care that much though |
Yeah, I'm not certain that's the best way to avoid a mutable alias, but it seems simple enough.
OK, sure, I'll leave them to it. |
Afaik mutable aliases are fine as long as the aliasing mutable reference isn't used anymore, i.e. there is no ABAB access. Does not using that explicit scope cause MIRI to fail? |
I thought the mere existence of aliases was a problem, but I could be wrong. I haven't tried with miri yet. |
AFAICT, miri is fine with it either way. Should I simplify it then? |
i think so 😄 |
@withoutboats this is ready for review |
r? @scottmcm |
The code here looks right, and given that @bors r+ |
📌 Commit fca686f7e7da8fb971958b3928178cd229fa1620 has been approved by |
⌛ Testing commit fca686f7e7da8fb971958b3928178cd229fa1620 with merge 321229650bb03b6b0edc8fd1f5056dc6fee9f7a9... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-actions |
Looks like this might be a real failure:
@bors r- |
Review discussion found that the concern about aliasing was overblown, so we can simplify this to cast from one slice to another directly.
fca686f
to
86b9f71
Compare
Ah, needed to catch up with #73565 -- plus tidy seems picker about |
@bors r+ (Sorry for the delay here!) |
📌 Commit 86b9f71 has been approved by |
☀️ Test successful - checks-actions, checks-azure |
This follows
array_chunks
from #74373 with a mutable version,array_chunks_mut
. The implementation is identical apart from mutability. The new tests are adaptations of thechunks_exact_mut
tests, plus an inference test like the one forarray_chunks
.I reused the unstable feature
array_chunks
and tracking issue #74985, but I can separate that if desired.r? @withoutboats
cc @lcnr