Given this code: `const uniqueSet = new Set([NaN, 0/0])` The content of `uniqueSet` is: `Set {NaN, NaN} (2)` This behaviour violates the step 2.a. of the SameValueZero spec which states: > "If x is NaN and y is NaN, return true." https://www.ecma-international.org/ecma-262/#sec-samevaluezero
I'm not sure but perhaps it's just a bug within the Set constructor.
FWIW the issue is not the constructor, rather the `.add(NaN)` followed by `.add(0/0)` or even `.add(Intinity/Infinity)`. Coming from here https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/runtime/SetPrototype.cpp#L102 I think the issue might be in here: https://github.com/WebKit/webkit/blob/dad62415730c8df3cbbe654eb15f0585e74af04c/Source/JavaScriptCore/runtime/HashMapImpl.h#L462-L470 where I suspect `areKeysEqual(exec, key, bucket->key())` might consider the globally available `NaN` and those produced by `0/0` or `Infinity/Infinity` different. The same applies for `Map` ``` const m = new Map; m.set(0/0, '0/0'); m.set(NaN, 'NaN'); m.get(0/0); m.get(NaN); ```
Created attachment 368446 [details] Modified testcase for Set.prototype.size Attaching a modified version of: `JSTests/es6/Set_Set.prototype.size.js` I suggest to add this change to cover this particular case. Sorry but I'm unable to run the `run-javascriptcore-tests` script from my machine due to the missing xcode dependency (cannot install the latest one on my macOS version), but I believe that with these changes it will fail.
(In reply to Andrea Giammarchi from comment #2) > FWIW the issue is not the constructor, rather the `.add(NaN)` followed by `.add(0/0)` or even `.add(Intinity/Infinity)`. > where I suspect `areKeysEqual(exec, key, bucket->key())` might consider the globally available `NaN` and those produced by `0/0` or `Infinity/Infinity` different. > The same applies for `Map` Agreed! Thanks for investigating further
I think the problem may be we are producing non pure NaN double JSValue.
(In reply to Yusuke Suzuki from comment #5) > I think the problem may be we are producing non pure NaN double JSValue. No. produced NaN is one of the pure NaN. What we want is normalizing NaN into one canonicalized value to calculate hash.
Created attachment 368498 [details] Patch
Comment on attachment 368498 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368498&action=review r=me > JSTests/stress/hash-table-with-normalized-nan.js:81 > +} an we also have a test for Map?
Comment on attachment 368498 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368498&action=review Thanks! >> JSTests/stress/hash-table-with-normalized-nan.js:81 >> +} > > an we also have a test for Map? Sounds nice, added.
Committed r244760: <https://trac.webkit.org/changeset/244760>
<rdar://problem/50313976>
*** Bug 197360 has been marked as a duplicate of this bug. ***
Committed r244763: <https://trac.webkit.org/changeset/244763>