Closed Bug 952870 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: till, Assigned: till)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [DocArea=JS][qa-])

Attachments

(1 file)

The ES6 spec changed to treat negative and positive 0 as the same key for 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.
Attachment #8351069 - Flags: review?(jorendorff)
The spec still allows SameValue-style keying as an option, correct?  Do we implement that yet?
SameValue is what we implement now. The option is a second argument. We currently ignore the second argument.
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.
Attachment #8351069 - Flags: review?(jorendorff) → review+
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.
https://hg.mozilla.org/mozilla-central/rev/30d4356308c7
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
(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.
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); });

?
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...)
Whiteboard: [DocArea=JS] → [DocArea=JS][qa-]
You need to log in before you can comment on or make changes to this bug.