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

RFC: movable array iterators #2185

Closed
Closed
Changes from 2 commits
Commits
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
68 changes: 68 additions & 0 deletions text/0000-array_into_iter_move.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
- Feature Name: array_into_iter_move
- Start Date: 2017-10-21
- RFC PR: (leave this empty)
- Rust Issue: (leave this empty)

# Summary
[summary]: #summary

Implement into_iter on array types (not reference, so it moves).

# Motivation
[motivation]: #motivation

Arrays are particularly useful in `flat_map`, but currently a Vec is required as a movable
iterator because arrays only have slice iterator semantics. Obviously this is not optimal.

Now with `ManuallyDrop` implemented, it should be possible to implement moving into_iter on arrays.

# Guide-level explanation
[guide-level-explanation]: #guide-level-explanation

You can convert an array into an iterator with move, just like Vec:

```rust
for x in [1, 2] {
// x is {integer} instead of &{integer}
}
```

```rust
let v: Vec<_> = (0..5).flat_map(|x| [x, x*2]).collect();
```

# Reference-level explanation
[reference-level-explanation]: #reference-level-explanation

Proposed implementation:

- a type `array::IntoIter` is created. The `array::into_iter` method is implemented.
- Inside the method:
* Wrap the array with `ManuallyDrop`.
* Create a `IntoIter` struct with contents moved.
- Inside the iterator:
* Keep track of valid range (index) and move (`ptr::read`) items out as `next()` is called.
* Don't forget to drop the items if the iterator itself is dropped in middle. This should be done with `drop_in_place`.

We should add a lint for redundant Vec in clippy to promote the use of this.

# Drawbacks
[drawbacks]: #drawbacks

This is not 100% backwards compatible as it changes the signature of into_iter (only if directly
called like below). A crater run is required.

```rust
[1, 2].into_iter();
// This was originally yielding references, but now values.
```
Copy link
Member

@bluss bluss Oct 24, 2017

Choose a reason for hiding this comment

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

This is pretty much ensured to break actual code for many users, both in crater (that we can test, find and maybe fix), and lots of code we can't find or test. Maybe we can begin warning for this long before the new iterator is implemented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Personally speaking, I don't think people would use into_iter despite the fact it yields references, and they would just iter instead. Thus I think fixing after a crater should be sufficient.

Copy link
Member

Choose a reason for hiding this comment

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

We don't reach all rust code in the world with crater. If the break is wide spread, we have to be more careful.

Copy link

Choose a reason for hiding this comment

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

And what about for &x in [1,2,3], isn't that just into_iter?

Copy link
Member

@bluss bluss Oct 25, 2017

Choose a reason for hiding this comment

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

That does not compile today I think so it's not a backwards compat concern.

Copy link

Choose a reason for hiding this comment

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

Oh I see. [1].into_iter() currently resolves to the impl for &[_; _] which the new [_; _] impl would override, right?

Copy link
Member

Choose a reason for hiding this comment

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

Yes!


# Rationale and alternatives
[alternatives]: #alternatives

TBD

# Unresolved questions
[unresolved]: #unresolved-questions

- What should the type signature look like? Possibly utilizing const generics.