-
Notifications
You must be signed in to change notification settings - Fork 688
Fix "arguments" and "eval" parsing #2661
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
Fix "arguments" and "eval" parsing #2661
Conversation
|
Uh bad news. This is a side effect of my other patch which added types for strings. What other effects can it cause. I will be back next week, lets discuss this first please. |
4656ce7 to
a969402
Compare
|
I've investigated a bit more and you were right, the issues are due to #2634. I think I found an universal solution for it, but it's definitely worth a discussion. |
jerry-core/parser/js/js-lexer.c
Outdated
| context_p->status_flags |= PARSER_ARGUMENTS_NEEDED | PARSER_LEXICAL_ENV_NEEDED; | ||
| context_p->lit_object.literal_p->status_flags |= LEXER_FLAG_NO_REG_STORE; | ||
| context_p->lit_object.type = LEXER_LITERAL_OBJECT_ARGUMENTS; | ||
| if (!(context_p->status_flags & PARSER_ARGUMENTS_NOT_NEEDED)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps an ident check could work here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, it worked!
This patch fixes jerryscript-project#2656 and fixes jerryscript-project#2659 as well. JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik frobert@inf.u-szeged.hu
a969402 to
0bdfa1a
Compare
LaszloLango
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also fix #2699 in this PR.
|
@LaszloLango I've checked whether this PR solves #2699 but unfortunately it isn't since they are not related to each other (only the |
|
@rerobika, it is OK to do it in a new PR if they are not related. |
zherczeg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LaszloLango
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This patch fixes #2656 and fixes #2659 as well.
JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik frobert@inf.u-szeged.hu