-
Couldn't load subscription status.
- Fork 8k
zend_object_handler.c: call zend_get_call_trampoline_func() directly #20297
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
base: master
Are you sure you want to change the base?
Conversation
Abstracting away the bool parameter is not that useful, and doesn't make the code more legible. So just get rid of them and call the function with the boolean parameter directly.
Every single callsite already checks if it set to call this function, so no need to duplicate the work
|
|
||
| ZEND_API zend_function *zend_get_call_trampoline_func(const zend_class_entry *ce, zend_string *method_name, bool is_static) /* {{{ */ | ||
| ZEND_API ZEND_ATTRIBUTE_NONNULL zend_function *zend_get_call_trampoline_func( | ||
| const zend_function *magic_call, zend_string *method_name, bool is_static) /* {{{ */ |
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.
The is_static parameter should also go away then.
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.
It is used to add a flag within the function body, and there is no convenient way to extract this information otherwise.
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.
You should be able to take this from magic_call->common.fn_flags just like the attribute flags.
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.
This probably saves few re-loads but this won't make any visible effect for the expensive operation (function call).
I'm not sure if the modified API becomes more clear. Especially if we keep is_static argument.
|
I'm in favor if we can get rid of the |
Abstracting away the bool parameter is not that useful, and doesn't make the code more legible.
So just get rid of them and call the function with the boolean parameter directly.