Skip to content

Conversation

erikjwaxx
Copy link

This merge request passes the array key as a third parameter to an array_reduce callback.

Copy link
Member

Choose a reason for hiding this comment

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

You can use ZEND_HASH_FOREACH_KEY_VAL(ht, num, str, val) here, then if str == NULL it's a num key.

Copy link
Author

Choose a reason for hiding this comment

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

That's a good point. No sense in dereferencing things from the bucket when there are macros to do it for you :).

…KEY_VAL to iterate directly over (key, value) pairs instead of dereferencing them from the containing bucket.
Copy link
Member

Choose a reason for hiding this comment

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

And also ZVAL_STR(&args[2], zend_string_copy(arr_str_key));

Copy link
Member

Choose a reason for hiding this comment

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

Or ZVAL_STR_COPY(&args[2], arr_str_key).

Copy link
Member

Choose a reason for hiding this comment

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

yeah, this is better

@nikic
Copy link
Member

nikic commented Mar 7, 2015

Passing an additional parameter to the reduce callback is a BC break, please open a discussion in the internals mailing list about this.

@krakjoe
Copy link
Member

krakjoe commented Jan 4, 2017

Since this has merge conflicts, and since an internals discussion never materialised, it seems the author has abandoned this proposal, so I'm closing this PR.

Please take this action as encouragement to do as requested, open an internals discussion, if consensus emerges from that, please open a clean PR.

@krakjoe krakjoe closed this Jan 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants