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

Fix named function expression creation. #2634

Merged
merged 1 commit into from
Dec 12, 2018

Conversation

zherczeg
Copy link
Member

@zherczeg zherczeg commented Dec 6, 2018

Create a local lexical environment with the name of the function. While this is not too memory efficient, some corner cases requires its existence.

@zherczeg zherczeg changed the title [WIP] Fix named function expression creation. Fix named function expression creation. Dec 10, 2018
@zherczeg
Copy link
Member Author

I think this patch is ready for review.

@legendecas
Copy link
Contributor

Maybe you could add a test case to check if it works as expected?

@zherczeg
Copy link
Member Author

Good point. I forgot it.

tests/jerry/function-expr-named.js Show resolved Hide resolved
jerry-core/vm/vm.c Show resolved Hide resolved
tests/jerry/function-expr-named.js Show resolved Hide resolved
Copy link
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@akosthekiss akosthekiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in general. +1 to @rerobika 's suggestions to extend the test case.

Create a local lexical environment with the name of the function. While
this is not too memory efficient, some corner cases requires its existence.

JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com
@zherczeg
Copy link
Member Author

Thank you for the review. Tests added, even a new one as well.

Copy link
Member

@akosthekiss akosthekiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@akosthekiss akosthekiss merged commit 83ee9cf into jerryscript-project:master Dec 12, 2018
legendecas added a commit to yodaos-project/ShadowNode that referenced this pull request Dec 20, 2018
legendecas added a commit to yodaos-project/ShadowNode that referenced this pull request Dec 21, 2018
yorkie pushed a commit to yodaos-project/ShadowNode that referenced this pull request Dec 21, 2018
* Revert "jerry: ref to function self should not create new object (#368)"
* jerry: change literal status flags to enum
Picked from jerryscript-project/jerryscript#2474
* jerry: fix named function expression creation
picked from jerryscript-project/jerryscript#2634
* Include test sets
legendecas added a commit to yodaos-project/ShadowNode that referenced this pull request Dec 24, 2018
* Revert "jerry: ref to function self should not create new object (#368)"
* jerry: change literal status flags to enum
Picked from jerryscript-project/jerryscript#2474
* jerry: fix named function expression creation
picked from jerryscript-project/jerryscript#2634
* Include test sets
legendecas added a commit to yodaos-project/ShadowNode that referenced this pull request Dec 24, 2018
* Revert "jerry: ref to function self should not create new object (#368)"
* jerry: change literal status flags to enum
Picked from jerryscript-project/jerryscript#2474
* jerry: fix named function expression creation
picked from jerryscript-project/jerryscript#2634
* Include test sets
@zherczeg zherczeg deleted the named_func_exp branch January 8, 2019 07:33
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 this pull request may close these issues.

None yet

5 participants