Skip to content

Slice iter cleanup #146436

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
Merged
bors merged 4 commits into from
Dec 3, 2025
Merged

Slice iter cleanup #146436

bors merged 4 commits into from
Dec 3, 2025

Conversation

@hkBst
Copy link
Member

@hkBst hkBst commented Sep 11, 2025

No description provided.

@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 Sep 11, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 11, 2025

r? @scottmcm

rustbot has assigned @scottmcm.
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

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@hkBst
Copy link
Member Author

hkBst commented Oct 10, 2025

r? libs

@rustbot rustbot assigned ibraheemdev and unassigned scottmcm Oct 10, 2025
@ibraheemdev
Copy link
Member

ibraheemdev commented Oct 22, 2025

Going to pass this one along. r? libs

@rustbot rustbot assigned joboet and unassigned ibraheemdev Oct 22, 2025
joboet
joboet requested changes Oct 31, 2025
View reviewed changes
@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 31, 2025
@rustbot

This comment has been minimized.

Sign in to view
@hkBst
Copy link
Member Author

hkBst commented Nov 1, 2025

@rustbot ready

Perhaps a perf run to see if I haven't regressed anything unwittingly?

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 1, 2025
joboet
joboet reviewed Nov 3, 2025
View reviewed changes
library/core/src/slice/iter.rs
// SAFETY: Nothing else points to or will point to the contents of this slice.
Some(unsafe { &mut *head })
}
// SAFETY: we have `&mut self`, so are allowed to temporarily materialize a mut slice
Copy link
Member

@joboet joboet Nov 3, 2025

Choose a reason for hiding this comment

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

I'd add that next must not be called if __get_unchecked was called, so invalidating previously returned items does not conflict with TrustedRandomAccess.

Copy link
Member Author

@hkBst hkBst Nov 3, 2025

Choose a reason for hiding this comment

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

While that is true, I don't see how this particular next method is special in that regard. So would you then like all next methods of iterators that impl TrustedRandomAccess to get such a comment?

@rustbot
Copy link
Collaborator

rustbot commented Nov 4, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@hkBst
Copy link
Member Author

hkBst commented Nov 4, 2025

@joboet Thanks for reviewing so far; your comments have been very helpful!

I've removed all uses of overflowing_* and also switched all remaining conditions like if n >= len { code } else { code that needs the condition to be correct } to if n < len { code that needs the condition to be correct } else { code }.

joboet
joboet reviewed Nov 13, 2025
View reviewed changes
library/core/src/slice/iter.rs
Comment on lines 1695 to 1700
if self.v.is_empty() {
(0, Some(0))
} else {
let n = self.v.len() / self.chunk_size;
let rem = self.v.len() % self.chunk_size;
let n = if rem > 0 { n + 1 } else { n };
let n = (self.v.len() - 1) / self.chunk_size + 1;
(n, Some(n))
}
Copy link
Member

@joboet joboet Nov 6, 2025

Choose a reason for hiding this comment

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

Same here.

Copy link
Member Author

@hkBst hkBst Nov 14, 2025

Choose a reason for hiding this comment

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

I changed 4 cases of this manual div_ceil to the real thing..

@hkBst hkBst requested a review from joboet November 28, 2025 12:25
This was referenced Nov 28, 2025
Open
Closed
@joboet
Copy link
Member

joboet commented Dec 2, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 2, 2025

📌 Commit b60788e has been approved by joboet

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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 2, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 2, 2025
@matthiaskrgr
Rollup merge of rust-lang#146436 - hkBst:slice-iter-1, r=joboet
b8338e9 
Slice iter cleanup
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 2, 2025
Closed
bors added a commit that referenced this pull request Dec 2, 2025
@bors
Auto merge of #149552 - matthiaskrgr:rollup-j8hm2a1, r=matthiaskrgr
c923e31 
Rollup of 4 pull requests

Successful merges:

 - #141980 (Rework `c_variadic`)
 - #146436 (Slice iter cleanup)
 - #148250 (array_chunks: slightly improve docs)
 - #149520 (also introduce Peekable::next_if_map_mut next to next_if_map)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 2, 2025
Merged
bors added a commit that referenced this pull request Dec 2, 2025
@bors
Auto merge of #149560 - matthiaskrgr:rollup-lce3baq, r=matthiaskrgr
a4cfac7 
Rollup of 5 pull requests

Successful merges:

 - #146436 (Slice iter cleanup)
 - #148250 (array_chunks: slightly improve docs)
 - #148678 (Merge E0412 into E0425)
 - #149520 (also introduce Peekable::next_if_map_mut next to next_if_map)
 - #149538 (std: sys: fs: uefi: Make time in FileAttr optional)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 38d5d28 into rust-lang:main Dec 3, 2025
11 checks passed
@rustbot rustbot added this to the 1.93.0 milestone Dec 3, 2025
@bors
Copy link
Collaborator

bors commented Dec 3, 2025

⌛ Testing commit b60788e with merge a4cfac7...

rust-timer added a commit that referenced this pull request Dec 3, 2025
@rust-timer
Unrolled build for #146436
0311eec 
Rollup merge of #146436 - hkBst:slice-iter-1, r=joboet

Slice iter cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joboet joboet Awaiting requested review from joboet

Assignees

@joboet joboet

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.93.0

Development

Successfully merging this pull request may close these issues.

7 participants

@hkBst @rustbot @rust-log-analyzer @ibraheemdev @joboet @bors @scottmcm