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

Document workings of successors more clearly #135895

Merged
merged 1 commit into from
Mar 7, 2025
Merged

Conversation

hkBst
Copy link
Member

@hkBst hkBst commented Jan 22, 2025

This is an attempt to fix #135087 together with #135886, but I am not sure if I've succeeded in adding much clarity here, so don't be shy with your comments.

@rustbot
Copy link
Collaborator

rustbot commented Jan 22, 2025

r? @Amanieu

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

@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 Jan 22, 2025
joboet added a commit to joboet/rust that referenced this pull request Jan 24, 2025
Document purpose of closure in from_fn.rs more clearly

partial fix for rust-lang#135087 together with rust-lang#135895
Zalathar added a commit to Zalathar/rust that referenced this pull request Jan 28, 2025
Document purpose of closure in from_fn.rs more clearly

partial fix for rust-lang#135087 together with rust-lang#135895
Zalathar added a commit to Zalathar/rust that referenced this pull request Jan 28, 2025
Document purpose of closure in from_fn.rs more clearly

partial fix for rust-lang#135087 together with rust-lang#135895
Zalathar added a commit to Zalathar/rust that referenced this pull request Jan 28, 2025
Document purpose of closure in from_fn.rs more clearly

partial fix for rust-lang#135087 together with rust-lang#135895
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 28, 2025
Document purpose of closure in from_fn.rs more clearly

partial fix for rust-lang#135087 together with rust-lang#135895
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 28, 2025
Document purpose of closure in from_fn.rs more clearly

partial fix for rust-lang#135087 together with rust-lang#135895
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 28, 2025
Rollup merge of rust-lang#135886 - hkBst:patch-14, r=workingjubilee

Document purpose of closure in from_fn.rs more clearly

partial fix for rust-lang#135087 together with rust-lang#135895
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Jan 29, 2025
Document purpose of closure in from_fn.rs more clearly

partial fix for #135087 together with rust-lang/rust#135895
@hkBst
Copy link
Member Author

hkBst commented Feb 19, 2025

r? @joboet
since you reviewed the other PR for the related issue

@rustbot rustbot assigned joboet and unassigned Amanieu Feb 19, 2025
Copy link
Member

@joboet joboet left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! I think the language could be a bit more fluid by avoiding the repetition of "the iterator". Currently all sentences start with that, which feels a bit robotic. Phrasing aside, I think this is a good improvement.

/// The iterator will apply the closure successively to the stored option's value until the option is `None`.
/// This also means that once the stored option is `None` it will remain `None`,
/// as the closure will not be called again, so the created iterator is a `FusedIterator`.
/// The iterator's items will be the initial item and all of its successors as calculated by the successor closure.
Copy link
Member

Choose a reason for hiding this comment

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

This seems redundant to me – isn't this explained by the preceding sentences?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is mostly there as a direct result of the old text saying "The iterator will yield the Ts returned from the closure.", and it does read pretty similar to the topline text ("Creates an iterator which, starting from an initial item, computes each successive item from the preceding one."), but includes slightly more detail (mentioning the successor closure which was explained in the previous paragraph.

I think it's still useful as a nice summary of the previous paragraph, which is quite detailed, and may be more readily understood than the detailed paragraph. But if you still think it is better without it, then I can remove it.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough...

@joboet
Copy link
Member

joboet commented Feb 28, 2025

@rustbot author

@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 Feb 28, 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
Copy link
Member Author

hkBst commented Mar 2, 2025

@rustbot ready

@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 Mar 2, 2025
@joboet
Copy link
Member

joboet commented Mar 6, 2025

Thank you!
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 6, 2025

📌 Commit 6867806 has been approved by joboet

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Mar 6, 2025

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@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 Mar 6, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 6, 2025
…mpiler-errors

Rollup of 25 pull requests

Successful merges:

 - rust-lang#135733 (Implement `&pin const self` and `&pin mut self` sugars)
 - rust-lang#135895 (Document workings of successors more clearly)
 - rust-lang#136922 (Pattern types: Avoid having to handle an Option for range ends in the type system or the HIR)
 - rust-lang#137303 (Remove `MaybeForgetReturn` suggestion)
 - rust-lang#137327 (Undeprecate env::home_dir)
 - rust-lang#137358 (Match Ergonomics 2024: add context and examples to the unstable book)
 - rust-lang#137534 ([rustdoc] hide item that is not marked as doc(inline) and whose src is doc(hidden))
 - rust-lang#137565 (Try to point of macro expansion from resolver and method errors if it involves macro var)
 - rust-lang#137637 (Check dyn flavor before registering upcast goal on wide pointer cast in MIR typeck)
 - rust-lang#137643 (Add DWARF test case for non-C-like `repr128` enums)
 - rust-lang#137744 (Re-add `Clone`-derive on `Thir`)
 - rust-lang#137758 (fix usage of ty decl macro fragments in attributes)
 - rust-lang#137764 (Ensure that negative auto impls are always applicable)
 - rust-lang#137772 (Fix char count in `Display` for `ByteStr`)
 - rust-lang#137798 (ci: use ubuntu 24 on arm large runner)
 - rust-lang#137802 (miri native-call support: all previously exposed provenance is accessible to the callee)
 - rust-lang#137805 (adjust Layout debug printing to match the internal field name)
 - rust-lang#137808 (Do not require that unsafe fields lack drop glue)
 - rust-lang#137820 (Clarify why InhabitedPredicate::instantiate_opt exists)
 - rust-lang#137825 (Provide more context on resolve error caused from incorrect RTN)
 - rust-lang#137834 (rustc_fluent_macro: use CARGO_CRATE_NAME instead of CARGO_PKG_NAME)
 - rust-lang#137868 (Add minimal platform support documentation for powerpc-unknown-linux-gnuspe)
 - rust-lang#137910 (Improve error message for `AsyncFn` trait failure for RPIT)
 - rust-lang#137920 (interpret/provenance_map: consistently use range_is_empty)
 - rust-lang#138038 (Update `compiler-builtins` to 0.1.151)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 6, 2025
…mpiler-errors

Rollup of 25 pull requests

Successful merges:

 - rust-lang#135733 (Implement `&pin const self` and `&pin mut self` sugars)
 - rust-lang#135895 (Document workings of successors more clearly)
 - rust-lang#136922 (Pattern types: Avoid having to handle an Option for range ends in the type system or the HIR)
 - rust-lang#137303 (Remove `MaybeForgetReturn` suggestion)
 - rust-lang#137327 (Undeprecate env::home_dir)
 - rust-lang#137358 (Match Ergonomics 2024: add context and examples to the unstable book)
 - rust-lang#137534 ([rustdoc] hide item that is not marked as doc(inline) and whose src is doc(hidden))
 - rust-lang#137565 (Try to point of macro expansion from resolver and method errors if it involves macro var)
 - rust-lang#137637 (Check dyn flavor before registering upcast goal on wide pointer cast in MIR typeck)
 - rust-lang#137643 (Add DWARF test case for non-C-like `repr128` enums)
 - rust-lang#137744 (Re-add `Clone`-derive on `Thir`)
 - rust-lang#137758 (fix usage of ty decl macro fragments in attributes)
 - rust-lang#137764 (Ensure that negative auto impls are always applicable)
 - rust-lang#137772 (Fix char count in `Display` for `ByteStr`)
 - rust-lang#137798 (ci: use ubuntu 24 on arm large runner)
 - rust-lang#137802 (miri native-call support: all previously exposed provenance is accessible to the callee)
 - rust-lang#137805 (adjust Layout debug printing to match the internal field name)
 - rust-lang#137808 (Do not require that unsafe fields lack drop glue)
 - rust-lang#137820 (Clarify why InhabitedPredicate::instantiate_opt exists)
 - rust-lang#137825 (Provide more context on resolve error caused from incorrect RTN)
 - rust-lang#137834 (rustc_fluent_macro: use CARGO_CRATE_NAME instead of CARGO_PKG_NAME)
 - rust-lang#137868 (Add minimal platform support documentation for powerpc-unknown-linux-gnuspe)
 - rust-lang#137910 (Improve error message for `AsyncFn` trait failure for RPIT)
 - rust-lang#137920 (interpret/provenance_map: consistently use range_is_empty)
 - rust-lang#138038 (Update `compiler-builtins` to 0.1.151)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5b07412 into rust-lang:master Mar 7, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 7, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2025
Rollup merge of rust-lang#135895 - hkBst:patch-15, r=joboet

Document workings of successors more clearly

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

core::iter::from_fn and core::iter::successors documentation does not explain callback function signature
5 participants