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

Number(<float>e<int>) should work #204

Closed
pop opened this issue Nov 5, 2019 · 2 comments · Fixed by #245
Closed

Number(<float>e<int>) should work #204

pop opened this issue Nov 5, 2019 · 2 comments · Fixed by #245

Comments

@pop
Copy link
Contributor

pop commented Nov 5, 2019

It looks like scientific notation numbers starting with a float compile to either undefined (positive float base) and when used in Number() triggers a panic.

For example:

Number(1e1) // Works
Number(-1e1) // Works
Number(1e-1) // Works
Number(-1e-1) // Works
Number(1.0e1) // Fails
Number(-1.0e1) // Fails
Number(1.0e-1) // Fails
Number(-1.0e-1) // Fails

This behavior works in Node.js as expected:

> Number(1.0e1)
10
> Number(1.0e-1)
0.1
> Number(-1.0e1)
-10
> Number(-1.0e-1)
-0.1

Number() is able to parse String inputs correctly, but raw scientific notation numbers with a float base trigger a panic. This may be a parsing issue as the errors look like this:

$ cargo run # Parsing `Number(1.0e1)`
    Finished dev [unoptimized + debuginfo] target(s) in 0.03s
     Running `target/debug/boa`
thread 'main' panicked at 'parsing failed: Expected([Punctuator(Comma), Punctuator(CloseParen)], Token { data: Identifier("e1"), pos: Position { column_number: 9, line_number: 5 } }, "function call arguments")', src/libcore/result.rs:1084:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

refs #182 implementing #34

@adumbidiot
Copy link
Contributor

I ran into this while trying to fix some panics in the lexer. It looks like the lexer is trying to lex the number as two separate ones. The above panic will no longer occur, but the parser throws an error.

@adumbidiot
Copy link
Contributor

I'm going to try fixing this

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

Successfully merging a pull request may close this issue.

2 participants