Issue #1739 - Implement numeric separators.

Resolves #1739

5 files changed, 194 insertions, 49 deletions

diff --git a/js/src/frontend/TokenStream.cpp b/js/src/frontend/TokenStream.cpp
index 7a253cc0e9..083bcd5045 100644
--- a/js/src/frontend/TokenStream.cpp
+++ b/js/src/frontend/TokenStream.cpp
@@ -1476,21 +1476,45 @@ TokenStream::getTokenInternal(TokenKind* ttp, Modifier modifier)
     // Look for a decimal number.
     //
     if (c1kind == Dec) {
+        MOZ_ASSERT(JS7_ISDEC(c));
         tp = newToken(-1);
         numStart = userbuf.addressOfNextRawChar() - 1;
-
-      decimal:
         decimalPoint = NoDecimal;
         hasExp = false;
-        while (JS7_ISDEC(c))
+        do {
             c = getCharIgnoreEOL();
+            if (JS7_ISDEC(c))
+                continue;
+            if (c != '_')
+                break;
+            c = getCharIgnoreEOL();
+            if (!JS7_ISDEC(c)) {
+                ungetCharIgnoreEOL(c);
+                reportError(JSMSG_MISSING_DIGIT_AFTER_SEPARATOR);
+                goto error;
+            }
+        } while (true);
 
+      decimal_rest:
         if (c == '.') {
             decimalPoint = HasDecimal;
-          decimal_dot:
-            do {
-                c = getCharIgnoreEOL();
-            } while (JS7_ISDEC(c));
+            c = getCharIgnoreEOL();
+            if (JS7_ISDEC(c)) {
+              decimal_dot:
+                do {
+                    c = getCharIgnoreEOL();
+                    if (JS7_ISDEC(c))
+                        continue;
+                    if (c != '_')
+                        break;
+                    c = getCharIgnoreEOL();
+                    if (!JS7_ISDEC(c)) {
+                        ungetCharIgnoreEOL(c);
+                        reportError(JSMSG_MISSING_DIGIT_AFTER_SEPARATOR);
+                        goto error;
+                    }
+                } while (true);
+            }
         }
         if (c == 'e' || c == 'E') {
             hasExp = true;
@@ -1504,7 +1528,17 @@ TokenStream::getTokenInternal(TokenKind* ttp, Modifier modifier)
             }
             do {
                 c = getCharIgnoreEOL();
-            } while (JS7_ISDEC(c));
+                if (JS7_ISDEC(c))
+                    continue;
+                if (c != '_')
+                    break;
+                c = getCharIgnoreEOL();
+                if (!JS7_ISDEC(c)) {
+                    ungetCharIgnoreEOL(c);
+                    reportError(JSMSG_MISSING_DIGIT_AFTER_SEPARATOR);
+                    goto error;
+                }
+            } while (true);
         }
         ungetCharIgnoreEOL(c);
 
@@ -1533,8 +1567,7 @@ TokenStream::getTokenInternal(TokenKind* ttp, Modifier modifier)
             if (!GetDecimalInteger(cx, numStart, userbuf.addressOfNextRawChar(), &dval))
                 goto error;
         } else {
-            const char16_t* dummy;
-            if (!js_strtod(cx, numStart, userbuf.addressOfNextRawChar(), &dummy, &dval))
+            if (!GetDecimalNonInteger(cx, numStart, userbuf.addressOfNextRawChar(), &dval))
                 goto error;
         }
         tp->type = TOK_NUMBER;
@@ -1576,8 +1609,19 @@ TokenStream::getTokenInternal(TokenKind* ttp, Modifier modifier)
                 goto error;
             }
             numStart = userbuf.addressOfNextRawChar() - 1;  // one past the '0x'
-            while (JS7_ISHEX(c))
+            do {
                 c = getCharIgnoreEOL();
+                if (JS7_ISHEX(c))
+                    continue;
+                if (c != '_')
+                    break;
+                c = getCharIgnoreEOL();
+                if (!JS7_ISHEX(c)) {
+                    ungetCharIgnoreEOL(c);
+                    reportError(JSMSG_MISSING_DIGIT_AFTER_SEPARATOR);
+                    goto error;
+                }
+            } while (true);
         } else if (c == 'b' || c == 'B') {
             radix = 2;
             c = getCharIgnoreEOL();
@@ -1587,8 +1631,19 @@ TokenStream::getTokenInternal(TokenKind* ttp, Modifier modifier)
                 goto error;
             }
             numStart = userbuf.addressOfNextRawChar() - 1;  // one past the '0b'
