zend_API: Add Z_PARAM_ENUM_NAME() #20898
Conversation
TimWolla
requested review from
DanielEScherzer,
devnexen,
dstogov,
kocsismate,
ndossche and
zeriyoshi
as code owners
It is becoming increasingly common to have enum parameters on internal functions. Manually turning the singleton enum object into the case name is verbose, especially for optional parameters. Add a new `Z_PARAM_ENUM_NAME()` helper to directly retrieve the case name.
TimWolla
force-pushed
the
z-param-enum-name
branch
from
873e4c7 to
43e9fd5
Compare
Girgias
left a comment
Girgias
left a comment
There was a problem hiding this comment.
Makes sense to me. Just a few questions regarding const qualifiers.
| php_random_randomizer *randomizer = Z_RANDOM_RANDOMIZER_P(ZEND_THIS); | ||
| double min, max; | ||
| zend_object *bounds = NULL; | ||
| zend_string *bounds = NULL; |
There was a problem hiding this comment.
Why not mark this as const as you did with other usages?
| zend_string *bounds = NULL; | |
| const zend_string *bounds = NULL; |
There was a problem hiding this comment.
Probably related to the order of replacements I made some file reminded me that this could be const. I'll fix this up before merging.
| PHP_FUNCTION(pcntl_setqos_class) | ||
| { | ||
| zval *qos_obj; | ||
| zend_string *qos; |
There was a problem hiding this comment.
Ditto
| zend_string *qos; | |
| const zend_string *qos; |
ndossche
left a comment
ndossche
left a comment
There was a problem hiding this comment.
No further remarks from my side. This is nicer.
I wonder whether we should also add the _OR_NULL & _EX variants etc.
I feel that a nullable enum is something that is rarely needed, since the "null case" could just be part of the enum with a proper name. |
3 participants
It is becoming increasingly common to have enum parameters on internal functions. Manually turning the singleton enum object into the case name is verbose, especially for optional parameters.
Add a new
Z_PARAM_ENUM_NAME()helper to directly retrieve the case name.