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

core::iter::from_fn and core::iter::successors documentation does not explain callback function signature #135087

Open
rick-de-water opened this issue Jan 3, 2025 · 5 comments · May be fixed by #135895
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@rick-de-water
Copy link
Contributor

Location

core::iter::from_fn
core::iter::successors

Summary

Both of these functions take callback methods that return an Option<T>. However, nowhere is actually described that this Option<T> is used to detect the end of the iterator (i.e. return Some while there are items, return None when at the end). I was able to deduce how things worked eventually by looking at the examples, but I was definitely a bit confused at first.

There should be a clear description on what the callback is expected to return, not just when the callback is called.

@rick-de-water rick-de-water added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Jan 3, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 3, 2025
@workingjubilee workingjubilee added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jan 4, 2025
@ranger-ross
Copy link
Contributor

ranger-ross commented Jan 5, 2025

Yeah... core::iter::successors in particular is difficult to understand if you are not familar with the type signature of checked_mul

Perhaps,


core::iter::from_fn

Creates a new iterator where each iteration calls the provided closure F: FnMut() -> Option<T>. The iterator will yield the T's returned from the closure until it returns None.

[...]


core::iter::successors

Creates a new iterator where each successive item is computed based on the preceding one.

The iterator starts with the given first item (if any) and calls the given FnMut(&T) -> Option<T> closure to compute each item’s successor. The iterator will yield the T's returned from the closure until it returns None.

[...]

jhpratt added a commit to jhpratt/rust that referenced this issue Jan 5, 2025
…s, r=jhpratt

Clarified the documentation on `core::iter::from_fn` and `core::iter::successors`

This PR clarifies the closure requirements for `core::iter::from_fn` and `core::iter::successors`.

`std::iter::successors` in particular is a bit difficult to understand if you are not already familiar with the signature of [`checked_mul`](https://docs.rs/num/latest/num/trait.CheckedMul.html) used in the example.

See rust-lang#135087
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 6, 2025
Rollup merge of rust-lang#135118 - ranger-ross:better-docs-on-iter-fns, r=jhpratt

Clarified the documentation on `core::iter::from_fn` and `core::iter::successors`

This PR clarifies the closure requirements for `core::iter::from_fn` and `core::iter::successors`.

`std::iter::successors` in particular is a bit difficult to understand if you are not already familiar with the signature of [`checked_mul`](https://docs.rs/num/latest/num/trait.CheckedMul.html) used in the example.

See rust-lang#135087
@Skgland
Copy link
Contributor

Skgland commented Jan 6, 2025

core::iter::from_fn

Creates a new iterator where each iteration calls the provided closure F: FnMut() -> Option<T>. The iterator will yield the T's returned from the closure until it returns None.

[...]

Note that the iterator can resume after None depending on the function, until it returns `None` makes it sound like its a fused iterator.

E.g. this works

fn main() {
    let mut i = 0;
    let mut iter = std::iter::from_fn(move || {
        i += 1;
        if i % 3 == 0 {
            None
        } else {
            Some(i)  
        }
    });
    
    assert_eq!((&mut iter).collect::<Vec<_>>(), [1,2]);
    assert_eq!(iter.next(), Some(4));
    assert_eq!(iter.next(), Some(5));
    assert_eq!(iter.next(), None);
}

play-ground

@ranger-ross
Copy link
Contributor

Note that the iterator can resume after None depending on the function, until it returns None makes it sound like its a fused iterator.

Yeah, that was brought up in #135118 here 👉 #135118 (comment)

I ended up dropping the "until it returns None." part

@hkBst
Copy link
Contributor

hkBst commented Jan 19, 2025

@rick-de-water would it help to document that this function argument is essentially the returned iterators next function?

@rick-de-water
Copy link
Contributor Author

rick-de-water commented Jan 22, 2025

@hkBst Yes, that would work for from_fn. successors is a bit more complicated because it's fused, so explaining that the function won't ever get called again after the first None is also important.

hkBst added a commit to hkBst/rust that referenced this issue Jan 22, 2025
This is an attempt to fix rust-lang#135087 together with rust-lang#135886, but I am not sure if I've succeeded in adding much clarity here, so don't be shy with your comments.
@hkBst hkBst linked a pull request Jan 22, 2025 that will close this issue
joboet added a commit to joboet/rust that referenced this issue Jan 24, 2025
Document purpose of closure in from_fn.rs more clearly

partial fix for rust-lang#135087 together with rust-lang#135895
@clubby789 clubby789 removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants