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

Fixed computed property in object literal with negative integers #1752

Merged

Conversation

andreabergia
Copy link
Contributor

There is a code path, in compiled mode only (not in interpreted!), where something like o = { [-1]: xxx } would not work correctly.

The problem is a combination of factors:

  • in the interpreter, we would treat -1 as a double, not an int;
  • but there are various places in ScriptRuntime where integer keys are allowed only for positive or zero numbers, per the spec - otherwise they need to be treated as strings.

We have modified the existing test for computed properties and fixed the code.

There is a code path, in compiled mode only (not in interpreted!),
where something like `o = { [-1]: xxx }` would not work correctly.

The problem is a combination of factors:
- in the interpreter, we would treat `-1` as a `double`, not an `int`;
- but there are various places in `ScriptRuntime` where integer keys
  are allowed _only_ for positive or zero numbers, per the spec -
  otherwise they need to be treated as strings.

We have modified the existing test for computed properties and fixed
the code.

Co-authored-by: Satish Srinivasan <satish.srinivasan@servicenow.com>
@andreabergia andreabergia mentioned this pull request Dec 9, 2024
@gbrail
Copy link
Collaborator

gbrail commented Dec 10, 2024

Yes, good catch, thank you!

@gbrail gbrail merged commit 9d33f5b into mozilla:master Dec 10, 2024
3 checks passed
@andreabergia andreabergia deleted the fix-computed-property-negative-number-opt0 branch December 10, 2024 07:24
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 this pull request may close these issues.

2 participants