Make hashmap iterators implement ExactSize #19327
Conversation
| impl<'a, K, V> DoubleEndedIterator<(&'a K, &'a V)> for Entries<'a, K, V> { | ||
| #[inline] | ||
| fn next_back(&mut self) -> Option<(&'a K, &'a V)> { | ||
| self.inner.next_back() |
There was a problem hiding this comment.
Hashmap is fundamentally unordered. Semantically, what does it even mean to iterate backwards? Couldn't this just call next and call it a day? Or am I missing a usecase?
There was a problem hiding this comment.
I think it'd be surprising if forward iteration and backward iteration returned the same series of elements. Anyhow, I only implemented DoubleEndedIterator because it's needed for ExactSize for some reason.
There was a problem hiding this comment.
Yes, I'm fine with impl'ing DoubleEnded and ExactSize for usability with that tooling. I don't however think people should be able to rely on the order of elements when you call next/next_back between iterations. I think it's fine to have next_back be a synonym for next here.
There was a problem hiding this comment.
I think that would go against the spirit of DoubleEndedIterator. We should at least be doing something "backwards-ish" or at least as backwards as makes sense for a hashtable. People might have a usecase for it and I feel like we shouldn't just lie in the interface.
There was a problem hiding this comment.
I can see the argument either way, honestly. Plus we can explicitly document that behaviour can't be relied on for different sequences of next/next_back. Which is backwards-compatible to "undo". I'm not a huge fan of duplicating a bunch on unsafe code in the already-beefy rawtable. Making next=next_back seems like a simple way to do that.
Interested in getting some other opinions though.
There was a problem hiding this comment.
Me too. I wish someone "owned" this code. :)
|
Closing out of inactivity (and to help clear out @bors's queue), but feel free to reopen with a rebase! |
No description provided.