-
Notifications
You must be signed in to change notification settings - Fork 684
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. |
@@ -639,27 +682,28 @@ ecma_op_function_call (ecma_object_t *func_obj_p, /**< Function object */ | |||
* Returned value must be freed with ecma_free_value | |||
*/ | |||
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 [email protected]
f4b2bea
to
a59cf4f
Compare
Thank you for the comments, patch is updated. |
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) |
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<[email protected]> 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<[email protected]> 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<[email protected]> 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%