Use zend_is_iterable #3146
Use zend_is_iterable #3146
Conversation
|
LGTM |
|
I'm not sure what advantage there is of making this change ? |
|
@krakjoe This was the last check for an |
| @@ -2229,7 +2229,7 @@ static xmlNodePtr to_xml_array(encodeTypePtr type, zval *data, int style, xmlNod | |||
| return xmlParam; | |||
| } | |||
|
|
|||
| if (Z_TYPE_P(data) == IS_OBJECT && instanceof_function(Z_OBJCE_P(data), zend_ce_traversable)) { | |||
| if (Z_TYPE_P(data) == IS_OBJECT && zend_is_iterable(data)) { | |||
There was a problem hiding this comment.
Maybe it could be replaced by just if (zend_is_iterable(data)) and moving array check (L2290:2455) to top of this part?
There was a problem hiding this comment.
That's not going to work, since code in this branch requires only an object (zend_is_iterable will return true for arrays, too).
Edit: changing zend_is_iterable to not accept all iterables is going to be a bit of a misnomer, also.
There was a problem hiding this comment.
Closing this PR as performance and check were affected :(
There was a problem hiding this comment.
@tpunt true, but it would fail even if it was an array as the first part of the conditional can never be true at that point, but I do agree we should evaluate each case before changing it, alternatively we can have some macros to cover common usages like the one in question here
No description provided.