Conversation
| zval *error; | ||
| ZEND_HASH_FOREACH_VAL(Z_ARRVAL(previous_errors), error) { |
There was a problem hiding this comment.
| zval *error; | |
| ZEND_HASH_FOREACH_VAL(Z_ARRVAL(previous_errors), error) { | |
| ZEND_HASH_FOREACH_VAL(Z_ARRVAL(previous_errors), zval *error) { |
| ZEND_ATTRIBUTE_NONNULL static const char *php_uri_parser_whatwg_build_errors(zval *errors) | ||
| { | ||
| ZEND_ASSERT(Z_TYPE_P(errors) == IS_ARRAY); |
There was a problem hiding this comment.
This should take a HashTable* directly instead of the assert.
There was a problem hiding this comment.
Also for php_uri_parser_whatwg_build_errors_into_exception, php_uri_parser_whatwg_build_errors_and_throw, php_uri_parser_whatwg_build_path etc. or only here?
There was a problem hiding this comment.
Yes, if possible, use HashTable * (sometimes I also forget about it).
As far as I can see, there's some assertions about errors being undef (e.g. in php_uri_pass_errors_by_ref_and_free(): ZEND_ASSERT(Z_TYPE_P(errors) == IS_UNDEF || Z_TYPE_P(errors) == IS_ARRAY);), but this one at least is likely a dead case, since apparently errors is always an array.
There was a problem hiding this comment.
although a more general refactoring shouldn't necessarily be done inside this PR, since it can become a rabbit hole.
Co-authored-by: Tim Düsterhus <209270+TimWolla@users.noreply.github.com>
|
Does this need |
3 participants
Aligns builder and parser error order. Parser is LIFO, which is consistent with Lexbor logs (but imo surprising).