-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
Parser Panic #240
Comments
Thanks @adumbidiot |
Would it be helpful to try to replace some of the panics in the lexer with the |
@jasonwilliams I guess a LexerError is when the user inputs something that is not allowed as per JS specification. That will show him that his code is invalid. What happens here is that ` is not implemented in Boa, so I think "panic" is ok, as it is actually bugs out in Boa before realizing it is invalid as per JS syntax. If template literals were implemented it should show LexerError, something like: |
I still kinda feel like the engine should still return an error. Just as older engines reject newer syntax with lexing and parsing errors and such, I feel like Boa should reject syntax it doesn't support with errors. |
It should be possible to return an Err(LexerError) here: Then perform a match here: printing a stderr with the message from the Err() if there is one. |
This has now been fixed, thanks @adumbidiot |
I feel like a lot more work has to be done here; My pull only fixed some of the lexer panics and none of the parser panics. |
@adumbidiot have you tried this against the new parser? |
New parser! Plus loads of tidy up in various places. Co-authored-by: Jason Williams <jwilliams720@bloomberg.net> Co-authored-by: HalidOdat <halidodat@gmail.com> Co-authored-by: Iban Eguia <iban.eguia@cern.ch> Co-authored-by: Iban Eguia <razican@protonmail.ch>
I'm going to close this issue as the main problem in question has been fixed, and some other parser issues have been resolved. If there are new issues we should make a ticket for them |
I want to re-open this, since it seems this is now also panicking. We should never panic, and checking what this gives in Firefox, for example, I can see the following:
So, let's re-open it and track it, since we get this panic:
It seems that in the test262 branch this is fixed:
|
When putting the following in test.js:
var`a = 1;
running the command
cargo run -- test.js
will give the following panic:This should probably be an error, not a panic.
The text was updated successfully, but these errors were encountered: