Skip to content

pre-RFC: HashSet::pop() #37986

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

Closed
joshtriplett opened this issue Nov 24, 2016 · 6 comments
Closed

pre-RFC: HashSet::pop() #37986

joshtriplett opened this issue Nov 24, 2016 · 6 comments

Comments

@joshtriplett
Copy link
Member

I'd like to have a pop() method available for HashSet, which removes an item from the set if any, and returns an Option<T> (returning None if empty). That would make it easy to iterate over a set with while let element = set.pop() { ... }, without holding a mutable reference to the set as HashSet::drain() does, so you can insert more items into the set as you iterate.

Does that seem like a reasonable method to add to HashSet? Does this need an RFC, or just a patch to std?

@jonas-schievink
Copy link
Contributor

The more interesting question is how you'd implement this without quadratic behaviour

@nagisa
Copy link
Member

nagisa commented Nov 24, 2016

Duplicate-ish to #33549

@sfackler
Copy link
Member

I believe these kinds of issues should also be filed on the RFCs repo rather than here.

@joshtriplett
Copy link
Member Author

@jonas-schievink You could keep a "hint" in the set for the next bucket to check for an element, and increment that after popping out an element. That would avoid traversing all the empty buckets every time. So, unless you had a loop where every insert happened to put a new element at the point directly behind the current hint, you shouldn't get quadratic behavior.

@nagisa Not quite, I think. An iterator over the entries wouldn't allow mutating the set at the same time.

@sfackler You're right, I should have filed this on the RFCs repository.

@bluss
Copy link
Member

bluss commented Nov 24, 2016

OrderMap has an O(1) (average) .pop() because its implementation allows it.

Like jonas said, needs shrewd plan for how to do it in HashSet/HashMap if they don't change implementation.

@sfackler
Copy link
Member

Closing in favor of the RFCs issue.

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

No branches or pull requests

5 participants