Skip to content

10% of calls to .*iter() are redundant #26188

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

Closed
Veedrac opened this issue Jun 10, 2015 · 3 comments
Closed

10% of calls to .*iter() are redundant #26188

Veedrac opened this issue Jun 10, 2015 · 3 comments

Comments

@Veedrac
Copy link
Contributor

Veedrac commented Jun 10, 2015

The source has just over 200 calls to iter, iter_mut or into_iter to things that take and T: IntoIterator. This is, by my count, about 10% of them!

Some examples are left.extend(Some(x).into_iter()) instead of left.extend(Some(x)) and for &(s,t) in v.iter() instead of for &(s,t) in v.

I thought this would make a quick, trivial change to warm myself up to the build system and such, so I've already got a branch removing it. I thought I'd ask if it's actually desirable to do so, though, since it's a lot of seemingly-meaningless code churn.

If this is wanted, the docs should probably also be updated to emphasize that many calls to .*iter() aren't needed. Much of the documentation for iter() and co. uses them in places where they can be elided.

@steveklabnik
Copy link
Member

the docs should probably also be updated to emphasize that many calls to .*iter() aren't needed.

We do already specifically talk about this in the doc, but you have to remember that rustc's codebase is older than IntoIterator! :)

Generally speaking, we accept PRs that update the code to something more idiomatic. I don't speak for everyone, but I would be 👍 on it.

@steveklabnik
Copy link
Member

After pinging #rust-internals, it would seem that others agree. Please send a PR! Thanks :)

@Manishearth
Copy link
Member

Rust and Servo are bad examples for idiomatic rust code because of the hysterisis of rust upgrades.

I haven't seen new code doing this, so I don't think it's a pervasive problem -- just isolated to rust. 👍 to fixing it 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants