Improve `Iterator::by_ref` example #80805

camelid · 2021-01-08T01:46:13Z

I split the example into two: one that fails to compile, and one that
works. I also made them identical except for the addition of by_ref
so we don't confuse readers with random differences.

cc @steveklabnik, who is the one that added the previous version of this example

I split the example into two: one that fails to compile, and one that works. I also made them identical except for the addition of `by_ref` so we don't confuse readers with random differences.

rust-highfive · 2021-01-08T01:46:15Z

r? @withoutboats

(rust-highfive has picked a reviewer for you, use r? to override)

camelid · 2021-01-08T01:46:32Z

Might as well just r? @steveklabnik

library/core/src/iter/traits/iterator.rs

steveklabnik

Thanks for this! I think some of this is excellent, some of it feels like churn, and some of it moves us away from standard constructs.

library/core/src/iter/traits/iterator.rs

camelid · 2021-02-14T05:27:06Z

library/core/src/iter/traits/iterator.rs

+    /// This demonstrates a use case that needs `by_ref`:
+    ///
+    /// ```compile_fail,E0382
+    /// let a = [1, 2, 3, 4, 5];
+    /// let mut iter = a.iter();
+    ///
+    /// let sum: i32 = iter.take(3).fold(0, |acc, i| acc + i);
    /// assert_eq!(sum, 6);
    ///
-    /// // if we try to use iter again, it won't work. The following line
-    /// // gives "error: use of moved value: `iter`
-    /// // assert_eq!(iter.next(), None);
+    /// // Error! We can't use `iter` again because it was moved
+    /// // by `take`.
+    /// assert_eq!(iter.next(), Some(&4));
+    /// ```
    ///
-    /// // let's try that again
-    /// let a = [1, 2, 3];
+    /// Now, let's use `by_ref` to make this work:
    ///
+    /// ```
+    /// let a = [1, 2, 3, 4, 5];
    /// let mut iter = a.iter();
    ///
-    /// // instead, we add in a .by_ref()
-    /// let sum: i32 = iter.by_ref().take(2).fold(0, |acc, i| acc + i);
-    ///
-    /// assert_eq!(sum, 3);
+    /// // We add in a call to `by_ref` here so `iter` isn't moved.
+    /// let sum: i32 = iter.by_ref().take(3).fold(0, |acc, i| acc + i);
+    /// assert_eq!(sum, 6);
    ///
-    /// // now this is just fine:
-    /// assert_eq!(iter.next(), Some(&3));
-    /// assert_eq!(iter.next(), None);
+    /// // And now we can use `iter` again because we still own it.
+    /// assert_eq!(iter.next(), Some(&4));
    /// ```


At this point, should I just remove the later example? I don't feel like it adds much that the basic usage example doesn't show.

@camelid do youw ant to update this without the last example? looks good enough to me without it

@Dylan-DPC OK, I removed the example. Do you want to review this now?

Dylan-DPC-zz · 2021-04-05T11:11:54Z

i could review it, but i would prefer @steveklabnik to have a final look at it

joshtriplett · 2021-04-21T20:08:06Z

This looks reasonable to me.

steveklabnik · 2021-04-23T18:17:22Z

Looks good to me, thanks!

@bors: r+ rollup

bors · 2021-04-23T18:17:24Z

📌 Commit 49ccc3f has been approved by steveklabnik

Rollup of 11 pull requests Successful merges: - rust-lang#80805 (Improve `Iterator::by_ref` example) - rust-lang#84248 (Remove duplicated fn(Box<[T]>) -> Vec<T>) - rust-lang#84321 (rustdoc: Convert sub-variant toggle to HTML) - rust-lang#84359 (:arrow_up: rust-analyzer) - rust-lang#84374 (Clean up .gitignore) - rust-lang#84387 (Move `sys_common::poison` to `sync::poison`) - rust-lang#84430 (doc/platform-support: clarify UEFI support) - rust-lang#84433 (Prevent control, shift and alt keys to make search input lose focus) - rust-lang#84444 (doc: Get rid of "[+] show undocumented items" toggle on numeric From impls) - rust-lang#84456 (Fix ICE if original_span(fn_sig) returns a span not in body sourcefile) - rust-lang#84469 (Update comment on `PrimTy::name_str`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Improve Iterator::by_ref example

f755899

I split the example into two: one that fails to compile, and one that works. I also made them identical except for the addition of `by_ref` so we don't confuse readers with random differences.

rust-highfive assigned withoutboats Jan 8, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 8, 2021

rust-highfive assigned steveklabnik and unassigned withoutboats Jan 8, 2021

camelid commented Jan 8, 2021

View reviewed changes

library/core/src/iter/traits/iterator.rs Outdated Show resolved Hide resolved

steveklabnik reviewed Jan 11, 2021

View reviewed changes

library/core/src/iter/traits/iterator.rs Outdated Show resolved Hide resolved

library/core/src/iter/traits/iterator.rs Outdated Show resolved Hide resolved

library/core/src/iter/traits/iterator.rs Outdated Show resolved Hide resolved

camelid 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 Jan 21, 2021

Better line grouping

bfb0279

Add basic usage example

c1df9f1

camelid commented Feb 14, 2021

View reviewed changes

camelid removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 14, 2021

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 1, 2021

crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 20, 2021

Remove redundant second example

49ccc3f

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 4, 2021

steveklabnik approved these changes Apr 23, 2021

View reviewed changes

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 Apr 23, 2021

JohnTitor mentioned this pull request Apr 23, 2021

Rollup of 11 pull requests #84490

Merged

bors merged commit dcb4083 into rust-lang:master Apr 23, 2021

rustbot added this to the 1.53.0 milestone Apr 23, 2021

camelid deleted the iter-by_ref-example branch April 24, 2021 02:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve `Iterator::by_ref` example #80805

Improve `Iterator::by_ref` example #80805

camelid commented Jan 8, 2021

rust-highfive commented Jan 8, 2021

camelid commented Jan 8, 2021

steveklabnik left a comment

camelid Feb 14, 2021

Dylan-DPC-zz Mar 20, 2021

camelid Mar 20, 2021

Dylan-DPC-zz commented Apr 5, 2021

joshtriplett commented Apr 21, 2021

steveklabnik commented Apr 23, 2021

bors commented Apr 23, 2021

Improve Iterator::by_ref example #80805

Improve Iterator::by_ref example #80805

Conversation

camelid commented Jan 8, 2021

rust-highfive commented Jan 8, 2021

camelid commented Jan 8, 2021

steveklabnik left a comment

Choose a reason for hiding this comment

camelid Feb 14, 2021

Choose a reason for hiding this comment

Dylan-DPC-zz Mar 20, 2021

Choose a reason for hiding this comment

camelid Mar 20, 2021

Choose a reason for hiding this comment

Dylan-DPC-zz commented Apr 5, 2021

joshtriplett commented Apr 21, 2021

steveklabnik commented Apr 23, 2021

bors commented Apr 23, 2021

Improve `Iterator::by_ref` example #80805

Improve `Iterator::by_ref` example #80805