Skip to content

Conversation

jorgsowa
Copy link
Contributor

Validates mode in the function array_filter() and creates complementary ARRAY_FILTER_USE_VALUE without exposing it as a PHP constant.

@bukka
Copy link
Member

bukka commented Sep 2, 2024

I think this might be safer to go through deprecation first.

@bukka
Copy link
Member

bukka commented Sep 2, 2024

Technically it's a BC break as the invalid mode is currently ignored.

@jorgsowa
Copy link
Contributor Author

jorgsowa commented Sep 2, 2024

Makes sense. I will wait for PHP 8.5. I was following a similar change with round() which didn't need RFC: #12252

@cmb69
Copy link
Member

cmb69 commented Oct 30, 2024

A deprecation is certainly more user friendly, but requiring the RFC process for this appears to be overkill. After all, users are not supposed to pass nonsense to well documented functions.

@bukka
Copy link
Member

bukka commented Nov 2, 2024

Well just that something users are not supposed to do, does not mean that it's not a BC break. In my opinion it is a BC break and as such we should go through the deprecation. I agree that doing RFC is overkill but I don't think we need RFC if we get some sort of agreement.

zval retval;
bool have_callback = 0;
zend_long use_type = 0;
zend_long mode = ARRAY_FILTER_USE_VALUE;
Copy link
Member

Choose a reason for hiding this comment

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

Please avoid the rename.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't mode a more relevant name than use_type that suggests boolean value?

This only touches internal variable, so I thought that shouldn't be a problem.

Copy link
Member

Choose a reason for hiding this comment

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

I think use_type refers to "ARRAY_FILTER_USE_", so mode doesn't seem more descriptive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mode is the literal name of the argument in the array_filter function.

https://www.php.net/manual/en/function.array-filter.php

For me, it doesn't make a big difference, rather than alignment, so I can change it tomorrow.

ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(array), num_key, string_key, operand) {
if (have_callback) {
if (use_type) {
if (mode) {
Copy link
Member

Choose a reason for hiding this comment

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

If you want to be explicit, you should use mode != ARRAY_FILTER_USE_VALUE, which should end up compiling to the same thing.

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