Add some Vec <-> VecDeque documentation #61447
Conversation
|
r? @sfackler (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
What's the precedent for documenting trait impl blocks vs trait functions? My understanding is the former is easier to see on the docs webpage because functions are minimized. |
|
@czipperz Well, @Mark-Simulacrum said on discord,
(I'm happy to do whatever here.) |
|
Looking through other parts of std, it seems like documenting the method is standard. Also, when you override the documentation it automatically expands in rustdoc: try https://doc.rust-lang.org/nightly/std/net/enum.SocketAddr.html for |
|
In rust-lang/rust a loose grep appears to suggest 96 cases of documentation on trait impls (among ~20,000 trait impls) so I would say the clear preference is definitely for the method. |
| /// // Start with a `VecDeque<i32>`. | ||
| /// let deque: VecDeque<_> = (1..5).collect(); | ||
| /// | ||
| /// // Turn it into a `Vec<i32>` with no allocation needed. |
There was a problem hiding this comment.
What's the motivation for spending so much time worrying about reallocation here?
There was a problem hiding this comment.
I think there is a few open issues where people want to know about our allocation strategy for various From impls. In other words, people want to know so they are being explicit about it.
There was a problem hiding this comment.
Basically, the only thing interesting about these two From impls is that they can (sometimes) avoid allocations compared to .into_iter().collect(). So I want to write an example that shows the lack of reallocation, but I also want to write it in such a way that it's a situation where we'd be ok committing to the lack of reallocation. So I want to make an example that doesn't do things like "well, I happen to know that right now it doesn't reallocate if I with_capacity(8) and don't use all the capacity", because I don't want to write text elaborating why those conditions work.
I agree this example is far more complicated than most, though, so would be happy to change it.
There was a problem hiding this comment.
I don't know if an example is really helpful at all here, if we don't actually want to guarantee anything.
There was a problem hiding this comment.
@sfackler Ok, I removed the example from here.
|
Should we be including links to |
This comment has been minimized.
This comment has been minimized.
These are more than just `.into_iter().collect()`, so talk about some of their nuances.
Co-Authored-By: Mazdak Farrokhzad <twingoow@gmail.com>
Since simulacrum suggested (on Discord) they're better there.
Co-Authored-By: Joe ST <joe@fbstj.net>
Let's try the auto-linking instead, since the relative ones don't work.
|
So converting to VecDeque can reallocate and to Vec can shift elements around |
|
@bors r+ |
|
📌 Commit 0150448 has been approved by |
bors
added
S-waiting-on-bors
Add some Vec <-> VecDeque documentation These are more than just `.into_iter().collect()`, so talk about some of their nuances. For VecDeque -> Vec I'm trying to intentionally not write a guarantee for people making their own `Vec`s, since the rules are more complicated than I think we want to commit to forever. The "Vec -> VecDeque doesn't reallocate" guarantee seems reasonable, though. (And I'm intentionally ambiguous about when it's O(1) instead of O(n).)
Rollup of 6 pull requests Successful merges: - #61447 (Add some Vec <-> VecDeque documentation) - #61704 (Pass LLVM linker flags to librustc_llvm build) - #61829 (rustbuild: include llvm-libunwind in dist tarball) - #61832 (update miri) - #61866 (Remove redundant `clone()`s) - #61869 (Cleanup some new active feature gates) Failed merges: r? @ghost
These are more than just
.into_iter().collect(), so talk about some of their nuances.For VecDeque -> Vec I'm trying to intentionally not write a guarantee for people making their own
Vecs, since the rules are more complicated than I think we want to commit to forever.The "Vec -> VecDeque doesn't reallocate" guarantee seems reasonable, though. (And I'm intentionally ambiguous about when it's O(1) instead of O(n).)