-
Notifications
You must be signed in to change notification settings - Fork 675
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 arrow function parsing #5093
Conversation
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
class A{} |
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.
Do we need this line?
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.
Yes, without it an assertion fails before the bug would cause a segmentation fault:
ICE: Assertion 'context_p->scope_stack_size == PARSER_MAXIMUM_DEPTH_OF_SCOPE_STACK' failed at jerry-core/parser/js/js-parser-expr.c(parser_parse_class):1068
Error: JERRY_FATAL_FAILED_ASSERTION
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.
Well, this sounds like another bug. Assertions should never fail. Maybe another '}' related bug for classes.
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.
Oh I was mistaken, I was looking at a different branch.
With this patch applied this assertion does not fail with that line removed and the correct syntax error is raised.
jerry-core/parser/js/js-parser.c
Outdated
@@ -2829,6 +2829,7 @@ parser_parse_arrow_function (parser_context_t *context_p, /**< context */ | |||
JERRY_ASSERT (context_p->token.type == LEXER_ARROW); | |||
|
|||
lexer_next_token (context_p); | |||
bool reached_end_of_function = false; |
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.
I would call this next_token_needed
jerry-core/parser/js/js-parser.c
Outdated
@@ -2877,6 +2878,10 @@ parser_parse_arrow_function (parser_context_t *context_p, /**< context */ | |||
#endif /* JERRY_PARSER_DUMP_BYTE_CODE */ | |||
|
|||
parser_restore_context (context_p, &saved_context); | |||
if (reached_end_of_function) |
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.
Newline before.
78824da
to
43640ac
Compare
f577102
to
ae50054
Compare
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 fixes jerryscript-project#5085 JerryScript-DCO-1.0-Signed-off-by: Máté Tokodi mate.tokodi@szteszoftver.hu
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 #5085
JerryScript-DCO-1.0-Signed-off-by: Mate Tokodi mate.tokodi@szteszoftver.hu