Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

codegen: remove underscore from bigint #7285

Closed
Boshen opened this issue Nov 14, 2024 · 1 comment · Fixed by #7367
Closed

codegen: remove underscore from bigint #7285

Boshen opened this issue Nov 14, 2024 · 1 comment · Fixed by #7367
Assignees

Comments

@Boshen
Copy link
Member

Boshen commented Nov 14, 2024

p.print_str(self.raw.as_str());

looks like it prints the raw val from the original source text

Good catch, underscore should be removed from codegen.

Originally posted by @Boshen in #7280 (comment)

@overlookmotel
Copy link
Contributor

Please also see #7254. Of the options outlined there, personally I'd tend towards the option of removing the raw field.

In my view, the ideal solution would be to parse the BigInt's value in the parser. At that point we are running through the source text byte-by-byte anyway.

I would propose:

  • Adding a value: &'a str field.
  • Removing the base field.
pub struct BigIntLiteral<'a> {
    pub span: Span,
    pub value: &'a str,
    pub raw: &'a str, // Leave for now, but likely remove later
}

In parser, parse it to a &str (without underscores, converted to decimal), using code like this:

impl<'a> From<&'a BigIntLiteral<'a>> for ESTreeLiteral<'a, ()> {
fn from(value: &'a BigIntLiteral) -> Self {
let src = &value.raw.strip_suffix('n').unwrap().cow_replace('_', "");
let src = match value.base {
BigintBase::Decimal => src,
BigintBase::Binary | BigintBase::Octal | BigintBase::Hex => &src[2..],
};
let radix = match value.base {
BigintBase::Decimal => 10,
BigintBase::Binary => 2,
BigintBase::Octal => 8,
BigintBase::Hex => 16,
};
let bigint = BigInt::from_str_radix(src, radix).unwrap();
Self {
span: value.span,
// BigInts can't be serialized to JSON
value: (),
raw: Some(value.raw.as_str()),
bigint: Some(bigint.to_string()),
regex: None,
}
}
}

Then codegen can print the value field.

A bit of background: Originally BigIntLiteral had a value: BigInt field. We had to remove it because BigInt stores data on the heap, so it's not possible to make it part of the AST, as that heap data didn't get freed when the AST is dropped (memory leak). So that's why we have to store the value as a string slice (which would be stored in arena).

However, removing the underscores in codegen is an improvement on how it is now - not a bad interim solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants