refactor(ast): fix BigInt memory leak by removing it

relates We'll need to evaluate the value by other means.
oxc-project · Feb 4, 2024 · 864d08a · 864d08a
1 parent b5e43fb
commit 864d08a
Show file tree

Hide file tree

Showing 13 changed files with 42 additions and 53 deletions.
diff --git a/crates/oxc_ast/Cargo.toml b/crates/oxc_ast/Cargo.toml
@@ -25,7 +25,6 @@ oxc_index     = { workspace = true }
 
 bitflags   = { workspace = true }
 num-bigint = { workspace = true }
-num-traits = { workspace = true }
 
 serde      = { workspace = true, features = ["derive"], optional = true }
 serde_json = { workspace = true, optional = true }

diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs
@@ -247,13 +247,11 @@ impl<'a> Expression<'a> {
     /// Returns literal's value converted to the Boolean type
     /// returns `true` when node is truthy, `false` when node is falsy, `None` when it cannot be determined.
     pub fn get_boolean_value(&self) -> Option<bool> {
-        use num_traits::Zero;
-
         match self {
             Self::BooleanLiteral(lit) => Some(lit.value),
             Self::NullLiteral(_) => Some(false),
             Self::NumberLiteral(lit) => Some(lit.value != 0.0),
-            Self::BigintLiteral(lit) => Some(!lit.value.is_zero()),
+            Self::BigintLiteral(lit) => Some(!lit.is_zero()),
             Self::RegExpLiteral(_) => Some(true),
             Self::StringLiteral(lit) => Some(!lit.value.is_empty()),
             _ => None,
@@ -453,7 +451,7 @@ impl<'a> PropertyKey<'a> {
                 Expression::StringLiteral(lit) => Some(lit.value.clone()),
                 Expression::RegExpLiteral(lit) => Some(Atom::from(lit.regex.to_string())),
                 Expression::NumberLiteral(lit) => Some(Atom::from(lit.value.to_string())),
-                Expression::BigintLiteral(lit) => Some(Atom::from(lit.value.to_string())),
+                Expression::BigintLiteral(lit) => Some(lit.raw.clone()),
                 Expression::NullLiteral(_) => Some("null".into()),
                 Expression::TemplateLiteral(lit) => {
                     lit.expressions.is_empty().then(|| lit.quasi()).flatten().cloned()

diff --git a/crates/oxc_ast/src/ast/literal.rs b/crates/oxc_ast/src/ast/literal.rs
@@ -6,7 +6,6 @@ use std::{
 };
 
 use bitflags::bitflags;
-use num_bigint::BigInt;
 use oxc_span::{Atom, Span};
 use oxc_syntax::{BigintBase, NumberBase};
 #[cfg(feature = "serde")]
@@ -107,18 +106,23 @@ impl<'a> Hash for NumberLiteral<'a> {
     }
 }
 
-#[derive(Debug, Clone, Hash)]
+#[derive(Debug, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize), serde(tag = "type"))]
 #[cfg_attr(all(feature = "serde", feature = "wasm"), derive(tsify::Tsify))]
 pub struct BigintLiteral {
     #[cfg_attr(feature = "serde", serde(flatten))]
     pub span: Span,
-    #[cfg_attr(feature = "serde", serde(serialize_with = "crate::serialize::serialize_bigint"))]
-    pub value: BigInt,
+    pub raw: Atom,
     #[cfg_attr(feature = "serde", serde(skip))]
     pub base: BigintBase,
 }
 
+impl BigintLiteral {
+    pub fn is_zero(&self) -> bool {
+        self.raw == "0n"
+    }
+}
+
 #[derive(Debug, Clone, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize), serde(tag = "type"))]
 #[cfg_attr(all(feature = "serde", feature = "wasm"), derive(tsify::Tsify))]

diff --git a/crates/oxc_ast/src/ast_builder.rs b/crates/oxc_ast/src/ast_builder.rs
@@ -5,7 +5,6 @@
     clippy::unused_self,
 )]
 
-use num_bigint::BigInt;
 use std::mem;
 
 use oxc_allocator::{Allocator, Box, String, Vec};
@@ -136,8 +135,8 @@ impl<'a> AstBuilder<'a> {
         BooleanLiteral { span, value }
     }
 
-    pub fn bigint_literal(&self, span: Span, value: BigInt, base: BigintBase) -> BigintLiteral {
-        BigintLiteral { span, value, base }
+    pub fn bigint_literal(&self, span: Span, raw: Atom, base: BigintBase) -> BigintLiteral {
+        BigintLiteral { span, raw, base }
     }
 
     pub fn template_literal(

diff --git a/crates/oxc_ast/src/ast_kind.rs b/crates/oxc_ast/src/ast_kind.rs
@@ -539,7 +539,7 @@ impl<'a> AstKind<'a> {
             Self::StringLiteral(s) => format!("NumberLiteral({})", s.value).into(),
             Self::BooleanLiteral(b) => format!("BooleanLiteral({})", b.value).into(),
             Self::NullLiteral(_) => "NullLiteral".into(),
-            Self::BigintLiteral(b) => format!("BigintLiteral({})", b.value).into(),
+            Self::BigintLiteral(b) => format!("BigintLiteral({})", b.raw).into(),
             Self::RegExpLiteral(r) => format!("RegExpLiteral({})", r.regex).into(),
             Self::TemplateLiteral(t) => format!(
                 "TemplateLiteral({})",

diff --git a/crates/oxc_ast/src/serialize.rs b/crates/oxc_ast/src/serialize.rs
@@ -1,5 +1,3 @@
-use std::fmt;
-
 use serde::{ser::Serializer, Serialize};
 
 use crate::ast::{Program, RegExpFlags};
@@ -28,14 +26,6 @@ impl<'a> Program<'a> {
     }
 }
 
-pub fn serialize_bigint<T, S>(value: &T, s: S) -> Result<S::Ok, S::Error>
-where
-    T: fmt::Display,
-    S: serde::Serializer,
-{
-    s.collect_str(&format_args!("{value}n"))
-}
-
 impl Serialize for RegExpFlags {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where

diff --git a/crates/oxc_codegen/Cargo.toml b/crates/oxc_codegen/Cargo.toml
@@ -24,8 +24,7 @@ oxc_span      = { workspace = true }
 oxc_allocator = { workspace = true }
 oxc_syntax    = { workspace = true }
 
-bitflags   = { workspace = true }
-num-bigint = { workspace = true }
+bitflags = { workspace = true }
 
 [dev-dependencies]
 oxc_parser = { workspace = true }
diff --git a/crates/oxc_codegen/src/gen.rs b/crates/oxc_codegen/src/gen.rs
@@ -1062,13 +1062,11 @@ fn print_non_negative_float<const MINIFY: bool>(value: f64, _p: &Codegen<{ MINIF
 
 impl<const MINIFY: bool> Gen<MINIFY> for BigintLiteral {
     fn gen(&self, p: &mut Codegen<{ MINIFY }>, _ctx: Context) {
-        use num_bigint::Sign;
-
-        if self.value.sign() == Sign::Minus {
-            p.print_space_before_operator(Operator::Unary(UnaryOperator::UnaryNegation));
+        if self.raw.contains('_') {
+            p.print_str(self.raw.replace('_', "").as_bytes());
+        } else {
+            p.print_str(self.raw.as_bytes());
         }
-        p.print_str(self.value.to_string().as_bytes());
-        p.print(b'n');
     }
 }
 

diff --git a/crates/oxc_linter/Cargo.toml b/crates/oxc_linter/Cargo.toml
@@ -38,7 +38,6 @@ serde         = { workspace = true }
 regex         = { workspace = true }
 rustc-hash    = { workspace = true }
 phf           = { workspace = true, features = ["macros"] }
-num-traits    = { workspace = true }
 itertools     = { workspace = true }
 dashmap       = { workspace = true }
 convert_case  = { workspace = true }

diff --git a/crates/oxc_linter/src/rules/eslint/no_compare_neg_zero.rs b/crates/oxc_linter/src/rules/eslint/no_compare_neg_zero.rs
@@ -58,7 +58,6 @@ impl NoCompareNegZero {
 }
 
 fn is_neg_zero(expr: &Expression) -> bool {
-    use num_traits::Zero;
     let Expression::UnaryExpression(unary) = expr.get_inner_expression() else {
         return false;
     };
@@ -67,7 +66,7 @@ fn is_neg_zero(expr: &Expression) -> bool {
     }
     match &unary.argument {
         Expression::NumberLiteral(number) => number.value == 0.0,
-        Expression::BigintLiteral(bigint) => bigint.value.is_zero(),
+        Expression::BigintLiteral(bigint) => bigint.is_zero(),
         _ => false,
     }
 }

diff --git a/crates/oxc_minifier/src/compressor/ast_util.rs b/crates/oxc_minifier/src/compressor/ast_util.rs
@@ -390,7 +390,10 @@ pub fn get_bigint_value(expr: &Expression) -> Option<BigInt> {
                 None
             }
         }
-        Expression::BigintLiteral(bigint_literal) => Some(bigint_literal.value.clone()),
+        Expression::BigintLiteral(_bigint_literal) => {
+            // TODO: evaluate the bigint value
+            None
+        }
         Expression::BooleanLiteral(bool_literal) => {
             if bool_literal.value {
                 Some(BigInt::one())
@@ -465,7 +468,7 @@ pub fn get_boolean_value(expr: &Expression) -> Option<bool> {
         Expression::NullLiteral(_) => Some(false),
         Expression::BooleanLiteral(boolean_literal) => Some(boolean_literal.value),
         Expression::NumberLiteral(number_literal) => Some(number_literal.value != 0.0),
-        Expression::BigintLiteral(big_int_literal) => Some(!big_int_literal.value.is_zero()),
+        Expression::BigintLiteral(big_int_literal) => Some(!big_int_literal.is_zero()),
         Expression::StringLiteral(string_literal) => Some(!string_literal.value.is_empty()),
         Expression::TemplateLiteral(template_literal) => {
             // only for ``
@@ -581,7 +584,7 @@ pub fn get_string_value<'a>(expr: &'a Expression) -> Option<Cow<'a, str>> {
             Some(Cow::Owned(number_literal.value.to_string()))
         }
         Expression::BigintLiteral(big_int_literal) => {
-            Some(Cow::Owned(format!("{}n", big_int_literal.value)))
+            Some(Cow::Owned(big_int_literal.raw.to_string()))
         }
         Expression::NullLiteral(_) => Some(Cow::Borrowed("null")),
         Expression::BooleanLiteral(bool_literal) => {

diff --git a/crates/oxc_minifier/src/compressor/fold.rs b/crates/oxc_minifier/src/compressor/fold.rs
@@ -2,7 +2,7 @@
 //!
 //! <https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/PeepholeFoldConstants.java>
 
-use std::{cmp::Ordering, mem, ops::Not};
+use std::{cmp::Ordering, mem};
 
 use num_bigint::BigInt;
 #[allow(clippy::wildcard_imports)]
@@ -674,13 +674,12 @@ impl<'a> Compressor<'a> {
                         );
                         return Some(self.ast.literal_number_expression(literal));
                     }
-                    Expression::BigintLiteral(big_int_literal) => {
-                        use std::ops::Neg;
-
-                        let value = big_int_literal.value.clone().neg();
-                        let literal =
-                            self.ast.bigint_literal(unary_expr.span, value, big_int_literal.base);
-                        return Some(self.ast.literal_bigint_expression(literal));
+                    Expression::BigintLiteral(_big_int_literal) => {
+                        // let value = big_int_literal.value.clone().neg();
+                        // let literal =
+                        // self.ast.bigint_literal(unary_expr.span, value, big_int_literal.base);
+                        // return Some(self.ast.literal_bigint_expression(literal));
+                        return None;
                     }
                     Expression::Identifier(ident) => {
                         if ident.name == "NaN" {
@@ -705,11 +704,12 @@ impl<'a> Compressor<'a> {
                             return Some(self.ast.literal_number_expression(literal));
                         }
                     }
-                    Expression::BigintLiteral(big_int_literal) => {
-                        let value = big_int_literal.value.clone().not();
-                        let leteral =
-                            self.ast.bigint_literal(unary_expr.span, value, big_int_literal.base);
-                        return Some(self.ast.literal_bigint_expression(leteral));
+                    Expression::BigintLiteral(_big_int_literal) => {
+                        // let value = big_int_literal.value.clone().not();
+                        // let leteral =
+                        // self.ast.bigint_literal(unary_expr.span, value, big_int_literal.base);
+                        // return Some(self.ast.literal_bigint_expression(leteral));
+                        return None;
                     }
                     Expression::Identifier(ident) => {
                         if ident.name == "NaN" {

diff --git a/crates/oxc_parser/src/js/expression.rs b/crates/oxc_parser/src/js/expression.rs
@@ -309,11 +309,12 @@ impl<'a> Parser<'a> {
             _ => return Err(self.unexpected()),
         };
         let token = self.cur_token();
-        let src = self.cur_src().strip_suffix('n').unwrap();
-        let value = parse_big_int(src, token.kind)
+        let raw = self.cur_src();
+        let src = raw.strip_suffix('n').unwrap();
+        let _value = parse_big_int(src, token.kind)
             .map_err(|err| diagnostics::InvalidNumber(err, token.span()))?;
         self.bump_any();
-        Ok(self.ast.bigint_literal(self.end_span(span), value, base))
+        Ok(self.ast.bigint_literal(self.end_span(span), Atom::from(raw), base))
     }
 
     pub(crate) fn parse_literal_regexp(&mut self) -> RegExpLiteral {