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.

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

missing_iterator_fold #12209

Open
Philippe-Cholet opened this issue Jan 29, 2024 · 3 comments
Open

missing_iterator_fold #12209

Philippe-Cholet opened this issue Jan 29, 2024 · 3 comments
Assignees
Labels
A-lint Area: New lints

Comments

@Philippe-Cholet
Copy link

Philippe-Cholet commented Jan 29, 2024

What it does

Lints implementations of core::iter::Iterator that do not specialize the fold method.

Advantage

  • Methods that consume the entire iterator (for_each, count, ...) rely on fold by default and would benefit from fold being specialized.
  • Adaptor iterators might use fold (or related methods) for their own usage and would benefit as well.

Drawbacks

Specialize it may not be faster than the default behavior.

Example

struct Iter(u8);
impl Iterator for Iter {
    type Item = u8;
    fn next(&mut self) -> Option<Self::Item> {
        todo!()
    }
}

Could be written as:

struct Iter(u8);
impl Iterator for Iter {
    type Item = u8;
    fn next(&mut self) -> Option<Self::Item> {
        todo!()
    }
    fn fold<B, F>(self, init: B, f: F) -> B
    where
        F: FnMut(B, Self::Item) -> B
    {
        todo!()
    }
}
@Philippe-Cholet Philippe-Cholet added the A-lint Area: New lints label Jan 29, 2024
@Philippe-Cholet
Copy link
Author

@rustbot claim

@Philippe-Cholet
Copy link
Author

Philippe-Cholet commented Jan 29, 2024

Hi I'm a member of rust-itertools. The idea originated there: rust-itertools/itertools#755

I think it would be beneficial to have other lints as well ([try_][r]fold, size_hint mainly) so this is only the first one.
A single LateLintPass would be needed for all of them.

@Philippe-Cholet
Copy link
Author

Philippe-Cholet commented Feb 9, 2024

After discussion on zulip and some reflexion, those lints (if wanted) should at least be part of the same lint pass as missing_trait_methods. Alternatively, missing_trait_methods could be configurable in clippy.toml. Have multiple lints would be more flexible but I understand the reluctance expressed.

EDIT: I will soon make a PR to make missing_trait_methods configurable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant