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

Document the behaviour of infinite iterators on potentially-computable methods #47547

Merged
merged 4 commits into from
Feb 10, 2018
Merged

Document the behaviour of infinite iterators on potentially-computable methods #47547

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fdfb964
Document the behaviour of infinite iterators on potentially-computabl…
varkor Jan 18, 2018
91668fb
Make example no_run
varkor Jan 18, 2018
0be5173
Adjust language as per suggestions
varkor Jan 19, 2018
f129374
Use repeat instead of RangeFrom
varkor Jan 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/libcore/iter/iterator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ fn _assert_is_object_safe(_: &Iterator<Item=()>) {}
/// This is the main iterator trait. For more about the concept of iterators
/// generally, please see the [module-level documentation]. In particular, you
/// may want to know how to [implement `Iterator`][impl].
///
Copy link
Member

Choose a reason for hiding this comment

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

Trailing white space.

[00:05:27] tidy error: /checkout/src/libcore/iter/mod.rs:300: trailing whitespace
[00:05:27] tidy error: /checkout/src/libcore/iter/mod.rs:306: trailing whitespace
[00:05:27] tidy error: /checkout/src/libcore/iter/iterator.rs:27: trailing whitespace

/// Note: Methods on infinite iterators that generally require traversing every
Copy link
Member Author

Choose a reason for hiding this comment

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

I thought it was probably helpful to point this out in the trait documentation, but if this feels like too much of a niche case to mention here, the module-level documentation could be sufficient.

/// element to produce a result may not terminate, even on traits for which a
/// result is determinable in finite time.
Copy link
Member

Choose a reason for hiding this comment

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

Given that the trait's doc-comment is essentially just a redirect to the module docs, I don't think this note needs to be here as well. Maybe mention diverging in fold specifically, as the most general case of the methods that diverge when given infinite input?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was divided 50:50 on this one. I've taken it out and replaced it with a comment on fold as you suggested.

///
/// [module-level documentation]: index.html
/// [impl]: index.html#implementing-iterator
Expand Down
14 changes: 14 additions & 0 deletions src/libcore/iter/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,22 @@
//! ```
//!
//! This will print the numbers `0` through `4`, each on their own line.
//!
//! Bear in mind that methods on infinite iterators, even those for which a
//! result can be computed in finite time, may not terminate. Specifically,
Copy link
Member

Choose a reason for hiding this comment

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

I think the words "can be computed" lead to a "well, how do I compute it then?" feeling. Perhaps it could be phrased along the lines of "even if a value for them could be determined mathematically", or something?

And instead of "may not terminate", what about being more explicit, with something like "will not return successfully"? (I don't know if talking about the different ways something could diverge would be useful.)

//! methods such as [`min`], which in the general case require traversing
//! every element in the iterator, are likely never to terminate for any
//! infinite iterators.
//!
//! ```
//! let positives = 1..;
//! let least = positives.min().unwrap(); // Oh no! An infinite loop!
//! // `positives.min` causes an infinite loop, so we won't reach this point!
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: in debug mode this panics due to overflow, and doesn't infinite-loop
https://play.rust-lang.org/?gist=bff007ce8e876616bbd965ec4c6dcba0&version=stable

//! println!("The least positive number is {}.", least);
//! ```
Copy link
Member

Choose a reason for hiding this comment

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

This block should be marked as ```no_run, otherwise I don't think the CI will accept it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, of course!

//!
//! [`take`]: trait.Iterator.html#method.take
//! [`min`]: trait.Iterator.html#method.min

#![stable(feature = "rust1", since = "1.0.0")]

Expand Down