tc39 / proposal-set-methods

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

Set.prototype.intersection needs to check for duplicate elements in step 6 #83

Closed anba closed 1 year ago

anba commented 1 year ago

Calling otherRec.[[Has]] can modify O.[[SetData]] to re-add an already visited key.

Test case:

let set = new Set([1, 2]);

let setLike = {
  size: Infinity,
  has(v) {
    // Remove and then re-add 1.
    if (v === 2) {
      set.delete(1);
      set.add(1);
    }
    return true;
  },
  keys() {
    throw new Error("Unexpected call to |keys| method");
  },
};

set.intersection(setLike);
ljharb commented 1 year ago

In my polyfill implementation, this test case passes - what's the failure you see based on the spec?

bakkot commented 1 year ago

@ljharb The problem is that resultSetData, and therefore the final result.[[SetData]], will have a duplicate entry. That's not supposed to happen. And with the algorithms as written it would be observable by iterating over the resulting set and seeing a single value twice.

ljharb commented 1 year ago

ah ok, but because my implementation uses actual Set, the resulting set is deduped for me, but that won't necessarily be the case in engines. makes sense.