Skip to content
This repository has been archived by the owner on Jul 8, 2024. It is now read-only.

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

Closed
anba opened this issue Jan 11, 2023 · 3 comments · Fixed by #87
Closed

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

anba opened this issue Jan 11, 2023 · 3 comments · Fixed by #87

Comments

@anba
Copy link

anba commented Jan 11, 2023

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
Copy link
Member

ljharb commented Jan 11, 2023

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

@bakkot
Copy link
Collaborator

bakkot commented Jan 11, 2023

@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
Copy link
Member

ljharb commented Jan 11, 2023

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.

ljharb added a commit to es-shims/Set.prototype.intersection that referenced this issue Jan 11, 2023
ljharb added a commit to es-shims/Set.prototype.intersection that referenced this issue Jan 11, 2023
ljharb added a commit to es-shims/Set.prototype.intersection that referenced this issue Jan 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants