Closed Bug 894026 Opened 11 years ago Closed 11 years ago

Implement BinaryIntegerLiteral and OctalIntegerLiteral

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla25
Tracking Status
relnote-firefox --- -

People

(Reporter: rwaldron, Assigned: Waldo)

References

(Blocks 1 open bug)

Details

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

Attachments

(1 file)

http://people.mozilla.org/~jorendorff/es6-draft.html#sec-7.8.3

BinaryIntegerLiteral ::
  0b BinaryDigit
  0B BinaryDigit
  BinaryIntegerLiteral BinaryDigit

BinaryDigit :: one of
  0 1

OctalIntegerLiteral ::
  0o OctalDigit
  0O OctalDigit
  OctalIntegerLiteral OctalDigit
Note that the spec draft grammar is wrong:

https://bugs.ecmascript.org/show_bug.cgi?id=1583
Attached patch PatchSplinter Review
Temporarily blocked on other stuff, this was easy.
Attachment #780005 - Flags: review?
Assignee: general → jwalden+bmo
Attachment #780005 - Flags: review? → review?(till)
Comment on attachment 780005 [details] [diff] [review]
Patch

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

::: js/src/tests/ecma_6/Expressions/octal-literals.js
@@ +92,5 @@
> +{
> +  "use strict";
> +  return 0o755;
> +}
> +assertEq(strict(), 7 * 64 + 5 * 8 + 5);

Shouldn’t this throw a SyntaxError? “A conforming implementation, when processing strict mode code (see 10.1.1), must not extend the syntax of NumericLiteral to include OctalIntegerLiteral as described in B.1.1.”
I'm pretty sure that should be changed to LegacyOctalIntegerLiteral now -- the term in B.1.1 got renamed.
Comment on attachment 780005 [details] [diff] [review]
Patch

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

Amount of conviction I have in allowing "0O" as the octal prefix: 0O0. I see how it makes sense to be consistent with the other prefixes, though.

Otherwise: nice!
Attachment #780005 - Flags: review?(till) → review+
"[..] in allowing "0O" as the octal prefix making sense", even
Yeah, 0O0 is so not a good idea.  I sent mail about it, which finally made it through now after prodding (sounds like @mit.edu might be deemed a spam source by postini :-\ ):

https://mail.mozilla.org/pipermail/es-discuss/2013-July/032192.html

I'll sit on this a little bit til that discussion, if any, ensues a bit.  Also, earlier is better and all, but if 0O0 gets excluded, I'd rather not unleash that on aurora if possible, so I'm going to be careful about uplift when landing this.  ;-)
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #5)
> I'm pretty sure that should be changed to LegacyOctalIntegerLiteral now --
> the term in B.1.1 got renamed.

Ah, you’re probably right — in that case, the spec should be updated. https://bugs.ecmascript.org/show_bug.cgi?id=1596
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e1dd28ede5d

I'll try to get a blog post about this done soon.
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
Something in this push made Windows checktests mad. Backed out per our IRC conversation.
https://hg.mozilla.org/integration/mozilla-inbound/rev/bbf37166d07c
I fired up a Windows build, and the jsapi-tests executable (which failed in tbpl logs, although nobody noticed it just at the time) was crashing in PRMJ_ShutdownNow() code, or something similar, clearly implicating the JS_Init changes -- not this bug.  Relanded:

https://hg.mozilla.org/integration/mozilla-inbound/rev/77df61af1a84
https://hg.mozilla.org/mozilla-central/rev/77df61af1a84
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Blogrified:

http://whereswalden.com/2013/08/12/micro-feature-from-es6-now-in-firefox-aurora-and-nightly-binary-and-octal-numbers/

Still needs MDN docs, I'll try to get to them next time I face downtime while compiling, or similar.
Assuming this does not need QA. If this needs QA, please remove the [qa-] whiteboard tag and add the verifyme keyword.
Whiteboard: [qa-]
Whiteboard: [qa-] → [qa-][DocArea=JS]
Firefox 25 for developers
https://developer.mozilla.org/en-US/Firefox/Releases/25#JavaScript

Reference
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#Numeric_literals

As always, review and additions to these docs are more than welcome on the MDN wiki :-)
You need to log in before you can comment on or make changes to this bug.