Skip to content

Remove [T]::array_chunks(_mut) #143289

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Remove [T]::array_chunks(_mut) #143289

wants to merge 1 commit into from
+14 −530

Conversation

scottmcm
Copy link
Member

@scottmcm scottmcm commented Jul 1, 2025 •
edited by rustbot
Loading

Since libs-api is proposing as much in #74985 (comment)

Closes #74985
Closes #76354

@rustbot
Copy link
Collaborator

rustbot commented Jul 1, 2025

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 1, 2025
@scottmcm scottmcm added the S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. label Jul 1, 2025
@scottmcm scottmcm mentioned this pull request Jul 1, 2025
Open
7 tasks
@scottmcm scottmcm removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 1, 2025
scottmcm
scottmcm commented Jul 1, 2025
View reviewed changes
library/alloc/src/string.rs
Comment on lines -790 to 798
if v.len() % 2 != 0 {
let (chunks, []) = v.as_chunks::<2>() else {
return Err(FromUtf16Error(()));
}
};
match (cfg!(target_endian = "little"), unsafe { v.align_to::<u16>() }) {
(true, ([], v, [])) => Self::from_utf16(v),
_ => char::decode_utf16(v.array_chunks::<2>().copied().map(u16::from_le_bytes))
_ => char::decode_utf16(chunks.iter().copied().map(u16::from_le_bytes))
.collect::<Result<_, _>>()
.map_err(|_| FromUtf16Error(())),
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Hooray for let-else and slice patterns meaning this can write "the chunks with no tail", rather than needing to have an explicit % 2.

@dtolnay dtolnay added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 1, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Amanieu
Copy link
Member

Amanieu commented Jul 1, 2025

From #74985 (comment):

We discussed this again the @rust-lang/libs-api meeting and concluded that there is little use for an array_chunks iterator when we already have as_chunks. As such we would like to remove this unstable API.

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented Jul 1, 2025 •
edited by dtolnay
Loading

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.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jul 1, 2025
@jhpratt
Copy link
Member

jhpratt commented Jul 1, 2025

Let me know when CI is passing an I can take a look.

@scottmcm scottmcm force-pushed the remove-array-chunks branch from 9c9725c to 4814b93 Compare July 2, 2025 02:52
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@scottmcm scottmcm force-pushed the remove-array-chunks branch from 4814b93 to bc6d1c2 Compare July 2, 2025 03:25
@rustbot
Copy link
Collaborator

rustbot commented Jul 2, 2025

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

scottmcm
scottmcm commented Jul 2, 2025
View reviewed changes
compiler/rustc_codegen_cranelift/patches/0027-sysroot_tests-128bit-atomic-operations.patch
@@ -19,7 +19,7 @@ index 1e336bf..35e6f54 100644
-#![cfg_attr(target_has_atomic = "128", feature(integer_atomics))]
#![cfg_attr(test, feature(cfg_select))]
#![feature(alloc_layout_extra)]
#![feature(array_chunks)]
#![feature(array_ptr_get)]
Copy link
Member Author

Choose a reason for hiding this comment

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

Achievement Acquired: first CI failure from a .patch file.

@scottmcm
Copy link
Member Author

scottmcm commented Jul 2, 2025

@jhpratt This is passing CI now, if you want to take a look, but no rush since it can't land in less than 10 days anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jhpratt jhpratt

Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Does [T]::array_chunks really need to be an iterator? Tracking Issue for slice::array_chunks
7 participants
@scottmcm @rustbot @rust-log-analyzer @Amanieu @rfcbot @jhpratt @dtolnay