Support complete collection of set methods, including multi-argument forms#586
Support complete collection of set methods, including multi-argument forms#586tetromino wants to merge 3 commits intogoogle:masterfrom
Conversation
…forms Also update the docs and tests. Working towards google#584
adonovan
left a comment
adonovan
left a comment
There was a problem hiding this comment.
Thanks for contributing these operators.
| leaves the set `S` unchanged. | ||
|
|
||
| `intersection_update` fails if the set `S` is frozen or has active iterators, or | ||
| if any element of any of the `*others` is unhashable. |
There was a problem hiding this comment.
For the record, this precludes a short cut when S is empty.
There was a problem hiding this comment.
That seems reasonable? We usually prohibit no-op mutation attempts - for example, if a set is immutable, we don't allow trying to update it with an element that it already contains.
As suggested by @adonovan
… not iterators This allows preserving lhs iteration order and prevents unnecessary copying. Note that for intersection_update, we have to manually iterate over the lhs set's hash table to modify it in-place - normal iteration doesn't allow mutating the hash table. Also take the opportunity to rename Set.InsertAll to Update, to match the starlark API. Add tests to verify iteration order.
|
I've added a new Set.IntersectionUpdate() which preserves lhs iteration order - please take a look. Notably, since IntersectionUpdate needs to delete from the lhs hash table whilst iterating over it, it needs to iterate manually instead of using a normal iterator. I believe this is sound as long as the entry being deleted is an entry that has already been examined, and as long as we save the next pointer (since deletion will mutate the entry struct). |
2 participants
Also update the docs and tests.
Working towards #584