Use lock for set iteration.

[joshwd36]

This Pull Request fixes the issue of concurrent modification of a set during iteration, similar to that in map in #1092.

It changes the following:

• Changes the key of OrderedSet to a SetKey, which contains either a JsValue or an empty value.
• Allows locking a set to allow safe iteration.
• When an entry is deleted while the set is locked, it replaces the entry with an empty value to maintain the indexes.
• When the set is unlocked any empty entries are deleted.
• Locks are implemented as a guard object so that they are released even if an error occurs.

This implementation is very similar to that of #1366, and so should not present any additional issues.

[Razican]

Test262 conformance changes:

Test result	master count	PR count	difference
Total	80,685	80,685	0
Passed	31,821	31,823	+2
Ignored	15,818	15,818	0
Failed	33,046	33,044	-2
Panics	2	2	0
Conformance	39.44%	39.44%	+0.00%

Fixed tests (2):

test/built-ins/Set/prototype/forEach/iterates-values-revisits-after-delete-re-add.js [strict mode] (previously Failed)
test/built-ins/Set/prototype/forEach/iterates-values-revisits-after-delete-re-add.js (previously Failed)

[jedel1043]

Couldn't this be implemented as rustc does, with type HashSet<K> = HashMap<K, ()>? Maybe I'm overlooking something, but this should avoid implementing the exact same logic by hand.

[joshwd36]

Couldn't this be implemented as rustc does, with type HashSet<K> = HashMap<K, ()>? Maybe I'm overlooking something, but this should avoid implementing the exact same logic by hand.

I did think of that, but there would have to be some differences as the behaviour when deleting is slightly different. I can investigate whether it would be worth it

[jedel1043]

Couldn't this be implemented as rustc does, with type HashSet<K> = HashMap<K, ()>? Maybe I'm overlooking something, but this should avoid implementing the exact same logic by hand.

I did think of that, but there would have to be some differences as the behaviour when deleting is slightly different. I can investigate whether it would be worth it

I investigated a little bit more and rustc in actuality uses a newtype wrapping a HashMap, making the definition of HashSet something more like struct HashSet<K>(HashMap<K, ()>). With this you can also customize the behaviour of delete, so I think it's worth considering.

[RageKnify]

Looks ok to me, but as @jedel1043 said, might be a good idea to make use of the NewTypes we have implemented for Map rather than having really similar ones here.

[RageKnify]

Hey @joshwd36, do you have time to get back to this? Could you explain the downsides to not using the struct implemented for the Map.

[Razican]

I'm closing this for now, since it's not being updated. Feel free to open a new PR with a rebased proposal.