Make ToNumber(string) support binary and octal literals

RESOLVED FIXED in mozilla36

Status

()

RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: 446240525, Assigned: 446240525)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

Trunk
mozilla36
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [DocArea=JS])

Attachments

(1 attachment)

(Assignee)

Comment 1

5 years ago
Assignee: nobody → 446240525
Attachment #8500921 - Flags: review?(till)
Comment on attachment 8500921 [details] [diff] [review]
bug-1079120.patch

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

This looks great, thanks!

I'm not sufficiently convinced it is web compatible to just land it, though. Asking jorendorff for a decision on that. Please hold off on landing until then.

::: js/src/tests/ecma_6/Number/ToNumber.js
@@ +18,5 @@
> +assertEq(+"0o66", 54);
> +
> +if(typeof getSelfHostedValue === "function"){
> +    assertEq(getSelfHostedValue("ToNumber")("0b11"), 3);
> +    assertEq(getSelfHostedValue("ToNumber")("0o66"), 54);

Nice!
Attachment #8500921 - Flags: review?(till)
Attachment #8500921 - Flags: review+
Attachment #8500921 - Flags: feedback?(jorendorff)
I am pretty sure this is web compatible, Chrome seems to have this already.
(Assignee)

Comment 4

4 years ago
(In reply to Tom Schuster [:evilpie] from comment #3)
> I am pretty sure this is web compatible, Chrome seems to have this already.

Chrome has experimented this for one year, and seems no breakage reported so far.

https://codereview.chromium.org/19300002
https://codereview.chromium.org/626153002
Comment on attachment 8500921 [details] [diff] [review]
bug-1079120.patch

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

> Chrome has experimented this for one year, and seems no breakage reported so far.

Ok, that does convince me; let's land this.

ziyunfei, can you make sure the patch still applies and passes tests and then set checkin-needed?

::: js/src/tests/ecma_6/Number/ToNumber.js
@@ +18,5 @@
> +assertEq(+"0o66", 54);
> +
> +if(typeof getSelfHostedValue === "function"){
> +    assertEq(getSelfHostedValue("ToNumber")("0b11"), 3);
> +    assertEq(getSelfHostedValue("ToNumber")("0o66"), 54);

Nice!
Attachment #8500921 - Flags: feedback?(jorendorff)
(Assignee)

Comment 6

4 years ago
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=1a0a13975435
Keywords: dev-doc-needed → checkin-needed, dev-doc-complete
https://hg.mozilla.org/mozilla-central/rev/9e3582d234c1
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.