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

Iterators #395

Closed
wants to merge 18 commits into from
Closed

Iterators #395

wants to merge 18 commits into from

Conversation

steveklabnik
Copy link
Member

A continuation of #381.

This is the draft of the iterators and closures chapter. I decided to cut the bit about closure implementation at the end.

This also

Fixes #374

by using @ruuda's example from https://ruudvanasseldonk.com/2016/11/30/zero-cost-abstractions. Is that okay with you, @ruuda?

@steveklabnik
Copy link
Member Author

Failures seem to be all spellcheck related

@ruuda
Copy link

ruuda commented Jan 10, 2017

Sure, go ahead.

```

Here, even though `x` is not an argument to `equal_to_x`, it's able to
refer to it, as it's a variable defined in the same scope. We couldn't

Choose a reason for hiding this comment

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

The wording here makes me a little nervous that people will read this and assume "bound variables must be in the same scope as the lambda", but is that how it works? I was assuming they just had to be in the scope chain.

Copy link
Member

Choose a reason for hiding this comment

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

I'm going to just take out the word "same", so that this says "a variable defined in the scope"

@sophiajt
Copy link

This feels a bit meatier, but seems it's time for that book-wise.

A few comments:

  • Would be nice to call back to our "impl Iterator for Counter" example later in the chapter. We show using for loops but I didn't see where we use it on the Counter.

  • We don't implement IntoIterator on a custom type. Do we want to? Otherwise this part seems a bit extra.

  • Generally, I think this is good. It's got the right balance of information and also pulling people into the "Rust way", which is a good way to kick off the new section.

### What is a closure
Rust gives you the ability to define *closures*, which are sort of like
functions. Instead of giving you a technical definintion, let's dive into
what clousures look like, syntactically:

Choose a reason for hiding this comment

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

Mmm clousures

let f = Foo;
let moves = |f| moves(f);
moves(f);
```

Choose a reason for hiding this comment

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

The shadowing here made it harder for me to read. Probably better to avoid shadowing just for clarity.

Copy link
Member Author

Choose a reason for hiding this comment

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

hehe we will fight about this forever, won't we 😄

Copy link
Member

Choose a reason for hiding this comment

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

I'm on @jonathandturner's side with this one. Between shadowing f and the closure names shadowing the function names.... yeah, that's a little bit too much mental overhead i think.

```

We only get `5`. Rather than taking a mutable borrow out on our `num`, we took
ownership of a copy.

Choose a reason for hiding this comment

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

Does move always copy?

Copy link
Member Author

@steveklabnik steveklabnik Jan 11, 2017

Choose a reason for hiding this comment

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

it copies but it does not Copy. (aka words are hard)

Copy link
Member

Choose a reason for hiding this comment

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

It might not copy data in memory if it can reuse the location efficiently.

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, this is about semantics, not about what happens after optimization

Copy link
Member

Choose a reason for hiding this comment

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

hnnnng i thought i understood this part but these comments make me confused again


`Fn` isn't the only trait, however, there are three. `Fn`, `FnMut`, and
`FnOnce`. This continues the patterns of threes we've seen elsewhere in Rust:
by owner, by reference, and by mutable reference. By using `Fn`, you may only

Choose a reason for hiding this comment

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

I visually matched them in the same order and got a confused. Fn->Owner, FnMut->Reference, FnOnce->Mutable Reference isn't right. Maybe we can reword to avoid people doing the visual matching?

Copy link
Member

Choose a reason for hiding this comment

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

Yup fixed

```

At the time, we didn't explain the `.iter()` bit, but now you know that it
makes an iterator. Rust's `for` loop is actually 'synatx sugar', that is, it's

Choose a reason for hiding this comment

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

sp?

Copy link
Member

Choose a reason for hiding this comment

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

SYNATX


Well, when we told you about the definition of `Iterator`, we committed a
small lie of omission. The `Iterator` trait has a number of other useful
methods. Like `map`:

Choose a reason for hiding this comment

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

Did we lie? You told us about map and collect earlier...

Copy link
Member

Choose a reason for hiding this comment

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

well we lied when we showed The Definition of The Iterator Trait and it only had the next method on it

let search = match args.next() {
Some(arg) => arg,
None => return "Didn't get a search string",
};

Choose a reason for hiding this comment

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

wrong indent x 2

and building a new vector, it focuses on the high-level objective of the loop.

But are they truly equivalent? Surely the more low-level loop will be faster?
Let's talk about performance.

## Summary: Performance

Choose a reason for hiding this comment

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

This could probably just be "Performance"?

Copy link
Member

Choose a reason for hiding this comment

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

Nice try, Stephen. We still have to write a summary section :P

```

That's right, the iterator version ended up slightly faster! We're not going
to share the bencharmark code exactly here, as the point is not to prove that

Choose a reason for hiding this comment

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

bencharmark


Closures don't have any further performance penalty over regular fn calls
This code sample is taken from an audio decoder. It "restores sample values

Choose a reason for hiding this comment

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

original link? do we refer to the creator in our footnotes?

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 don't know how that works, but that's one reason I CC'd @ruuda here; to see how they'd like to be credited, if using this is okay.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, just saw that they replied, great 👍

Copy link

Choose a reason for hiding this comment

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

Would be nice if you can add a link though :) Does the book have footnotes?

Copy link
Contributor

@matthewjasper matthewjasper left a comment

Choose a reason for hiding this comment

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

Reading through and comparing to the old book

  • Being able to refer to the last chapter's example is nice.
  • I think this chapter takes a while to get going, but I don't know if there's anything to do about it.
  • I guess that returning closures is for chapter 17 (at least until impl trait). and lifetimes in closures for chapter 19, but a forwards reference would help.
  • Are function pointer types worth talking about?
  • While the exact implementation of closures may not be important, I think there's a risk of making closures seem too much like magic (which is a more general problem with learning rust).


```rust
struct Counter {
count: usize,
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't match the u32 below.

} else {
None
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this method have integer overflow problems if you (for some reason) call next u32::MAX or so times?


let x = counter.next().unwrap();
println!("{}", x);
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth showing that the iterator will now keep returning None?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think the check for < 6 should be before the += 1 so that count won't continue to increase, is there a particular reason you implemented it this way @steveklabnik? It's slightly easier to talk through the way it is, but I'm going to try and make it work I think...

Copy link
Member

Choose a reason for hiding this comment

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

oh shoot i see why. poop.

Copy link
Member

Choose a reason for hiding this comment

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

Aha. So to address the u32::MAX issue and whether we should show that it keeps returning None, I'm going to add in this note from the std::iter docs: "Individual iterators may choose to resume iteration, and so calling next() again may or may not eventually start returning Some(Item) again at some point." So what happens after getting the first None depends on the implementation of the iterator.

let a = [10, 20, 30, 40, 50];

{
let result = match IntoIterator::into_iter(a) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be IntoIterator::into_iter(a.iter()).

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 yes; I missed this, thanks

}
},
};
result
Copy link
Contributor

Choose a reason for hiding this comment

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

result has type () and is unnecessary AFAICT in this code. This probably needs explaining if it's there for a reason.

Copy link
Member Author

Choose a reason for hiding this comment

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

it's here because I copied it from our other docs, which say this is the correct de-surgaring. @eddyb?

Copy link
Member

Choose a reason for hiding this comment

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

Weird. It would be relevant for break value but that's not the case here. I'd remove result.

Copy link
Member Author

@steveklabnik steveklabnik Jan 17, 2017

Choose a reason for hiding this comment

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

It's like this in the std::iter docs, should it be removed there too? or just here, to reduce some noise.

Copy link
Member

Choose a reason for hiding this comment

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

Both, IMO.

mut iter => loop {
```

The `match` only has one arm: we match the result, binding it to a mutable
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should explain why a let binding isn't used.

};
```

### Accepting closures as arguments with the `Fn` traits
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be earlier as it's what happens to closures most of the time?

Copy link
Member

Choose a reason for hiding this comment

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

Mmm I'm not sure. I'll think about it.

that's can be converted into an iterator to a `for` loop, and it will
just work. That's nice! However, arrays do not implement `IntoIterator`,
and so we had to call the `iter` method ourself. But since that returns
an iterator, calling `into_iter` on it does nothing, so we're still good!
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be explicit that IntoIterator is blanket implemented for all iterators?

refer to things in its environment by reference. If you mutate the environment,
you must use `FnMut`, and if you take ownership of the environment, `FnOnce`.
Most of the time, you can write `Fn`, and then the compiler will tell you if
you need `FnMut` or `FnOnce`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is when you try to call the function taking the closure, so I'm not sure that if this is really true.

Copy link
Member

Choose a reason for hiding this comment

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

True. I'll clear this up :)

## Closures and their environment

We've learned that functions can only use variables that are in scope, either
by being static or being declared as parameters. But closures can do more.
Copy link
Contributor

Choose a reason for hiding this comment

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

It is clear enough that this is talking about const? static hasn't been mentioned yet.

Copy link
Member

Choose a reason for hiding this comment

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

Very true, I'll change this to say const instead!

## Closures

### What is a closure
Rust gives you the ability to define *closures*, which are sort of like
functions. Instead of giving you a technical definintion, let's dive into

Choose a reason for hiding this comment

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

extra n in definintion

assert_eq!(2, plus_one(1));
```

But we dont have to. Why is this? Functions are part of an explicit interface

Choose a reason for hiding this comment

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

missing ' in dont


Small differences, but they're similar. Why come up with a different syntax
for closures? There's one additional way in which they're different from
functions: they posses an 'environment'.

Choose a reason for hiding this comment

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

missing another s in posses


So in this case, our closure took a mutable reference to `num`, and then when
we called `add_num`, it mutated the underlying value, as we'd expect. We also
needed to declare `add_num` as `mut` too, because were mutating its

Choose a reason for hiding this comment

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

missing ' in were

needed to declare `add_num` as `mut` too, because were mutating its
environment.

If we change to a `move` closure, its different:

Choose a reason for hiding this comment

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

missing ' in its

```

This example doesn't compile, becuase `x` doesn't have a long enough lifetime.
Becuase closures may borrow variables from their enclosing scope, we can

Choose a reason for hiding this comment

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

Becuase should be Because

}
```

This example doesn't compile, becuase `x` doesn't have a long enough lifetime.

Choose a reason for hiding this comment

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

becuase -> because

type. In this case, our closure takes an `i32` as an argument and returns an
`i32`, and so the generic bound we use is `Fn(i32) -> i32`.

Why a trait? Well, each closure has a unique type. Becuase of this, we can't

Choose a reason for hiding this comment

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

Becuase -> Because

}
```

Not too bad! So which style should you chose? Most Rust programmers prefer to

Choose a reason for hiding this comment

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

chose -> choose

something that often needs to be done very quickly, so you care about speed.
But here, we're creating an iterator, using two adaptors, and then finally
consuming the value. What would this code compile to? Well, as of this writing,
this, it compiles down to the same assembly you'd write by hand: there's no

Choose a reason for hiding this comment

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

redundant this?

3. Use the `collect` adaptor to consume the iterator and make a new vector.

That's how we end up with `[2, 3, 4]`. As you can see, closures are a very
important part of using iterators; they provide the way of customizing the

Choose a reason for hiding this comment

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

the way of -> a way of?

`IntoIterator` is another trait that we haven't discussed yet. As the name
suggests, it has an `into_iter` method that takes one argument, and turns
that argument into an `Iterator`. This means that we can pass anything
that's can be converted into an iterator to a `for` loop, and it will

Choose a reason for hiding this comment

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

that's -> that


Talk about using Associated Types here, foreshadow to advanced type systems
chapter about why this is a different thing than normal
Let's talk a bit more about `IntoIterator`. As we said above, it's job is to

Choose a reason for hiding this comment

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

it's job -> its job

sums that are less than 100, and then finally, take the first five of those
numbers. Finally, `sum` will add up all of the numbers into one last number.
This is kind of a silly calculation, but it shows off a few different iterator
adaptors, you can do all kinds of things! Check the documentation of `Iterator`

Choose a reason for hiding this comment

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

you can do all kinds of things! should be a new sentence?

```

We take one parameter, and it has the type `F`. We also return an `i32`. This
part isnt interesting. The next part is:

Choose a reason for hiding this comment

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

isnt -> isn't

@carols10cents
Copy link
Member

Not done yet, I'm only through the Closures section and took care of some of the spelling/grammar comments throughout.

@carols10cents carols10cents self-assigned this Jan 18, 2017
@carols10cents
Copy link
Member

carols10cents commented Jan 19, 2017

Pulling this out of a collapsed thread where @steveklabnik said:

it [move] copies but it does not Copy. (aka words are hard)

Ok this is the part that has me confused. If I change the i32 to String, which is NOT Copy, then this code does not compile:

let mut num = String::from("four");

{
    let mut add_num = move |x| num.push_str(x);

    add_num("six");
}

assert_eq!(String::from("four"), num);

says:

error[E0382]: use of moved value: `num`
  --> src/main.rs:11:34
   |
6  |     let mut add_num = move |x| num.push_str(x);
   |                       -------- value moved (into closure) here
...
11 | assert_eq!(String::from("four"), num);
   |                                  ^^^ value used here after move
   |
   = note: move occurs because `num` has type `std::string::String`, which does not implement the `Copy` trait

... which implies to me that, contrary to what steve said, this DOES Copy, since it only works at all if the value is Copy.

Am I misunderstanding more...? How should we change this sentence to be clearer?

In the last line, num now contains 4: add_num took ownership of a copy of num, rather than mutably borrowing num.

@eddyb
Copy link
Member

eddyb commented Jan 20, 2017

Heh. It "moves" captures into the closure value, but you can't ever move away a Copy value.
Yes the use of the keyword is pretty terrible.

texts of various sizes, different words, words of different lengths, and all
kinds of other things. The point here is this: iterators, while a high-level
abstraction, get compiled down to roughly the same code as if you'd written
the lower-level code yourself.
Copy link
Member

Choose a reason for hiding this comment

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

uggghhh i want to cite stroustrup accurately here but i can't find exactly where he said the part about not being able to hand code it any better... like https://blog.rust-lang.org/2015/05/11/traits.html cites stroustrup citing himself that i think i saw on some powerpoint???

Copy link
Member

Choose a reason for hiding this comment

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

@carols10cents
Copy link
Member

Heh. It "moves" captures into the closure value, but you can't ever move away a Copy value.
Yes the use of the keyword is pretty terrible.

@eddyb How should we change this sentence to be clearer:

In the last line, num now contains 4: add_num took ownership of a copy of num, rather than mutably borrowing num.

@carols10cents
Copy link
Member

@steveklabnik and anyone else who wants to reread this: I think I'm done. Any suggestions before I send this to nostarch sometime Monday?

Copy link
Contributor

@matthewjasper matthewjasper left a comment

Choose a reason for hiding this comment

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

One last thing, apart from what I've already said. And a typo.

fn main() {
let f1 = Foo;
let closure_that_borrows = |x| borrows(x);
closure_that_borrows(&f1);
Copy link
Contributor

@matthewjasper matthewjasper Jan 22, 2017

Choose a reason for hiding this comment

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

And then this should be

let f1 = Foo;
let closure_that_borrows = || borrows(&f1);
// Maybe even a comment that `f1` is now borrowed?
closure_that_borrows();

to match the above description.
Likewise with the next two.

<!-- Will add wingdings in libreoffice /Carol -->

The `iter` method on vectors allows us to produce an *iterator* from the
vector. Nex, the `map` method called on the iterator allows us to process each
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a typo, but CI didn't catch 'Nex'. Or maybe I'm misunderstanding something.

Copy link
Member

Choose a reason for hiding this comment

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

Oh i totally meant the Navy Exchange, yup (thank you for catching this :))

The compiler even reminds us that this only works with closures!

### Closures, Ownership, and Borrowing

Copy link
Contributor

Choose a reason for hiding this comment

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

This section is surely supposed to be about inference of capture borrowing, but then seems to get confused about parameter type inference. The changes I'm suggesting are assuming that it's about captures. If it's really supposed to about parameters, why?

Copy link
Member

Choose a reason for hiding this comment

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

It's entirely possible that I'm confused.

The property of being allowed to use variables from the surrounding scope is
also subject to all of the usual rules around ownership and borrowing. Since
closures attempt to infer the types of their parameters, they also infer how
those parameters are borrowed. Closures make that inference by looking at how
Copy link
Contributor

Choose a reason for hiding this comment

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

This sentence would then be something like 'Closures attempt to infer how the variables from their environment are borrowed.'

`&Foo`, then the type of the parameter must be `&Foo`. If it passes the
parameter to a function that takes `&mut Foo`, then the type of parameter must
be `&mut Foo`, and so on. If we try to use `f3` after the call to
`closure_that_moves` in the last line of `main`, we'll get a compiler error
Copy link
Contributor

Choose a reason for hiding this comment

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

In fact, after closure_that_moves is defined.


Here, Rust is able to look at how we use the parameters of each closure inside
their bodies. If the closure passes its parameter it to a function that takes
`&Foo`, then the type of the parameter must be `&Foo`. If it passes the
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be something like 'If the closure only uses borrows of a variable from the enclosing scope, then it will only capture the reference rather than take ownership of the value'.
And similarly for the next two.

the `match` expressions for `search` and `filename` are so similar. It would be
nice if we could use `?` on the `Option` returned from `next`, but `?` only
works with `Result` values currently. Even if we could use `?` on `Option` like
we can on `Result`, the value we would get would be borrowed, and we want to
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, one more thing. Why mention ?? And is this correct about borrowing?

Copy link
Member

Choose a reason for hiding this comment

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

This code is similar to the code for ?, so readers might be wondering if you can use ? here. Also, someday you probably will be able to.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense. I was concerned that the (possible) future feature was the only reason.

Copy link
Member Author

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

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

extremely minor nits, but we're good to go after

Rust's design has taken inspiration from a lot of previous work. One of Rust's
influences is functional programming, where functions are values that can be
used as arguments or return values to other functions, assigned to variables,
and so forth. We're going to sidestep the issue of what, exactly, function
Copy link
Member Author

Choose a reason for hiding this comment

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

functional

```rust
fn add_one_v1 (x: i32) -> i32 { x + 1 } // a function
let add_one_v2 = |x: i32| -> i32 { x + 1 }; // the full syntax for a closure
let add_one_v3 = |x: i32| { x + 1 }; // a closure eliding types
Copy link
Member Author

Choose a reason for hiding this comment

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

the : i32 should be removed here too

let search = match args.next() {
Some(arg) => arg,
None => return Err("Didn't get a search string"),
};
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 needs another indent

let filename = match args.next() {
Some(arg) => arg,
None => return Err("Didn't get a file name"),
};
Copy link
Member Author

Choose a reason for hiding this comment

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

as does this

@carols10cents
Copy link
Member

I merged this in but then rebased so github couldn't tell

@carols10cents carols10cents deleted the iterators branch January 24, 2017 02:49
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.

9 participants