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.

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

Merged
merged 1 commit into from
Jul 29, 2025
Merged

Conversation

scottmcm
Copy link
Member

@scottmcm scottmcm commented Jul 1, 2025

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

Closes #74985
Closes #76354

try-job: dist-various-1
try-job: dist-various-2

@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 removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 1, 2025
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.

@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

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.

@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

@@ -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.

@jhpratt
Copy link
Member

jhpratt commented Jul 2, 2025

LGTM! r=me if/when FCP is complete.

@scottmcm
Copy link
Member Author

Friendly reminder for the p-FCP here (#143289 (comment)), @joshtriplett @m-ou-se @BurntSushi

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Jul 17, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented Jul 17, 2025

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jul 27, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented Jul 27, 2025

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.

bors added a commit that referenced this pull request Jul 27, 2025
Rollup of 7 pull requests

Successful merges:

 - #143289 (Remove `[T]::array_chunks(_mut)`)
 - #143607 (Port the proc macro attributes to the new attribute parsing infrastructure)
 - #144471 (Remove `compiler-builtins-{no-asm,mangled-names}`)
 - #144495 (bump cargo_metadata)
 - #144523 (rustdoc: save target modifiers)
 - #144534 (check_static_item: explain should_check_for_sync choices)
 - #144535 (miri: for ABI mismatch errors, say which argument is the problem)

Failed merges:

 - #144536 (miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors r-
#144546 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 27, 2025
@scottmcm scottmcm force-pushed the remove-array-chunks branch from bc6d1c2 to 84919a4 Compare July 28, 2025 05:58
@rustbot

This comment has been minimized.

@scottmcm scottmcm force-pushed the remove-array-chunks branch from 84919a4 to 173926d Compare July 28, 2025 06:04
@scottmcm
Copy link
Member Author

Trying the weird platforms that failed in the rollup...
@bors try

@rust-bors
Copy link

rust-bors bot commented Jul 28, 2025

⌛ Trying commit 173926d with merge 653efc4

To cancel the try build, run the command @bors try cancel.

rust-bors bot added a commit that referenced this pull request Jul 28, 2025
Remove `[T]::array_chunks(_mut)`

try-job: dist-various-1
try-job: dist-various-2
@rust-bors
Copy link

rust-bors bot commented Jul 28, 2025

☀️ Try build successful (CI)
Build commit: 653efc4 (653efc42453b365d6cb04f21b38f4017de2dc3fa, parent: 733dab558992d902d6d17576de1da768094e2cf3)

@scottmcm
Copy link
Member Author

Well the job that failed in the rollup passed, so let's give it another shot
@bors r=jhpratt rollup=maybe (failed before in platform-specific library code)

@bors
Copy link
Collaborator

bors commented Jul 28, 2025

📌 Commit 173926d has been approved by jhpratt

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 28, 2025
samueltardieu added a commit to samueltardieu/rust that referenced this pull request Jul 28, 2025
…pratt

Remove `[T]::array_chunks(_mut)`

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

Closes rust-lang#74985
Closes rust-lang#76354

try-job: dist-various-1
try-job: dist-various-2
bors added a commit that referenced this pull request Jul 29, 2025
Rollup of 11 pull requests

Successful merges:

 - #143289 (Remove `[T]::array_chunks(_mut)`)
 - #143883 (Add `--link-targets-dir` argument to linkchecker)
 - #144034 (tests: Test line number in debuginfo for diverging function calls)
 - #144236 (Add `core::mem::DropGuard`)
 - #144268 (Add method `find_ancestor_not_from_macro` and `find_ancestor_not_from_extern_macro` to supersede `find_oldest_ancestor_in_same_ctxt`)
 - #144303 (Consolidate staging for `rustc_private` tools)
 - #144539 (constify with_exposed_provenance)
 - #144569 (rustc-dev-guide subtree update)
 - #144573 (Raw Pointers are Constant PatKinds too)
 - #144578 (Ensure correct aligement of rustc_hir::Lifetime on platforms with lower default alignments.)
 - #144582 (fix `Atomic*::as_ptr` wording)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Collaborator

bors commented Jul 29, 2025

⌛ Testing commit 173926d with merge cb6785f...

@bors
Copy link
Collaborator

bors commented Jul 29, 2025

☀️ Test successful - checks-actions
Approved by: jhpratt
Pushing cb6785f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 29, 2025
@bors bors merged commit cb6785f into rust-lang:master Jul 29, 2025
12 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 29, 2025
Copy link
Contributor

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing cdccba8 (parent) -> cb6785f (this PR)

Test differences

Show 347 test diffs

Stage 1

  • slice::test_array_chunks_count: pass -> [missing] (J0)
  • slice::test_array_chunks_infer: pass -> [missing] (J0)
  • slice::test_array_chunks_last: pass -> [missing] (J0)
  • slice::test_array_chunks_mut_count: pass -> [missing] (J0)
  • slice::test_array_chunks_mut_infer: pass -> [missing] (J0)
  • slice::test_array_chunks_mut_last: pass -> [missing] (J0)
  • slice::test_array_chunks_mut_nth: pass -> [missing] (J0)
  • slice::test_array_chunks_mut_nth_back: pass -> [missing] (J0)
  • slice::test_array_chunks_mut_remainder: pass -> [missing] (J0)
  • slice::test_array_chunks_mut_zip: pass -> [missing] (J0)
  • slice::test_array_chunks_nth: pass -> [missing] (J0)
  • slice::test_array_chunks_nth_back: pass -> [missing] (J0)
  • slice::test_array_chunks_remainder: pass -> [missing] (J0)
  • slice::test_array_chunks_zip: pass -> [missing] (J0)

Additionally, 333 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard cb6785f73df1aa3f558796a22a4ab9652cf38e26 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. dist-aarch64-apple: 5347.9s -> 9929.5s (85.7%)
  2. dist-powerpc64-linux: 8091.2s -> 5215.4s (-35.5%)
  3. x86_64-apple-2: 6410.1s -> 4707.0s (-26.6%)
  4. dist-aarch64-linux: 8121.4s -> 5994.6s (-26.2%)
  5. dist-x86_64-apple: 8685.0s -> 9855.1s (13.5%)
  6. x86_64-apple-1: 8932.5s -> 8218.8s (-8.0%)
  7. dist-apple-various: 7454.4s -> 8029.6s (7.7%)
  8. x86_64-gnu-llvm-19-2: 5781.0s -> 6212.3s (7.5%)
  9. x86_64-msvc-ext1: 6971.5s -> 7398.4s (6.1%)
  10. x86_64-gnu-llvm-19-1: 3396.6s -> 3603.3s (6.1%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@scottmcm scottmcm deleted the remove-array-chunks branch July 29, 2025 06:00
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (cb6785f): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.1% [-0.1%, -0.1%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -0.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
1.4% [1.4%, 1.4%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.3% [-2.3%, -2.3%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.5% [-2.3%, 1.4%] 2

Cycles

Results (secondary -3.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.7% [-4.8%, -2.7%] 2
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 469.046s -> 467.912s (-0.24%)
Artifact size: 376.81 MiB -> 376.82 MiB (0.00%)

github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Jul 30, 2025
Remove `[T]::array_chunks(_mut)`

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

Closes rust-lang#74985
Closes rust-lang#76354

try-job: dist-various-1
try-job: dist-various-2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting
Projects
None yet
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
10 participants