Removed reference counted pointers from JsValue variants (#1866)

This Pull Request fixes/closes #1864.

It changes the following:

- Removed `JsBigInt` from `Const` nodes, using a boxed `BigInt` instead.
- Modifies the `JsObject` variant so that it has a similar structure to other variants, where the internal structure is private.

The size of `JsValue` stays in 2 64-bit words (in a 64-bit system at least), and the size of `Const` also stays the same.

I have noticed that we clone tokens too much in the parser, so I was thinking that we should implement a by-value getter for `kind()`. Something like `kind_unwrap()`.

boa_engine/src/bigint.rs

@@ -3,8 +3,7 @@
 use crate::{builtins::Number, Context, JsValue};
 use boa_gc::{unsafe_empty_trace, Finalize, Trace};
 use num_integer::Integer;
-use num_traits::pow::Pow;
-use num_traits::{FromPrimitive, One, ToPrimitive, Zero};
+use num_traits::{pow::Pow, FromPrimitive, One, ToPrimitive, Zero};
 use std::{
     fmt::{self, Display},
     ops::{Add, BitAnd, BitOr, BitXor, Div, Mul, Neg, Rem, Shl, Shr, Sub},
@@ -290,6 +289,15 @@ impl From<RawBigInt> for JsBigInt {
     }
 }
 
+impl From<Box<RawBigInt>> for JsBigInt {
+    #[inline]
+    fn from(value: Box<RawBigInt>) -> Self {
+        Self {
+            inner: value.into(),
+        }
+    }
+}
+
 impl From<i8> for JsBigInt {
     #[inline]
     fn from(value: i8) -> Self {

boa_engine/src/bytecompiler.rs

@@ -554,7 +554,7 @@ impl<'b> ByteCompiler<'b> {
                     )),
                     Const::Int(v) => self.emit_push_integer(*v),
                     Const::Num(v) => self.emit_push_rational(*v),
-                    Const::BigInt(v) => self.emit_push_literal(Literal::BigInt(v.clone())),
+                    Const::BigInt(v) => self.emit_push_literal(Literal::BigInt(v.clone().into())),
                     Const::Bool(true) => self.emit(Opcode::PushTrue, &[]),
                     Const::Bool(false) => self.emit(Opcode::PushFalse, &[]),
                     Const::Null => self.emit(Opcode::PushNull, &[]),

boa_engine/src/object/jsobject.rs

@@ -26,13 +26,17 @@ pub type RefMut<'a, T, U> = boa_gc::RefMut<'a, T, U>;
 
 /// Garbage collected `Object`.
 #[derive(Trace, Finalize, Clone, Default)]
-pub struct JsObject(Gc<boa_gc::Cell<Object>>);
+pub struct JsObject {
+    inner: Gc<boa_gc::Cell<Object>>,
+}
 
 impl JsObject {
     /// Create a new `JsObject` from an internal `Object`.
     #[inline]
     fn from_object(object: Object) -> Self {
-        Self(Gc::new(boa_gc::Cell::new(object)))
+        Self {
+            inner: Gc::new(boa_gc::Cell::new(object)),
+        }
     }
 
     /// Create a new empty `JsObject`, with `prototype` set to `JsValue::Null`
@@ -90,7 +94,7 @@ impl JsObject {
     /// This is the non-panicking variant of [`borrow`](#method.borrow).
     #[inline]
     pub fn try_borrow(&self) -> StdResult<Ref<'_, Object>, BorrowError> {
-        self.0.try_borrow().map_err(|_| BorrowError)
+        self.inner.try_borrow().map_err(|_| BorrowError)
     }
 
     /// Mutably borrows the object, returning an error if the value is currently borrowed.
@@ -101,7 +105,7 @@ impl JsObject {
     /// This is the non-panicking variant of [`borrow_mut`](#method.borrow_mut).
     #[inline]
     pub fn try_borrow_mut(&self) -> StdResult<RefMut<'_, Object, Object>, BorrowMutError> {
-        self.0.try_borrow_mut().map_err(|_| BorrowMutError)
+        self.inner.try_borrow_mut().map_err(|_| BorrowMutError)
     }
 
     /// Checks if the garbage collected memory is the same.
@@ -669,7 +673,7 @@ Cannot both specify accessors and a value or writable attribute",
 impl AsRef<boa_gc::Cell<Object>> for JsObject {
     #[inline]
     fn as_ref(&self) -> &boa_gc::Cell<Object> {
-        &*self.0
+        &*self.inner
     }
 }
 
@@ -798,7 +802,7 @@ impl Debug for JsObject {
         // Instead, we check if the object has appeared before in the entire graph. This means that objects will appear
         // at most once, hopefully making things a bit clearer.
         if !limiter.visited && !limiter.live {
-            f.debug_tuple("JsObject").field(&self.0).finish()
+            f.debug_tuple("JsObject").field(&self.inner).finish()
         } else {
             f.write_str("{ ... }")
         }

boa_engine/src/string.rs

@@ -9,6 +9,7 @@ use std::{
     marker::PhantomData,
     ops::Deref,
     ptr::{copy_nonoverlapping, NonNull},
+    rc::Rc,
 };
 
 const CONSTANTS_ARRAY: [&str; 127] = [
@@ -314,7 +315,7 @@ impl Inner {
 #[derive(Finalize)]
 pub struct JsString {
     inner: NonNull<Inner>,
-    _marker: PhantomData<std::rc::Rc<str>>,
+    _marker: PhantomData<Rc<str>>,
 }
 
 impl Default for JsString {

boa_engine/src/syntax/ast/constant.rs

@@ -7,10 +7,9 @@
 //! [spec]: https://tc39.es/ecma262/#sec-primary-expression-literals
 //! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Grammar_and_types#Literals
 
-use crate::JsBigInt;
-use boa_gc::{Finalize, Trace};
+use boa_gc::{unsafe_empty_trace, Finalize, Trace};
 use boa_interner::{Interner, Sym, ToInternedString};
-
+use num_bigint::BigInt;
 #[cfg(feature = "deser")]
 use serde::{Deserialize, Serialize};
 
@@ -25,7 +24,7 @@ use serde::{Deserialize, Serialize};
 /// [spec]: https://tc39.es/ecma262/#sec-primary-expression-literals
 /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Grammar_and_types#Literals
 #[cfg_attr(feature = "deser", derive(Serialize, Deserialize))]
-#[derive(Clone, Debug, Trace, Finalize, PartialEq)]
+#[derive(Clone, Debug, Finalize, PartialEq)]
 pub enum Const {
     /// A string literal is zero or more characters enclosed in double (`"`) or single (`'`) quotation marks.
     ///
@@ -74,7 +73,7 @@ pub enum Const {
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-terms-and-definitions-bigint-value
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Grammar_and_types#Numeric_literals
-    BigInt(JsBigInt),
+    BigInt(Box<BigInt>),
 
     /// The Boolean type has two literal values: `true` and `false`.
     ///
@@ -113,6 +112,12 @@ pub enum Const {
     Undefined,
 }
 
+// Safety: Const does not contain any objects which needs to be traced,
+// so this is safe.
+unsafe impl Trace for Const {
+    unsafe_empty_trace!();
+}
+
 impl From<Sym> for Const {
     fn from(string: Sym) -> Self {
         Self::String(string)
@@ -131,8 +136,14 @@ impl From<i32> for Const {
     }
 }
 
-impl From<JsBigInt> for Const {
-    fn from(i: JsBigInt) -> Self {
+impl From<BigInt> for Const {
+    fn from(i: BigInt) -> Self {
+        Self::BigInt(Box::new(i))
+    }
+}
+
+impl From<Box<BigInt>> for Const {
+    fn from(i: Box<BigInt>) -> Self {
         Self::BigInt(i)
     }
 }

boa_engine/src/syntax/lexer/number.rs

@@ -9,6 +9,8 @@ use crate::{
 };
 use boa_interner::Interner;
 use boa_profiler::Profiler;
+use num_bigint::BigInt;
+use num_traits::Zero;
 use std::{io::Read, str};
 
 /// Number literal lexing.
@@ -251,7 +253,7 @@ impl<R> Tokenizer<R> for NumberLiteral {
 
                         // DecimalBigIntegerLiteral '0n'
                         return Ok(Token::new(
-                            TokenKind::NumericLiteral(Numeric::BigInt(0.into())),
+                            TokenKind::NumericLiteral(Numeric::BigInt(BigInt::zero().into())),
                             Span::new(start_pos, cursor.pos()),
                         ));
                     }
@@ -380,7 +382,7 @@ impl<R> Tokenizer<R> for NumberLiteral {
         let num = match kind {
             NumericKind::BigInt(base) => {
                 Numeric::BigInt(
-                    JsBigInt::from_string_radix(num_str, base).expect("Could not convert to BigInt")
+                    BigInt::parse_bytes(num_str.as_bytes(), base).expect("Could not convert to BigInt").into()
                     )
             }
             NumericKind::Rational /* base: 10 */ => {

boa_engine/src/syntax/lexer/token.rs

@@ -5,13 +5,12 @@
 //!
 //! [spec]: https://tc39.es/ecma262/#sec-tokens
 
-use crate::{
-    syntax::ast::{Keyword, Punctuator, Span},
-    syntax::lexer::template::TemplateString,
-    JsBigInt,
+use crate::syntax::{
+    ast::{Keyword, Punctuator, Span},
+    lexer::template::TemplateString,
 };
 use boa_interner::{Interner, Sym};
-
+use num_bigint::BigInt;
 #[cfg(feature = "deser")]
 use serde::{Deserialize, Serialize};
 
@@ -71,7 +70,7 @@ pub enum Numeric {
     Integer(i32),
 
     // A BigInt
-    BigInt(JsBigInt),
+    BigInt(Box<BigInt>),
 }
 
 impl From<f64> for Numeric {
@@ -88,10 +87,10 @@ impl From<i32> for Numeric {
     }
 }
 
-impl From<JsBigInt> for Numeric {
+impl From<BigInt> for Numeric {
     #[inline]
-    fn from(n: JsBigInt) -> Self {
-        Self::BigInt(n)
+    fn from(n: BigInt) -> Self {
+        Self::BigInt(Box::new(n))
     }
 }