Skip to content

change result::collect to take an iterator, and adds option::collect #11098

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

Merged
merged 2 commits into from
Dec 28, 2013

Conversation

erickt
Copy link
Contributor

@erickt erickt commented Dec 21, 2013

This patch changes result::collect (and adds a new option::collect) from creating a ~[T] to take an Iterator. This makes the function much more flexible, and may replace the need for #10989.

This patch is a little more complicated than it needs to be because of #11084. Once that is fixed we can replace the CollectIterator with a Scan iterator.

It also fixes a test warning.

@alexcrichton
Copy link
Member

I'd be a little tempted to implement these with Scan today, regardless of the little-bit-slower implementation (due to the code being much better).

I'm hesitant to add collect to option, but seeing how we have it in result it makes sense to me to add it to option as well.

What do you think about using Scan anyway for this implementation?

@erickt
Copy link
Contributor Author

erickt commented Dec 23, 2013

@alexcrichton: I'd be okay with changing over to Scan. Just to let you know though, switching this over to Scan isn't just a little bit slower, but it's twice as slow.

@alexcrichton
Copy link
Member

I would personally rather see a nicer implementation because this doesn't seem particularly common or the possible root cause of slowdown in an application, but I could be wrong.

@alexcrichton
Copy link
Member

With a squash to one commit, r=me

bors added a commit that referenced this pull request Dec 28, 2013
This patch changes `result::collect` (and adds a new `option::collect`) from creating a `~[T]` to take an `Iterator`. This makes the function much more flexible, and may replace the need for #10989.

This patch is a little more complicated than it needs to be because of #11084. Once that is fixed we can replace the `CollectIterator` with a `Scan` iterator.

It also fixes a test warning.
@bors bors closed this Dec 28, 2013
@bors bors merged commit 1da9112 into rust-lang:master Dec 28, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 14, 2023
[`unnecessary_literal_unwrap`]: also lint `unwrap_(err_)unchecked`

Closes rust-lang#11093

changelog: [`unnecessary_literal_unwrap`]: also lint `unwrap_unchecked` and `unwrap_err_unchecked`
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

Successfully merging this pull request may close these issues.

3 participants