Add zend_argument_type_error_ex, zend_argument_error_ex#21829
Add zend_argument_type_error_ex, zend_argument_error_ex#21829arnaud-lb wants to merge 2 commits intophp:masterfrom
Conversation
Girgias
left a comment
There was a problem hiding this comment.
I think this makes a lot of sense :)
TimWolla
left a comment
There was a problem hiding this comment.
Either way LGTM, consider adding the new functions to UPGRADING.INTERNALS with a short note.
These variants take the function as parameter instead of infering it from the call stack. This is useful in ZEND_TYPE_ASSERT, and will be used in PFA impl.
| ZEND_API ZEND_COLD void zend_argument_error_ex(const zend_function *function, | ||
| uint32_t arg_num, zend_class_entry *error_ce, const char *format, ...) |
There was a problem hiding this comment.
I'm only noticing now that error_ce and arg_num are flipped compared to the existing zend_argument_error(). That should be adjusted.
| ZEND_API ZEND_COLD void zend_argument_error_ex(const zend_function *function, | |
| uint32_t arg_num, zend_class_entry *error_ce, const char *format, ...) | |
| ZEND_API ZEND_COLD void zend_argument_error_ex(const zend_function *function, | |
| zend_class_entry *error_ce, uint32_t arg_num, const char *format, ...) |
There was a problem hiding this comment.
Could you explain why? Is it to match the order of the non-_ex function?
function and arg_num are related, so grouping them would make sense.
Looking at other format-and-throw functions, extra args tend to be grouped in between ce and format:
zend_object *zend_throw_exception(zend_class_entry *exception_ce, const char *message, zend_long code);
zend_object *zend_throw_exception_ex(zend_class_entry *exception_ce, zend_long code, const char *format, ...);void ZEND_FASTCALL zend_argument_error_variadic(zend_class_entry *error_ce, uint32_t arg_num, const char *format, va_list va);
void zend_argument_error(zend_class_entry *error_ce, uint32_t arg_num, const char *format, ...);What do you think of this?
| ZEND_API ZEND_COLD void zend_argument_error_ex(const zend_function *function, | |
| uint32_t arg_num, zend_class_entry *error_ce, const char *format, ...) | |
| ZEND_API ZEND_COLD void zend_argument_error_ex(zend_class_entry *error_ce, | |
| const zend_function *function, uint32_t arg_num, const char *format, ...) |
There was a problem hiding this comment.
Is it to match the order of the non-
_exfunction?
Yes, my motivation was making this consistent with the existing non-_ex zend_argument_error(). Changing the argument order of that one would would be too impactful, I feel. That's why I suggested adjusting the new functions for consistency with their direct sibling, rather than zend_throw_exception.
|
|
||
| if (type == E_ERROR) { | ||
| zend_argument_error_variadic(zend_ce_value_error, arg_num, format, va); | ||
| zend_argument_error_variadic(zend_active_function(), arg_num, zend_ce_value_error, format, va); |
There was a problem hiding this comment.
| zend_argument_error_variadic(zend_active_function(), arg_num, zend_ce_value_error, format, va); | |
| zend_argument_error_variadic(zend_active_function(), zend_ce_value_error, arg_num, format, va); |
| ZEND_API ZEND_COLD void ZEND_FASTCALL zend_argument_error_variadic( | ||
| const zend_function *function, uint32_t arg_num, | ||
| zend_class_entry *error_ce, const char *format, va_list va) |
There was a problem hiding this comment.
| ZEND_API ZEND_COLD void ZEND_FASTCALL zend_argument_error_variadic( | |
| const zend_function *function, uint32_t arg_num, | |
| zend_class_entry *error_ce, const char *format, va_list va) | |
| ZEND_API ZEND_COLD void ZEND_FASTCALL zend_argument_error_variadic( | |
| const zend_function *function, zend_class_entry *error_ce, | |
| uint32_t arg_num, const char *format, va_list va) |
| va_list va; | ||
|
|
||
| va_start(va, format); | ||
| zend_argument_error_variadic(function, arg_num, error_ce, format, va); |
There was a problem hiding this comment.
| zend_argument_error_variadic(function, arg_num, error_ce, format, va); | |
| zend_argument_error_variadic(function, error_ce, arg_num, format, va); |
|
|
||
| va_start(va, format); | ||
| zend_argument_error_variadic(error_ce, arg_num, format, va); | ||
| zend_argument_error_variadic(function, arg_num, error_ce, format, va); |
There was a problem hiding this comment.
| zend_argument_error_variadic(function, arg_num, error_ce, format, va); | |
| zend_argument_error_variadic(function, error_ce, arg_num, format, va); |
| va_list va; | ||
|
|
||
| va_start(va, format); | ||
| zend_argument_error_variadic(function, arg_num, zend_ce_type_error, format, va); |
There was a problem hiding this comment.
| zend_argument_error_variadic(function, arg_num, zend_ce_type_error, format, va); | |
| zend_argument_error_variadic(function, zend_ce_type_error, arg_num, format, va); |
|
|
||
| va_start(va, format); | ||
| zend_argument_error_variadic(zend_ce_type_error, arg_num, format, va); | ||
| zend_argument_error_variadic(function, arg_num, zend_ce_type_error, format, va); |
There was a problem hiding this comment.
| zend_argument_error_variadic(function, arg_num, zend_ce_type_error, format, va); | |
| zend_argument_error_variadic(function, zend_ce_type_error, arg_num, format, va); |
|
|
||
| va_start(va, format); | ||
| zend_argument_error_variadic(zend_ce_value_error, arg_num, format, va); | ||
| zend_argument_error_variadic(function, arg_num, zend_ce_value_error, format, va); |
There was a problem hiding this comment.
| zend_argument_error_variadic(function, arg_num, zend_ce_value_error, format, va); | |
| zend_argument_error_variadic(function, zend_ce_value_error, arg_num, format, va); |
| ZEND_API ZEND_COLD void ZEND_FASTCALL zend_argument_error_variadic( | ||
| const zend_function *function, uint32_t arg_num, | ||
| zend_class_entry *error_ce, const char *format, va_list va); | ||
| ZEND_API ZEND_COLD void zend_argument_error(zend_class_entry *error_ce, uint32_t arg_num, const char *format, ...); | ||
| ZEND_API ZEND_COLD void zend_argument_error_ex(const zend_function *function, | ||
| uint32_t arg_num, zend_class_entry *error_ce, const char *format, ...); | ||
| ZEND_API ZEND_COLD void zend_argument_type_error(uint32_t arg_num, const char *format, ...); | ||
| ZEND_API ZEND_COLD void zend_argument_type_error_ex( | ||
| const zend_function *function, uint32_t arg_num, | ||
| const char *format, ...); |
There was a problem hiding this comment.
| ZEND_API ZEND_COLD void ZEND_FASTCALL zend_argument_error_variadic( | |
| const zend_function *function, uint32_t arg_num, | |
| zend_class_entry *error_ce, const char *format, va_list va); | |
| ZEND_API ZEND_COLD void zend_argument_error(zend_class_entry *error_ce, uint32_t arg_num, const char *format, ...); | |
| ZEND_API ZEND_COLD void zend_argument_error_ex(const zend_function *function, | |
| uint32_t arg_num, zend_class_entry *error_ce, const char *format, ...); | |
| ZEND_API ZEND_COLD void zend_argument_type_error(uint32_t arg_num, const char *format, ...); | |
| ZEND_API ZEND_COLD void zend_argument_type_error_ex( | |
| const zend_function *function, uint32_t arg_num, | |
| const char *format, ...); | |
| ZEND_API ZEND_COLD void ZEND_FASTCALL zend_argument_error_variadic( | |
| const zend_function *function, zend_class_entry *error_ce, | |
| uint32_t arg_num, const char *format, va_list va); | |
| ZEND_API ZEND_COLD void zend_argument_error(zend_class_entry *error_ce, uint32_t arg_num, const char *format, ...); | |
| ZEND_API ZEND_COLD void zend_argument_error_ex(const zend_function *function, | |
| zend_class_entry *error_ce, uint32_t arg_num, const char *format, ...); | |
| ZEND_API ZEND_COLD void zend_argument_type_error(uint32_t arg_num, const char *format, ...); | |
| ZEND_API ZEND_COLD void zend_argument_type_error_ex( | |
| const zend_function *function, uint32_t arg_num, | |
| const char *format, ...); |
These variants take the function as parameter instead of inferring it from the call stack. This is useful in ZEND_TYPE_ASSERT, and will be used in PFA impl.
UPGRADING.INTERNALS: