952870 - Treat -0 and 0 as the same key in Maps and Sets

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Till Schneidereit [:till]]

The ES6 spec changed to treat negative and positive 0 as the same key for Maps and Sets

Comment 1

[Till Schneidereit [:till]]

Attachment: Treat -0 and 0 as the same key in Maps and Sets

This patch updates our implementation to match the spec.

I updated all tests, removing auto-regress/bug770954.js in the process because it was an exact duplicate of collections/Map-gc-4.js.

Comment 2

[Jeff Walden [:Waldo]]

The spec still allows SameValue-style keying as an option, correct?  Do we implement that yet?

Comment 3

[Jason Orendorff [:jorendorff]]

SameValue is what we implement now. The option is a second argument. We currently ignore the second argument.

Comment 4

[Jason Orendorff [:jorendorff]]

Comment on attachment 8351069 [details] [diff] [review]
Treat -0 and 0 as the same key in Maps and Sets

Review of attachment 8351069 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/builtin/MapObject.cpp
@@ +27,5 @@
>  using mozilla::Move;
>  using mozilla::ArrayLength;
>  using JS::DoubleNaNValue;
>  
> +

If you're going to remove this one, might as well remove them all.

This is a nicely organized file. But I don't think the form feeds are really necessary.

Comment 5

[Till Schneidereit [:till]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/30d4356308c7

> If you're going to remove this one, might as well remove them all.
> 
> This is a nicely organized file. But I don't think the form feeds are really
> necessary.

I didn't realize that this was on purpose. I don't quite understand the purpose and assume that it'll do something more meaningful than just showing up as a new line in vim or emacs. In any case, I reverted the change - it felt wrong to do drive-by changes of things I don't understand.

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/30d4356308c7

Comment 8

[Jason Orendorff [:jorendorff]]

(In reply to Till Schneidereit [:till] from comment #5)
> I didn't realize that this was on purpose. I don't quite understand the
> purpose and assume that it'll do something more meaningful than just showing
> up as a new line in vim or emacs.

http://www.emacswiki.org/emacs/PageBreaks

for whatever it's worth.

Comment 9

[4esn0k]

Hello,

May I ask, what will produce next code:

var map = new Map();
map.set(-0, 'x');
map.forEach(function (value, key) { alert(1 / key); });

?

Comment 10

[Jason Orendorff [:jorendorff]]

4esn0k: Good question. On es-discuss, Allen says that the key stored in the Map should be +0, not -0.
So that code should alert "Infinity".

(Till, do we already have a test for that specific thing? Might as well...)

Comment 11

[Florian Scholz (Open Web Docs)]

Developer release notes:
https://developer.mozilla.org/en-US/Firefox/Releases/29#JavaScript

Reference docs:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map#Key_equality
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set#Value_equality

As always, feel free to review or improve these docs in the MDN wiki. Much appreciated.