Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editorial note about Map/Set equality algorithm needs updating #3473

Open
Josh-Cena opened this issue Nov 9, 2024 · 9 comments · May be fixed by #3490
Open

Editorial note about Map/Set equality algorithm needs updating #3473

Josh-Cena opened this issue Nov 9, 2024 · 9 comments · May be fixed by #3490

Comments

@Josh-Cena
Copy link
Contributor

After #3337, Map/Set key equality no longer uses SameValueZero. The editorial note in several places need updating:

FWIW, I am much in favor of sticking to SameValueZero in the algorithms, because it helps readers understand better what's going on even without realizing that keys are canonicalized, but in the current form it seems to present more confusions. See also mdn/content#36698.

@bakkot bakkot added the editor call to be discussed in the next editor call label Nov 9, 2024
@bakkot
Copy link
Contributor

bakkot commented Nov 9, 2024

Eh... I think it's fine to describe the way Maps compare keys as "using SameValueZero" even though mechanically the algorithms are written to canonicalize and then use SameValue. There's not actually a difference.

@Josh-Cena
Copy link
Contributor Author

But either way people are confused:

  • If the editorial note says "SameValueZero" then people will say "but the actual algorithm doesn't!"
  • If the editorial note says "SameValue" then it's easily misread without a caveat everywhere that keys are canonicalized, which seems more heavyweight.

OTOH if the algorithm uses SameValueZero, we ease all these mental burdens for free.

@bakkot
Copy link
Contributor

bakkot commented Nov 10, 2024

At the cost of not using the same operation when adding/deleting vs querying, yes, but that's confusing in a different way.

Thoughts on adding a NOTE above the various if _p_.[[Key]] is _key_ steps which says something like "The use of CanonicalizeKeyedCollectionKey above makes this equivalent to SameValueZero"?

@Josh-Cena
Copy link
Contributor Author

If I shall ask: how is adding/deleting vs querying different if we need to canonicalize every input anyway?

@bakkot
Copy link
Contributor

bakkot commented Nov 10, 2024

I'm afraid I don't understand the question; can you rephrase?

If you're asking why we need to canonicalize when adding, it's because otherwise we'd have to canonicalize when iterating over the contents of the data structure, and it's conceptually cleaner to do it on input rather than output.

@Josh-Cena
Copy link
Contributor Author

I'm asking about this:

At the cost of not using the same operation when adding/deleting vs querying

I was supposing that the keys are canonicalized at the input boundary, so I'm not sure why these methods would not all be able to use SameValueZero.

@bakkot
Copy link
Contributor

bakkot commented Nov 10, 2024

If the keys are canonicalized at the input boundary then there is no reason to use SameValueZero. SameValueZero is just SameValue plus special handling for -0, but if you canonicalize then -0 can't be an input.

@Josh-Cena
Copy link
Contributor Author

Yes, but our point is that although SameValueZero and SameValue are operationally equivalent in this case, using the former in the algorithm lowers a lot of cognitive burden for readers, because if you use SameValue, then explaining it as either SameValueZero or SameValue in prose doesn't make much sense.

@bakkot
Copy link
Contributor

bakkot commented Nov 20, 2024

#3490

@bakkot bakkot removed the editor call to be discussed in the next editor call label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants