Issue #1240 - Part 11 - Fix several issue reported on review.

Skip over block delimiters when parsing BigInt literals.
Update BigInt hashing to account for the possibility of moving GC.
https://bugzilla.mozilla.org/show_bug.cgi?id=1531018
Make HashableValue comparison of BigInts infallible.
https://bugzilla.mozilla.org/show_bug.cgi?id=1530406
Fix BigInt constructor API CallArgs usage.
https://bugzilla.mozilla.org/show_bug.cgi?id=1526279

4 files changed, 16 insertions, 32 deletions

diff --git a/js/src/builtin/BigInt.cpp b/js/src/builtin/BigInt.cpp
index f6c244ce39..6c78970d74 100644
--- a/js/src/builtin/BigInt.cpp
+++ b/js/src/builtin/BigInt.cpp
@@ -76,20 +76,6 @@ BigIntObject::unbox() const
     return getFixedSlot(PRIMITIVE_VALUE_SLOT).toBigInt();
 }
 
-bool
-js::intrinsic_ToBigInt(JSContext* cx, unsigned argc, Value* vp)
-{
-    CallArgs args = CallArgsFromVp(argc, vp);
-    MOZ_ASSERT(args.length() == 1);
-
-    BigInt* result = ToBigInt(cx, args[0]);
-    if (!result)
-        return false;
-
-    args.rval().setBigInt(result);
-    return true;
-}
-
 // BigInt proposal section 5.3.4
 bool
 BigIntObject::valueOf_impl(JSContext* cx, const CallArgs& args)
@@ -129,7 +115,7 @@ BigIntObject::toString_impl(JSContext* cx, const CallArgs& args)
     // Steps 4-5.
     if (args.hasDefined(0)) {
         double d;
-        if (!ToInteger(cx, args[0], &d))
+        if (!ToInteger(cx, args.get(0), &d))
             return false;
         if (d < 2 || d > 36) {
             JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_BAD_RADIX);
@@ -187,12 +173,12 @@ BigIntObject::asUintN(JSContext* cx, unsigned argc, Value* vp)
 
     // Step 1.
     uint64_t bits;
-    if (!ToIndex(cx, args[0], &bits)) {
+    if (!ToIndex(cx, args.get(0), &bits)) {
         return false;
     }
 
     // Step 2.
-    RootedBigInt bi(cx, ToBigInt(cx, args[1]));
+    RootedBigInt bi(cx, ToBigInt(cx, args.get(1)));
     if (!bi) {
         return false;
     }
@@ -215,12 +201,12 @@ BigIntObject::asIntN(JSContext* cx, unsigned argc, Value* vp)
 
     // Step 1.
     uint64_t bits;
-    if (!ToIndex(cx, args[0], &bits)) {
+    if (!ToIndex(cx, args.get(0), &bits)) {
         return false;
     }
 
     // Step 2.
-    RootedBigInt bi(cx, ToBigInt(cx, args[1]));
+    RootedBigInt bi(cx, ToBigInt(cx, args.get(1)));
     if (!bi) {
         return false;
     }
diff --git a/js/src/builtin/BigInt.h b/js/src/builtin/BigInt.h
index f1bf471ecb..6971549fc3 100644
--- a/js/src/builtin/BigInt.h
+++ b/js/src/builtin/BigInt.h
@@ -47,9 +47,6 @@ class BigIntObject : public NativeObject
 extern JSObject*
 InitBigIntClass(JSContext* cx, Handle<GlobalObject*> global);
 
-extern bool
-intrinsic_ToBigInt(JSContext* cx, unsigned argc, JS::Value* vp);
-
 } // namespace js
 
 #endif
diff --git a/js/src/builtin/MapObject.cpp b/js/src/builtin/MapObject.cpp
index 8bd87d8f16..fe748a6bde 100644
--- a/js/src/builtin/MapObject.cpp
+++ b/js/src/builtin/MapObject.cpp
@@ -82,7 +82,7 @@ HashValue(const Value& v, const mozilla::HashCodeScrambler& hcs)
     if (v.isSymbol())
         return v.toSymbol()->hash();
     if (v.isBigInt())
-        return v.toBigInt()->hash();
+        return MaybeForwarded(v.toBigInt())->hash();
     if (v.isObject())
         return hcs.scramble(v.asRawBits());
 
@@ -103,13 +103,9 @@ HashableValue::operator==(const HashableValue& other) const
     bool b = (value.asRawBits() == other.value.asRawBits());
 
     // BigInt values are considered equal if they represent the same
-    // integer. This test should use a comparison function that doesn't
-    // require a JSContext once one is defined in the BigInt class.
+    // mathematical value.
     if (!b && (value.isBigInt() && other.value.isBigInt())) {
-        JS::RootingContext* rcx = GetJSContextFromMainThread();
-        RootedValue valueRoot(rcx, value);
-        RootedValue otherRoot(rcx, other.value);
-        SameValue(nullptr, valueRoot, otherRoot, &b);
+        b = BigInt::equal(value.toBigInt(), other.value.toBigInt());
     }
 
 #ifdef DEBUG
@@ -390,8 +386,9 @@ MarkKey(Range& r, const HashableValue& key, JSTracer* trc)
     HashableValue newKey = key.mark(trc);
 
     if (newKey.get() != key.get()) {
-        // The hash function only uses the bits of the Value, so it is safe to
-        // rekey even when the object or string has been modified by the GC.
+        // The hash function must take account of the fact that the thing being
+        // hashed may have been moved by GC. This is only an issue for BigInt as for
+        // other types the hash function only uses the bits of the Value.
         r.rekeyFront(newKey);
     }
 }
diff --git a/js/src/vm/BigIntType.cpp b/js/src/vm/BigIntType.cpp
index 6beed9af11..7b8375526f 100644
--- a/js/src/vm/BigIntType.cpp
+++ b/js/src/vm/BigIntType.cpp
@@ -1391,10 +1391,14 @@ BigInt* BigInt::parseLiteralDigits(ExclusiveContext* cx,
 
   result->initializeDigitsToZero();
 
+  RangedPtr<const CharT> begin = start;
   for (; start < end; start++) {
     uint32_t digit;
     CharT c = *start;
-    if (c >= '0' && c < limit0) {
+    if (c == '_' && start > begin && start < end - 1) {
+      // skip over block delimiters unless at the very start or end
+      continue;
+    } else if (c >= '0' && c < limit0) {
       digit = c - '0';
     } else if (c >= 'a' && c < limita) {
       digit = c - 'a' + 10;