-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Generated arginfo header files: remove empty zend_function_entry arrays #15705
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
Conversation
When a class (or enum) has no methods, rather than using an array that only contains `ZEND_FE_END`, use `NULL` for the functions. The implementation of class registration for internal classes, `do_register_internal_class()` in zend_API.c, already skips classes where the functions are `NULL`. By removing these unneeded arrays, we can reduce the size of the header files, while also removing an unneeded call to zend_register_functions() for each internal class with no extra methods.
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.
ack for ext/random
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.
Ack for my stuff.
In general seems right overall, but needs more ACKs of course.
I actually thought about doing this myself some time ago, but never got to it. Thanks for this.
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.
good to go for sockets, pgsql, pcntl, intl, gd
ACK for pdo, pdo_dblib, pdo_odbc, odbc, mysqli and sqlite3.
Thank you!
BTW, as one of 8.4 RM, I believe this is a okay to include in 8.4 as it is not a userland impacting change.
ACK for pdo, pdo_dblib, pdo_odbc, odbc, mysqli and sqlite3. Thank you!
BTW, as one of 8.4 RM, I believe this is a okay to include in 8.4 as it is not a userland impacting change.
Glad to hear it can be included - I have a few other ideas for improvements to the generated arginfo files but to avoid merge conflicts I'm going to wait until after this is (hopefully) merged before sending the next one.
I wonder if a:
#define {$functionEntryName} NULL
or
static const *{$functionEntryName} = NULL;
would have been preferable. I've currently got a case of an extension that currently doesn't have any global functions, thus requiring me to explicitly pass NULL in the zend_module_entry. Should any global function be added in the future, I would need to remember to not just change the stub file, but also the zend_module_entry to make use of ext_functions there.
@TimWolla Yes, I think your idea makes a lot of sense!
When a class (or enum) has no methods, rather than using an array that only contains
ZEND_FE_END, useNULLfor the functions. The implementation of class registration for internal classes,do_register_internal_class()in zend_API.c, already skips classes where the functions areNULL. By removing these unneeded arrays, we can reduce the size of the header files, while also removing an unneeded call to zend_register_functions() for each internal class with no extra methods.