Clarify WindowsMut (Lending)Iterator #135773
Conversation
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
library/core/src/slice/mod.rs
Outdated
| /// There's no `windows_mut` Iterator-equivalent of `windows`, | ||
| /// (as `[0,1,2].windows_mut(2).collect()` would create 2 mutable references to the middle elements of that array), |
There was a problem hiding this comment.
Instead of an example, I think it may be worth giving the reasoning here. For example, "... because the Iterator trait cannot represent the required lifetimes (though this is possible with a [LendingIterator]). However, you can sometimes..."
There was a problem hiding this comment.
I've now lead with this more abstract lifetime reasoning, though I did not cut the counter(!)example, as I think it is a simple to understand but powerful proof of non-existence of windows_mut as an Iterator. Let me know what you think!
This comment has been minimized.
This comment has been minimized.
library/core/src/slice/mod.rs
Outdated
| /// Because the [Iterator] trait cannot represent the required lifetimes, there is no `windows_mut` analog to `windows`. | ||
| /// (If there was, then `[0,1,2].windows_mut(2).collect()` would create 2 mutable references | ||
| /// to the middle element of that array, which would violate [the rules of references], | ||
| /// though a [LendingIterator] analog is possible.) | ||
| /// However, you can sometimes use [`Cell::as_slice_of_cells`](crate::cell::Cell::as_slice_of_cells) in |
There was a problem hiding this comment.
I think things can be simplified a bit: complete thoughts shouldn't be in ( ... ), and I don't think we need to point link to the reference docs (we can probably assume users know that two mutable references to the same thing is bad). So suggested wording, also dropping the LendingIterator mention:
| /// Because the [Iterator] trait cannot represent the required lifetimes, there is no `windows_mut` analog to `windows`. | |
| /// (If there was, then `[0,1,2].windows_mut(2).collect()` would create 2 mutable references | |
| /// to the middle element of that array, which would violate [the rules of references], | |
| /// though a [LendingIterator] analog is possible.) | |
| /// However, you can sometimes use [`Cell::as_slice_of_cells`](crate::cell::Cell::as_slice_of_cells) in | |
| /// There is no `windows_mut` analog to `windows` because [`Iterator`] does not represent the | |
| /// required lifetimes, e.g. `[0, 1, 2].windows_mut(2).collect()` would allow creating two mutable | |
| /// references to the middle element of the array. However, you can sometimes use | |
| /// [`Cell::as_slice_of_cells`] (crate::cell::Cell::as_slice_of_cells) in |
Then add something like "This pattern can also be achieved with a [LendingIterator]." after the example (to keep the paragraph introducing the example only about std).
There was a problem hiding this comment.
Regarding complete thought in parens: The second site I looked up gave this example: Tonight's gala is a huge occasion. (Everyone who's somebody will be there.), so I don't quite see what is wrong with my use of parens here. I've used the parens in the sense that you can get more information by reading what's inside them, but you can also safely skip over them, and this way the reference outside of std is not in the unparenthesized text. Upon further reflection I suppose I could move the parens to only go around "though a [LendingIterator] analog is possible".
There was a problem hiding this comment.
I've made the parenthetical more terse and adjusted punctuation. Let me know if you think it helps.
There was a problem hiding this comment.
It is grammatically correct, but readability tends to be better if parenthesized data is kept brief. If there are complete asides that need to be expressed, it is preferable to put it in a footnote or separate paragraph, or just fold it into the text somehow.
Reword looks fine though 👍
hkBst
force-pushed
the
patch-10
branch
2 times, most recently
from
01f49b0 to
cc52066
Compare
|
@tgross35 I've rewrapped text and squashed, but there is one long line with the link to the book that I don't know how to shorten, and am afraid to break up. Let me know if there is anything that can be done there, otherwise I think this is good to go. |
library/core/src/slice/mod.rs
Outdated
| /// Because the [Iterator] trait cannot represent the required lifetimes, | ||
| /// there is no `windows_mut` analog to `windows`; | ||
| /// `[0,1,2].windows_mut(2).collect()` would violate [the rules of references] | ||
| /// (though a [LendingIterator] analog is possible). | ||
| /// You can sometimes use | ||
| /// [`Cell::as_slice_of_cells`](crate::cell::Cell::as_slice_of_cells) in | ||
| /// conjunction with `windows` instead: |
There was a problem hiding this comment.
| /// Because the [Iterator] trait cannot represent the required lifetimes, | |
| /// there is no `windows_mut` analog to `windows`; | |
| /// `[0,1,2].windows_mut(2).collect()` would violate [the rules of references] | |
| /// (though a [LendingIterator] analog is possible). | |
| /// You can sometimes use | |
| /// [`Cell::as_slice_of_cells`](crate::cell::Cell::as_slice_of_cells) in | |
| /// conjunction with `windows` instead: | |
| /// Because the [Iterator] trait cannot represent the required lifetimes, there is no | |
| /// `windows_mut` analog to `windows`; `[0,1,2].windows_mut(2).collect()` would violate | |
| /// [the rules of references] (though a [LendingIterator] analog is possible). You can sometimes | |
| /// use [`Cell::as_slice_of_cells`](crate::cell::Cell::as_slice_of_cells) in conjunction with | |
| /// `windows` instead: |
Looks like it may have gotten chopped up somehow, it should be something like the wrapping here (I wish rustfmt just did this...)
No worries about links, those are find to exceed 100.
There was a problem hiding this comment.
I did manual line breaking at logical places, which is why it looks a bit ragged.
There was a problem hiding this comment.
Ah, makes sense. We tend to avoid semantic line breaks in docs because it's a small benefit for reading diffs but makes reading the code less nice, and the code gets read significantly more than diffs do.
Anyway, thanks for all the follow up here!
fixes 133628
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
|
@tgross35 thanks for reviewing! |
…llaumeGomez Rollup of 10 pull requests Successful merges: - rust-lang#135773 (Clarify WindowsMut (Lending)Iterator) - rust-lang#135807 (Implement phantom variance markers) - rust-lang#135876 (fix doc for std::sync::mpmc) - rust-lang#135988 (Add a workaround for parallel rustc crashing when there are delayed bugs) - rust-lang#136037 (Mark all NuttX targets as tier 3 target and support the standard library) - rust-lang#136064 (Add a suggestion to cast target_feature fn items to fn pointers.) - rust-lang#136082 (Incorporate `iter_nodes` into `graph::DirectedGraph`) - rust-lang#136112 (Clean up all dead files inside `tests/ui/`) - rust-lang#136114 (Use identifiers more in diagnostics code) - rust-lang#136118 (Change `collect_and_partition_mono_items` tuple return type to a struct) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#135773 - hkBst:patch-10, r=tgross35 Clarify WindowsMut (Lending)Iterator fixes rust-lang#133628
fixes #133628