Skip to content

Conversation

@TimWolla
Copy link
Member

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.

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. Just a few questions regarding const qualifiers.

Copy link
Member

@ndossche ndossche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No further remarks from my side. This is nicer.
I wonder whether we should also add the _OR_NULL & _EX variants etc.

@TimWolla
Copy link
Member Author

I wonder whether we should also add the _OR_NULL

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.

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.
@arnaud-lb
Copy link
Member

This is better than what we do currently, but I'm wondering if we could improve even more, as converting enums to string is defeating their purpose, so internal code doesn't benefit from enums like user code does.

We could generate C enums from internal enums. We can then add a Z_PARAM_ENUM() that fetches the C enum value:

php_uri_enum_comparison_mode comparison_mode;

Z_PARAM_ENUM(comparison_mode, php_uri_ce_comparison_mode)

...

static void uri_equals(INTERNAL_FUNCTION_PARAMETERS, php_uri_object *that_object, php_uri_enum_comparison_mode comparison_mode)
{
    if (comparison_mode == PHP_ENUM_URI_COMPARISON_MODE_EXCLUDE_FRAGMENT) {

Another way would be to make it possible to compare enum instances by address:

// members are populated when enum cases are instantiated
ZEND_API struct {
    zend_object *exclude_fragment_case;
    zend_object *...;
} php_enum_uri_comparison_modes;

...

static void uri_equals(INTERNAL_FUNCTION_PARAMETERS, php_uri_object *that_object, zend_object *comparison_mode)
{
    if (comparison_mode == php_enum_uri_comparison_modes.exclude_fragment_case) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants