On Wed, May 15, 2013 at 7:12 AM, Dmitry Stogov <dmi...@zend.com> wrote:
> According to fast-path execution, I would really like to not introduce the
> additional checks.
> Especially for concat_function I would change it in the following way:
>
> if (Z_TYPE_P(op1) != IS_STRING) {
> if (Z_TYPE_P(op1) == IS_OBJECT && Z_OBJ_HANDLER_P(op1, do_operation)) {
> return Z_OBJ_HANDLER_P(op1, do_operation)(ZEND_CONCAT, result, op1, op2
> TSRMLS_CC);
> }
> zend_make_printable_zval(op1, &op1_copy, &use_copy1);
> }
> if (Z_TYPE_P(op2) != IS_STRING) {
> if (Z_TYPE_P(op2) == IS_OBJECT && Z_OBJ_HANDLER_P(op2, do_operation)) {
> return Z_OBJ_HANDLER_P(op2, do_operation)(ZEND_CONCAT, result, op1, op2
> TSRMLS_CC);
> } zend_make_printable_zval(op2, &op2_copy, &use_copy2);
> }
>
>
> And in similar way for mod, shift, etc
>
> if (UNEXPECTED(Z_TYPE_P(op1) != IS_LONG)) {
>
> if (Z_TYPE_P(op1) == IS_OBJECT && Z_OBJ_HANDLER_P(op1, do_operation)) {
> return Z_OBJ_HANDLER_P(op1, do_operation)(ZEND_MOD, result, op1, op2
> TSRMLS_CC);
> }
> zendi_convert_to_long(op1, op1_copy, result);
> }
> op1_lval = Z_LVAL_P(op1);
> if (UNEXPECTED(Z_TYPE_P(op2) != IS_LONG)) {
>
> if (Z_TYPE_P(op2) == IS_OBJECT && Z_OBJ_HANDLER_P(op2, do_operation)) {
> return Z_OBJ_HANDLER_P(op2, do_operation)(ZEND_MOD, result, op1, op2
> TSRMLS_CC);
> } zendi_convert_to_long(op2, op2_copy, result);
> }
>
I now added checks to make sure that the overloading code is never in the
fast path. As an example see mod_function:
https://github.com/php/php-src/pull/342/files#L2R1034
I had to use if (Z_TYPE_P(op1) != IS_LONG || Z_TYPE_P(op2) != IS_LONG)
checks instead of individual checks as in your examples, otherwise the code
would behave differently (with no fallback to the normal code if FAILURE is
returned and also if the second operand is an object then the first one
would already be cast to long).
If there is no more feedback on the RFC, then I'll start voting in a day or
two.
Nikita
