Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:67646
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.214.176 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CA+9eiLvTEKngseTeQWE_=fHO0vkCZ8tw23UyxoYytV8DrqaAiQ@mail.gmail.com>
References: <CAF+90c_PPTajBqs636f373DKF=hotrK1ObTojddxx=bxC3g16w@mail.gmail.com>
	<CA+9eiLvnV-i7Zp15oQ1EAzu2u8v_sc_Og8_bqHPBYWH9TyJ9cQ@mail.gmail.com>
	<CAF+90c_KfGWc7U5qA2Ey7FDqscvsRijAF16mqr+aG+561=WT9w@mail.gmail.com>
	<CA+9eiLuiDLYJtwxuUE8_1Het-S-NqaqDC_6gLm8scU253DTidw@mail.gmail.com>
	<CAF+90c9YV4P4MAHe8u2bq3FjVS0LxUtV9mfpni+4j0brvX7yNw@mail.gmail.com>
	<CA+9eiLtF3H=iu87udX8DNm87fWy3oofU1NyZOk7ALeKivRJiTg@mail.gmail.com>
	<CAF+90c9eTPM2ctqj9vGkBxsPSB+ommQnx2bT7e9aZ5VY_N+43A@mail.gmail.com>
	<CA+9eiLuqCLhM_5fDkL_7Cdk64oVbwPBMNhFAtixmca3mWWX8VA@mail.gmail.com>
	<CAF+90c_fS_+euRE6+7HpOci8aUs5yqZWmXEhiK0gu6oyS7PTfg@mail.gmail.com>
	<CA+9eiLvTEKngseTeQWE_=fHO0vkCZ8tw23UyxoYytV8DrqaAiQ@mail.gmail.com>
Date: Fri, 7 Jun 2013 19:14:35 +0200
Message-ID: <CAF+90c_OdYiTm7eViwnmzFpmg4GNBZMwU6Pc3nzcwJ+6HThU8g@mail.gmail.com>
To: Dmitry Stogov <dmitry@zend.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=047d7b67637aa4e79b04de939096
Subject: Re: [PHP-DEV] [RFC] Internal operator overloading and GMP improvements
From: nikita.ppv@gmail.com (Nikita Popov)

--047d7b67637aa4e79b04de939096
Content-Type: text/plain; charset=ISO-8859-1

On Wed, May 15, 2013 at 7:12 AM, Dmitry Stogov <dmitry@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

--047d7b67637aa4e79b04de939096--