Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:55607
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.214.42 as permitted sender)
MIME-Version: 1.0
Sender: ekneuss@gmail.com
In-Reply-To: <4E7C764C.3000808@uw.no>
References: <4E7C764C.3000808@uw.no>
Date: Fri, 23 Sep 2011 14:28:30 +0200
Message-ID: <CACQesk7q+PXPd3hVNY5nvar8JvWRTCzjST7kW_cRKMAtV7XX2g@mail.gmail.com>
To: "Daniel K." <dk@uw.no>
Cc: internals@lists.php.net
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] [PATCH] Fix for bug #55754
From: colder@php.net (Etienne Kneuss)

Hi,

On Fri, Sep 23, 2011 at 14:06, Daniel K. <dk@uw.no> wrote:
> When a built-in function is defined with ZEND_SEND_PREFER_REF, PHP will
> issue a strict warning if you use an assignment expression as the paramet=
er.
>
> As an example, current() show this behaviour.
>
> current() has this arginfo defined:
>
> ZEND_BEGIN_ARG_INFO(arginfo_current, ZEND_SEND_PREFER_REF)
> =C2=A0 =C2=A0ZEND_ARG_INFO(ZEND_SEND_PREFER_REF, arg)
> ZEND_END_ARG_INFO()
>
> and a call like:
>
> <?php
> current($foo =3D array("bar"));
> ?>
>
> Presents you with:
>
> PHP Strict Standards: =C2=A0Only variables should be passed by reference =
in
> %s on line %d
>
>
> I think it is wrong to warn about this, because, to me, the _PREFER_
> part of ZEND_SEND_PREFER_REF, conveys that we should prefer to pass by
> reference, if possible, and not care otherwise.
>
> The below patch implements the not care part that was missing until now.
>
>
> This is done by having the lexer mark the result variable of the
> assignment expression as not passable by reference, and changing the
> function zend_do_pass_param() to ignore the marked variables when
> considering if a parameter could be passed by reference or not.
>
> I have run make test, before and after, with no regressions. The only
> test affected was the one I sent earlier to specifically test for this bu=
g.
>
> I am, however, not sure if this is the right approach to solve the
> problem, in which case, I hope to at least have put one of you on the
> right track to the _real_ solution, and to have made you interested in
> fixing it properly.

The patch looks strange to me, why would you only consider stuff like
foo($a =3D 2) ? what about passing any other expressions:
foo(array(..)), foo(funcNotReturningARef()) etc... ?

To me it makes not much sense to distinguish different non-ref
expressions in that regard, they should all be handled the same.
Whether we want the actual error on some of our functions  like
current()/end() etc.. is another question, but that should be fixed at
a totally different level.

>
> The patch is for php-5.3.8
>
>
> Daniel K.
>
> diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
> index c325a7e..df30d3d 100644
> --- a/Zend/zend_compile.c
> +++ b/Zend/zend_compile.c
> @@ -2096,7 +2096,7 @@ void zend_do_pass_param(znode *param, zend_uchar
> op, int offset TSRMLS_DC) /* {{
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0if (function_ptr) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (ARG_MAY_BE_SEN=
T_BY_REF(function_ptr, (zend_uint) offset)) {
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 if (param->op_type & (IS_VAR|IS_CV)) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 if (param->op_type & (IS_VAR|IS_CV) && param->u.EA.type !=3D
> ZEND_PARSED_EXPR_NO_PASS_BY_REF) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0send_by_reference =3D 1;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (op =3D=3D ZEND_SEND_VAR && zend_i=
s_function_or_method_call(param)) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Method=
 call */
> diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h
> index 15f24df..475f976 100644
> --- a/Zend/zend_compile.h
> +++ b/Zend/zend_compile.h
> @@ -650,6 +650,7 @@ int zendlex(znode *zendlval TSRMLS_DC);
> =C2=A0#define ZEND_PARSED_VARIABLE =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 (1<<4)
> =C2=A0#define ZEND_PARSED_REFERENCE_VARIABLE (1<<5)
> =C2=A0#define ZEND_PARSED_NEW =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0(1<<6)
> +#define ZEND_PARSED_EXPR_NO_PASS_BY_REF =C2=A0 =C2=A0 =C2=A0 =C2=A0(1<<7=
)
>
>
> =C2=A0/* unset types */
> diff --git a/Zend/zend_language_parser.y b/Zend/zend_language_parser.y
> index 1753e97..666b9e2 100644
> --- a/Zend/zend_language_parser.y
> +++ b/Zend/zend_language_parser.y
> @@ -578,7 +578,7 @@ non_empty_for_expr:
>
> =C2=A0expr_without_variable:
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0T_LIST '(' { zend_=
do_list_init(TSRMLS_C); } assignment_list ')' '=3D'
> expr { zend_do_list_end(&$$, &$7 TSRMLS_CC); }
> - =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A0 =C2=A0 variable '=3D' expr =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 { zend_check_writable_variable(&$=
1);
> zend_do_assign(&$$, &$1, &$3 TSRMLS_CC); }
> + =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A0 =C2=A0 variable '=3D' expr =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 { zend_check_writable_variable(&$=
1);
> zend_do_assign(&$$, &$1, &$3 TSRMLS_CC); $$.u.EA.type =3D
> ZEND_PARSED_EXPR_NO_PASS_BY_REF; }
> =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0 variable '=3D' '&' vari=
able { zend_check_writable_variable(&$1);
> zend_do_end_variable_parse(&$4, BP_VAR_W, 1 TSRMLS_CC);
> zend_do_end_variable_parse(&$1, BP_VAR_W, 0 TSRMLS_CC);
> zend_do_assign_ref(&$$, &$1, &$4 TSRMLS_CC); }
> =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0 variable '=3D' '&' T_NE=
W class_name_reference {
> zend_error(E_DEPRECATED, "Assigning the return value of new by reference
> is deprecated"); =C2=A0zend_check_writable_variable(&$1);
> zend_do_extended_fcall_begin(TSRMLS_C); zend_do_begin_new_object(&$4,
> &$5 TSRMLS_CC); } ctor_arguments { zend_do_end_new_object(&$3, &$4, &$7
> TSRMLS_CC); zend_do_extended_fcall_end(TSRMLS_C);
> zend_do_end_variable_parse(&$1, BP_VAR_W, 0 TSRMLS_CC); $3.u.EA.type =3D
> ZEND_PARSED_NEW; zend_do_assign_ref(&$$, &$1, &$3 TSRMLS_CC); }
> =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0 T_NEW class_name_refere=
nce { zend_do_extended_fcall_begin(TSRMLS_C);
> zend_do_begin_new_object(&$1, &$2 TSRMLS_CC); } ctor_arguments {
> zend_do_end_new_object(&$$, &$1, &$4 TSRMLS_CC);
> zend_do_extended_fcall_end(TSRMLS_C);}
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>



--=20
Etienne Kneuss
http://www.colder.ch