tc39 / proposal-set-methods

Proposal for new Set methods in JS
https://tc39.github.io/proposal-set-methods/
Other
655 stars 15 forks source link

Inconsistent editorial style for removing elements in Set.prototype.difference #99

Closed syg closed 7 months ago

syg commented 1 year ago

In difference, the "receiver is <= in size than argument" path removes elements from the result set by setting the removed element in [[SetData]] to ~empty~, while the "receiver is > in size than argument" path removes elements with the "remove" verb, which AFAICT we don't really do. Was this a conscious choice?

bakkot commented 1 year ago

Was this a conscious choice?

Yes, but I see why it's confusing. The idea is that the result sets are not observable until the methods return, so there's no possible outstanding iterators, which are what normally necessitates the ~empty~ tombstones. But in difference specifically, we're iterating over the set by index, and the index would be invalidated if we removed an element without having a tombstone.

I can change it to be more consistent. Do you have a preference between the two styles?

syg commented 1 year ago

Ah I see, makes sense. I think the observability part is a bit subtle. If you have no objections against making consistent, I prefer the use of ~empty~ tombstones everywhere. I am also fine with it remaining as it is (which I am fine with, given the explanation), though that still strikes me as optimizing a spec implementation that is already asymptotically bad, so why bother.

bakkot commented 1 year ago

See https://github.com/tc39/proposal-set-methods/pull/100