-            while (c == '0' || c == '1')
+            do {
                 c = getCharIgnoreEOL();
+                if (c == '0' || c == '1')
+                    continue;
+                if (c != '_')
+                    break;
+                c = getCharIgnoreEOL();
+                if (c != '0' && c != '1') {
+                    ungetCharIgnoreEOL(c);
+                    reportError(JSMSG_MISSING_DIGIT_AFTER_SEPARATOR);
+                    goto error;
+                }
+            } while (true);
         } else if (c == 'o' || c == 'O') {
             radix = 8;
             c = getCharIgnoreEOL();
@@ -1598,33 +1653,50 @@ TokenStream::getTokenInternal(TokenKind* ttp, Modifier modifier)
                 goto error;
             }
             numStart = userbuf.addressOfNextRawChar() - 1;  // one past the '0o'
-            while ('0' <= c && c <= '7')
+            do {
+                c = getCharIgnoreEOL();
+                if ('0' <= c && c <= '7')
+                    continue;
+                if (c != '_')
+                    break;
                 c = getCharIgnoreEOL();
+                if (c < '0' || c > '7') {
+                    ungetCharIgnoreEOL(c);
+                    reportError(JSMSG_MISSING_DIGIT_AFTER_SEPARATOR);
+                    goto error;
+                }
+            } while (true);
         } else if (JS7_ISDEC(c)) {
+            // Octal integer literals are not permitted in strict mode.
+            // Maybe one day we can get rid of this base-8 madness for good.
+            if (!reportStrictModeError(JSMSG_DEPRECATED_OCTAL))
+                goto error;
+
             radix = 8;
             numStart = userbuf.addressOfNextRawChar() - 1;  // one past the '0'
+            bool nonOctalDecimalIntegerLiteral = false;
             while (JS7_ISDEC(c)) {
-                // Octal integer literals are not permitted in strict mode code.
-                if (!reportStrictModeError(JSMSG_DEPRECATED_OCTAL))
-                    goto error;
-
-                // Outside strict mode, we permit 08 and 09 as decimal numbers,
-                // which makes our behaviour a superset of the ECMA numeric
-                // grammar. We might not always be so permissive, so we warn
-                // about it.
-                if (c >= '8') {
-                    if (!warning(JSMSG_BAD_OCTAL, c == '8' ? "08" : "09"))
-                        goto error;
-
-                    // Use the decimal scanner for the rest of the number.
-                    goto decimal;
-                }
+                if (c >= '8')
+                    nonOctalDecimalIntegerLiteral = true;
                 c = getCharIgnoreEOL();
             }
+
+            if (c == '_') {
+                reportError(JSMSG_INVALID_NUMERIC_SEPARATOR);
+                goto error;
+            }
+            if (nonOctalDecimalIntegerLiteral) {
+                // Use the decimal scanner for the rest of the number.
+                decimalPoint = NoDecimal;
+                hasExp = false;
+                goto decimal_rest;
+            }
         } else {
             // '0' not followed by 'x', 'X' or a digit;  scan as a decimal number.
             numStart = userbuf.addressOfNextRawChar() - 1;
-            goto decimal;
+            decimalPoint = NoDecimal;
+            hasExp = false;
+            goto decimal_rest;
         }
         ungetCharIgnoreEOL(c);
 
@@ -1647,8 +1719,11 @@ TokenStream::getTokenInternal(TokenKind* ttp, Modifier modifier)
 
         double dval;
         const char16_t* dummy;
-        if (!GetPrefixInteger(cx, numStart, userbuf.addressOfNextRawChar(), radix, &dummy, &dval))
+        if (!GetPrefixInteger(cx, numStart, userbuf.addressOfNextRawChar(), radix,
+                              PrefixIntegerSeparatorHandling::SkipUnderscore, &dummy, &dval))
+        {
             goto error;
+        }
         tp->type = TOK_NUMBER;
         tp->setNumber(dval, NoDecimal);
         goto out;
diff --git a/js/src/js.msg b/js/src/js.msg
index 587481864d..b03da4153d 100644
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -205,7 +205,6 @@ MSG_DEF(JSMSG_BAD_GENERATOR_RETURN,    0, JSEXN_TYPEERR,   "generator function c
 MSG_DEF(JSMSG_BAD_GENEXP_BODY,         1, JSEXN_SYNTAXERR, "illegal use of {0} in generator expression")
 MSG_DEF(JSMSG_BAD_INCOP_OPERAND,       0, JSEXN_SYNTAXERR, "invalid increment/decrement operand")
 MSG_DEF(JSMSG_BAD_METHOD_DEF,          0, JSEXN_SYNTAXERR, "bad method definition")
-MSG_DEF(JSMSG_BAD_OCTAL,               1, JSEXN_SYNTAXERR, "{0} is not a legal ECMA-262 octal constant")
 MSG_DEF(JSMSG_BAD_POW_LEFTSIDE,        0, JSEXN_SYNTAXERR, "unparenthesized unary expression can't appear on the left-hand side of '**'")
 MSG_DEF(JSMSG_BAD_PROP_ID,             0, JSEXN_SYNTAXERR, "invalid property id")
 MSG_DEF(JSMSG_BAD_RETURN_OR_YIELD,     1, JSEXN_SYNTAXERR, "{0} not in function")
@@ -270,6 +269,7 @@ MSG_DEF(JSMSG_OF_AFTER_FOR_LOOP_DECL,  0, JSEXN_SYNTAXERR, "a declaration in the
 MSG_DEF(JSMSG_IN_AFTER_LEXICAL_FOR_DECL,0,JSEXN_SYNTAXERR, "a lexical declaration in the head of a for-in loop can't have an initializer")
 MSG_DEF(JSMSG_INVALID_FOR_IN_DECL_WITH_INIT,0,JSEXN_SYNTAXERR,"for-in loop head declarations may not have initializers")
 MSG_DEF(JSMSG_INVALID_ID,              1, JSEXN_SYNTAXERR, "{0} is an invalid identifier")
+MSG_DEF(JSMSG_INVALID_NUMERIC_SEPARATOR, 0, JSEXN_SYNTAXERR, "numeric separators are not allowed in \"0\"-prefixed octal literals")
 MSG_DEF(JSMSG_LABEL_NOT_FOUND,         0, JSEXN_SYNTAXERR, "label not found")
 MSG_DEF(JSMSG_LET_COMP_BINDING,        0, JSEXN_SYNTAXERR, "'let' is not a valid name for a comprehension variable")
 MSG_DEF(JSMSG_LEXICAL_DECL_NOT_IN_BLOCK,   1, JSEXN_SYNTAXERR, "{0} declaration not directly within block")
@@ -281,6 +281,7 @@ MSG_DEF(JSMSG_LINE_BREAK_AFTER_THROW,  0, JSEXN_SYNTAXERR, "no line break is all
 MSG_DEF(JSMSG_LINE_BREAK_BEFORE_ARROW, 0, JSEXN_SYNTAXERR, "no line break is allowed before '=>'")
 MSG_DEF(JSMSG_MALFORMED_ESCAPE,        1, JSEXN_SYNTAXERR, "malformed {0} character escape sequence")
 MSG_DEF(JSMSG_MISSING_BINARY_DIGITS,   0, JSEXN_SYNTAXERR, "missing binary digits after '0b'")
+MSG_DEF(JSMSG_MISSING_DIGIT_AFTER_SEPARATOR, 0, JSEXN_SYNTAXERR, "missing digit after '_' numeric separator")
 MSG_DEF(JSMSG_MISSING_EXPONENT,        0, JSEXN_SYNTAXERR, "missing exponent")
 MSG_DEF(JSMSG_MISSING_EXPR_AFTER_THROW,0, JSEXN_SYNTAXERR, "throw statement is missing an expression")
 MSG_DEF(JSMSG_MISSING_FORMAL,          0, JSEXN_SYNTAXERR, "missing formal parameter")
diff --git a/js/src/jsnum.cpp b/js/src/jsnum.cpp
index c15b3de0d1..549596e579 100644
--- a/js/src/jsnum.cpp
+++ b/js/src/jsnum.cpp
@@ -71,12 +71,16 @@ ComputeAccurateDecimalInteger(ExclusiveContext* cx, const CharT* start, const Ch
     if (!cstr)
         return false;
 
+    size_t j = 0;
     for (size_t i = 0; i < length; i++) {
         char c = char(start[i]);
+        if (c == '_') {
+          continue;
+        }
         MOZ_ASSERT(('0' <= c && c <= '9') || ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z'));
-        cstr[i] = c;
+        cstr[j++] = c;
     }
-    cstr[length] = 0;
+    cstr[j] = 0;
 
     char* estr;
     int err = 0;
@@ -111,8 +115,14 @@ class BinaryDigitReader
         if (digitMask == 0) {
             if (start == end)
                 return -1;
-
             int c = *start++;
+            if (c == '_') {
+                // Skip over one _ and one _ only.
+                if (start == end)
+                    return -1;
+                c = *start++;
+            }
+
             MOZ_ASSERT(('0' <= c && c <= '9') || ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z'));
             if ('0' <= c && c <= '9')
                 digit = c - '0';
@@ -209,7 +219,8 @@ js::ParseDecimalNumber(const mozilla::Range<const char16_t> chars);
 template <typename CharT>
 bool
 js::GetPrefixInteger(ExclusiveContext* cx, const CharT* start, const CharT* end, int base,
-                     const CharT** endp, double* dp)
+                     PrefixIntegerSeparatorHandling separatorHandling, const CharT** endp,
+                     double* dp)
 {
     MOZ_ASSERT(start <= end);
     MOZ_ASSERT(2 <= base && base <= 36);
@@ -225,6 +236,8 @@ js::GetPrefixInteger(ExclusiveContext* cx, const CharT* start, const CharT* end,
             digit = c - 'a' + 10;
         else if ('A' <= c && c <= 'Z')
             digit = c - 'A' + 10;
+        else if (c == '_' && separatorHandling == PrefixIntegerSeparatorHandling::SkipUnderscore)
+            continue;
         else
             break;
         if (digit >= base)
@@ -242,7 +255,7 @@ js::GetPrefixInteger(ExclusiveContext* cx, const CharT* start, const CharT* end,
     /*
      * Otherwise compute the correct integer from the prefix of valid digits
      * if we're computing for base ten or a power of two.  Don't worry about
-     * other bases; see 15.1.2.2 step 13.
+     * other bases; see ES2018, 18.2.5 `parseInt(string, radix)`, step 13.
      */
     if (base == 10)
         return ComputeAccurateDecimalInteger(cx, start, s, dp);
@@ -255,11 +268,13 @@ js::GetPrefixInteger(ExclusiveContext* cx, const CharT* start, const CharT* end,
 
 template bool
 js::GetPrefixInteger(ExclusiveContext* cx, const char16_t* start, const char16_t* end, int base,
-                     const char16_t** endp, double* dp);
+                     PrefixIntegerSeparatorHandling separatorHandling, const char16_t** endp,
+                     double* dp);
 
 template bool
-js::GetPrefixInteger(ExclusiveContext* cx, const Latin1Char* start, const Latin1Char* end,
-                     int base, const Latin1Char** endp, double* dp);
+js::GetPrefixInteger(ExclusiveContext* cx, const Latin1Char* start, const Latin1Char* end, int base,
+                     PrefixIntegerSeparatorHandling separatorHandling, const Latin1Char** endp,
+                     double* dp);
 
 bool
 js::GetDecimalInteger(ExclusiveContext* cx, const char16_t* start, const char16_t* end, double* dp)
@@ -270,6 +285,8 @@ js::GetDecimalInteger(ExclusiveContext* cx, const char16_t* start, const char16_
     double d = 0.0;
     for (; s < end; s++) {
         char16_t c = *s;
+        if (c == '_')
+            continue;
         MOZ_ASSERT('0' <= c && c <= '9');
         int digit = c - '0';
         d = d * 10 + digit;
@@ -285,6 +302,36 @@ js::GetDecimalInteger(ExclusiveContext* cx, const char16_t* start, const char16_
     return ComputeAccurateDecimalInteger(cx, start, s, dp);
 }
 
+bool
+js::GetDecimalNonInteger(ExclusiveContext* cx, const char16_t* start, const char16_t* end, double* dp)
+{
+    MOZ_ASSERT(start <= end);
+
+    size_t length = end - start;
+    Vector<char, 32> chars(cx);
+    if (!chars.growByUninitialized(length + 1))
+        return false;
+
+    const char16_t* s = start;
+    size_t i = 0;
+    for (; s < end; s++) {
+        char16_t c = *s;
+        if (c == '_')
+            continue;
+        MOZ_ASSERT(('0' <= c && c <= '9') || c == '.' || c == 'e' || c == 'E' || c == '+' ||
+                   c == '-');
+        chars[i++] = char(c);
+    }
+    chars[i] = 0;
+
+    char* ep;
+    int err; // unused
+    *dp = js_strtod_harder(cx->dtoaState(), chars.begin(), &ep, &err);
+    MOZ_ASSERT(ep >= chars.begin());
+
+    return true;
+}
+
 static bool
 num_parseFloat(JSContext* cx, unsigned argc, Value* vp)
 {
@@ -355,7 +402,7 @@ ParseIntImpl(JSContext* cx, const CharT* chars, size_t length, bool stripPrefix,
     /* Steps 11-15. */
     const CharT* actualEnd;
     double d;
-    if (!GetPrefixInteger(cx, s, end, radix, &actualEnd, &d))
+    if (!GetPrefixInteger(cx, s, end, radix, PrefixIntegerSeparatorHandling::None, &actualEnd, &d))
         return false;
 
     if (s == actualEnd)
@@ -1322,7 +1369,7 @@ CharsToNumber(ExclusiveContext* cx, const CharT* chars, size_t length, double* r
              */
             const CharT* endptr;
             double d;
-            if (!GetPrefixInteger(cx, bp + 2, end, radix, &endptr, &d) ||
+            if (!GetPrefixInteger(cx, bp + 2, end, radix, PrefixIntegerSeparatorHandling::None, &endptr, &d) ||
                 endptr == bp + 2 ||
                 SkipSpace(endptr, end) != end)
             {
diff --git a/js/src/jsnum.h b/js/src/jsnum.h
index ed8e9d18e4..f169a9c38d 100644
--- a/js/src/jsnum.h
+++ b/js/src/jsnum.h
@@ -131,31 +131,51 @@ template <typename CharT>
 extern double
 ParseDecimalNumber(const mozilla::Range<const CharT> chars);
 
+/* Separator handling for integers. Either skip underscores for numerical
+ * separators (ES2021) or not
+ */
+enum class PrefixIntegerSeparatorHandling : bool {
+    None, SkipUnderscore
+};
+
 /*
  * Compute the positive integer of the given base described immediately at the
- * start of the range [start, end) -- no whitespace-skipping, no magical
+ * start of the range [start, end] -- no whitespace-skipping, no magical
  * leading-"0" octal or leading-"0x" hex behavior, no "+"/"-" parsing, just
  * reading the digits of the integer.  Return the index one past the end of the
  * digits of the integer in *endp, and return the integer itself in *dp.  If
  * base is 10 or a power of two the returned integer is the closest possible
  * double; otherwise extremely large integers may be slightly inaccurate.
  *
- * If [start, end) does not begin with a number with the specified base,
- * *dp == 0 and *endp == start upon return.
+ * The |separatorHandling| controls whether or not numeric separators can be
+ * part of an integer string. If the option is enabled, all individual '_'
+ * characters in the string are ignored.
+ *
+ * If [start, end] does not begin with a number with the specified base,
+ * then upon return *dp == 0 and *endp == start.
  */
 template <typename CharT>
 extern MOZ_MUST_USE bool
 GetPrefixInteger(ExclusiveContext* cx, const CharT* start, const CharT* end, int base,
-                 const CharT** endp, double* dp);
+                 PrefixIntegerSeparatorHandling separatorHandling, const CharT** endp,
+                 double* dp);
 
 /*
- * This is like GetPrefixInteger, but only deals with base 10, and doesn't have
- * and |endp| outparam.  It should only be used when the characters are known to
- * only contain digits.
+ * This is like GetPrefixInteger, but only deals with base 10, always ignores
+ * '_', and doesn't have an |endp| outparam.  It should only be used when the
+ * characters are known to only contain digits and '_'.
  */
 extern MOZ_MUST_USE bool
 GetDecimalInteger(ExclusiveContext* cx, const char16_t* start, const char16_t* end, double* dp);
 
+/*
+ * This is like GetDecimalInteger, but also allows non-integer numbers. It
+ * should only be used when the characters are known to only contain digits,
+ * '.', 'e' or 'E', '+' or '-', and '_'.
+ */
+extern MOZ_MUST_USE bool
+GetDecimalNonInteger(ExclusiveContext* cx, const char16_t* start, const char16_t* end, double* dp);
+
 extern MOZ_MUST_USE bool
 StringToNumber(ExclusiveContext* cx, JSString* str, double* result);
 
diff --git a/js/src/vm/JSONParser.cpp b/js/src/vm/JSONParser.cpp
index 975fecb562..680a34fef0 100644
--- a/js/src/vm/JSONParser.cpp
+++ b/js/src/vm/JSONParser.cpp
@@ -288,8 +288,10 @@ JSONParser<CharT>::readNumber()
 
         double d;
         const CharT* dummy;
-        if (!GetPrefixInteger(cx, digitStart.get(), current.get(), 10, &dummy, &d))
+        if (!GetPrefixInteger(cx, digitStart.get(), current.get(), 10, 
+                              PrefixIntegerSeparatorHandling::None, &dummy, &d)) {
             return token(OOM);
+        }
         MOZ_ASSERT(current == dummy);
         return numberToken(negative ? -d : d);
     }