-
Notifications
You must be signed in to change notification settings - Fork 687
Rework function call. #2414
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
Rework function call. #2414
Conversation
|
Please add a test for this. |
f80dd6a to
f4b2bea
Compare
|
Test case added. |
| static ecma_value_t | ||
| ecma_op_function_construct_simple_or_external (ecma_object_t *func_obj_p, /**< Function object */ | ||
| ecma_op_function_construct_ecma_or_external (ecma_object_t *func_obj_p, /**< Function object */ | ||
| const ecma_value_t *arguments_list_p, /**< arguments list */ |
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.
wrong indentation
| { | ||
| if (JERRY_UNLIKELY (ecma_get_object_is_builtin (func_obj_p))) | ||
| { | ||
| return ecma_builtin_dispatch_call (func_obj_p, |
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.
Built-in function objects should never have a construct_flag, but it would be nice to see a JERRY_ASSERT (!ecma_op_function_has_construct_flag (arguments_list_p)) before returning with ecma_builtin_dispatch_call.
This case currently seems impossible but the assertion could prevent the wrong usage of the ecma_op_function_set_construct_flag in the future.
Furthermore add a construct flag, which disallows calling certain functions without new. Constructing bound arrow functions correctly throws error now. JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com
f4b2bea to
a59cf4f
Compare
|
Thank you for the comments, patch is updated. |
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
|
LGTM (informally) |
yichoi
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
Furthermore add a construct flag, which disallows calling certain functions without new. Constructing bound arrow functions correctly throws error now. Author: Zoltan Herczeg<zherczeg.u-szeged@partner.samsung.com> PR-URL: jerryscript-project/jerryscript#2414
Furthermore add a construct flag, which disallows calling certain functions without new. Constructing bound arrow functions correctly throws error now. Author: Zoltan Herczeg<zherczeg.u-szeged@partner.samsung.com> PR-URL: jerryscript-project/jerryscript#2414
Furthermore add a construct flag, which disallows calling certain functions without new. Constructing bound arrow functions correctly throws error now. Author: Zoltan Herczeg<zherczeg.u-szeged@partner.samsung.com> PR-URL: jerryscript-project/jerryscript#2414
Furthermore add a construct flag, which disallows calling certain functions without new. Constructing bound arrow functions correctly throws error now.
It seems the change has close to no effect:
Binary: 50 byte reduction, perf: -0.004%, mem: 0.000%