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

Maps and sets incorrectly distinguish NaN values #888

Closed
gibson042 opened this issue Mar 30, 2022 · 1 comment
Closed

Maps and sets incorrectly distinguish NaN values #888

gibson042 opened this issue Mar 30, 2022 · 1 comment
Labels
confirmed issue reported has been reproduced fixed - please verify Issue has been fixed. Please verify and close.

Comments

@gibson042
Copy link

Environment: XS 11.6.0 32 4

Description XS incorrectly differentiates NaN values in maps and sets.

Steps to Reproduce

(new Set([NaN, Number("x"), Math.sqrt(-1), Infinity * 0, Infinity - Infinity])).size // => 2
(new Map([[NaN, true]])).get(Infinity / Infinity) // => undefined

Expected behavior
All NaN values should be indistinguishable from each other, with the first expression above returning 1 and the second returning true because both Map Objects and Set Objects specify that "distinct [key] values are discriminated using the SameValueZero comparison algorithm", which via Number::sameValueZero must return true when both operands are NaN.

Actual behavior
XS maps and sets differentiate NaN from constructed NaN values.

@phoddie phoddie added the confirmed issue reported has been reproduced label Mar 30, 2022
@phoddie
Copy link
Collaborator

phoddie commented Mar 30, 2022

Thank you!

XS normalizes zero for Map and Set keys here:

else if (XS_NUMBER_KIND == kind) {
if (slot->value.number == 0)
slot->value.number = 0;
sum = *((txU8*)&slot->value.number);
}

Extending that to also normalize NaN gives the expected result:

		else if (XS_NUMBER_KIND == kind) {
			if (slot->value.number == 0)
				slot->value.number = 0;
			else if (c_isnan(slot->value.number))
				slot->value.number = C_NAN;
			sum = *((txU8*)&slot->value.number);
		}

Some more careful review and test262 testing will be needed to be sure this is a correct fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed issue reported has been reproduced fixed - please verify Issue has been fixed. Please verify and close.
Projects
None yet
Development

No branches or pull requests

2 participants