Skip to content
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

Use .into_iter() rather than .drain(..) #8483

Merged
merged 1 commit into from
Mar 3, 2022
Merged

Use .into_iter() rather than .drain(..) #8483

merged 1 commit into from
Mar 3, 2022

Conversation

ldm0
Copy link
Contributor

@ldm0 ldm0 commented Feb 28, 2022 •
edited by giraffate
Loading

Replacing .drain(..) with .into_iter() makes my project's binary size smaller.

Fixes #1908

Applicability of this suggestion is MaybeIncorrect rather than MachineApplicable due to the complexity of "checking otherwise usage" X-|

changelog: Add new lint [iter_with_drain]

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @giraffate (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 28, 2022
giraffate
giraffate reviewed Mar 1, 2022
View reviewed changes
Copy link
Contributor

@giraffate giraffate left a comment

Choose a reason for hiding this comment

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

It looks like that some tests are failing. Can they be addressed?

clippy_lints/src/methods/mod.rs Outdated Show resolved Hide resolved
@giraffate giraffate added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 1, 2022
@ldm0 ldm0 force-pushed the iter_with_drain_simple branch 2 times, most recently from 9189f60 to 275a1b0 Compare March 1, 2022 05:51
flip1995
flip1995 reviewed Mar 1, 2022
View reviewed changes
clippy_lints/src/methods/iter_with_drain.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/iter_with_drain.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/iter_with_drain.rs

if let Some(range) = higher::Range::hir(arg) {
let left_full = match range {
Range { start: Some(start), .. } if is_integer_const(cx, start, 0) => true,
Copy link
Member

Choose a reason for hiding this comment

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

This could be extended in the future to suggest .into_iter().skip(N) for constants != 0.

Doesn't necessarily have to be done in this PR though.

tests/ui/iter_with_drain.rs Show resolved Hide resolved
@ldm0 ldm0 force-pushed the iter_with_drain_simple branch from 8bc83aa to a176d11 Compare March 1, 2022 18:06
flip1995
flip1995 reviewed Mar 2, 2022
View reviewed changes
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

LGTM overall. Just one small comment.

I leave the final review and approval to @giraffate

clippy_lints/src/methods/mod.rs Outdated Show resolved Hide resolved
@ldm0 ldm0 force-pushed the iter_with_drain_simple branch from 7599526 to c7f574c Compare March 2, 2022 09:44
@ldm0 ldm0 force-pushed the iter_with_drain_simple branch from c7f574c to ad295b8 Compare March 3, 2022 05:09
@ldm0
Suggest into_iter() over drain(..)
6cc2eea 
Add doc

Add description

iter_with_drain dogfood

Disable emiting on struct field.

Fix clippy

Add eq_path for SpanlessEq

Fix tests

Better error message

Fix doc test

Fix version

Apply suggestions
@ldm0 ldm0 force-pushed the iter_with_drain_simple branch from ad295b8 to 6cc2eea Compare March 3, 2022 05:10
@giraffate
Copy link
Contributor

@bors r=flip1995,giraffate

It looks good, thanks!

@bors
Copy link
Contributor

bors commented Mar 3, 2022

📌 Commit 6cc2eea has been approved by flip1995,giraffate

@bors
Copy link
Contributor

bors commented Mar 3, 2022

⌛ Testing commit 6cc2eea with merge d56f457...

@bors
Copy link
Contributor

bors commented Mar 3, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995,giraffate
Pushing d56f457 to master...

@bors bors merged commit d56f457 into rust-lang:master Mar 3, 2022
@ldm0 ldm0 deleted the iter_with_drain_simple branch March 3, 2022 16:48
This was referenced Mar 15, 2022
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@giraffate giraffate giraffate left review comments

@flip1995 flip1995 flip1995 left review comments

Assignees

@giraffate giraffate

Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Suggest using into_iter() instead of drain(..) on Vec
5 participants
@ldm0 @rust-highfive @giraffate @bors @flip1995