Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:55609
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.54 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CACQesk7q+PXPd3hVNY5nvar8JvWRTCzjST7kW_cRKMAtV7XX2g@mail.gmail.com>
References: <4E7C764C.3000808@uw.no>
	<CACQesk7q+PXPd3hVNY5nvar8JvWRTCzjST7kW_cRKMAtV7XX2g@mail.gmail.com>
Date: Fri, 23 Sep 2011 22:04:38 +0800
Message-ID: <CAHMUw2HXDWdx9gDemPMFcuHM=mrmwujbdJQVuP3MO3-KYyWXcQ@mail.gmail.com>
To: Etienne Kneuss <colder@php.net>
Cc: internals@lists.php.net, "Daniel K." <dk@uw.no>
Content-Type: multipart/alternative; boundary=0016363b9f42346fc304ad9c4aa5
Subject: Re: [PHP-DEV] [PATCH] Fix for bug #55754
From: tjerk.meesters@gmail.com (Tjerk Meesters)

--0016363b9f42346fc304ad9c4aa5
Content-Type: text/plain; charset=ISO-8859-1

Hi,

Ran into this exact issue just the other day. In the pre-5.4 days, to get
the first element of a returned array I would use current(). I still think
that's correct and shouldn't raise a digital eyebrow.
On Sep 23, 2011 8:30 PM, "Etienne Kneuss" <colder@php.net> wrote:
> 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
parameter.
>>
>> As an example, current() show this behaviour.
>>
>> current() has this arginfo defined:
>>
>> ZEND_BEGIN_ARG_INFO(arginfo_current, ZEND_SEND_PREFER_REF)
>>    ZEND_ARG_INFO(ZEND_SEND_PREFER_REF, arg)
>> ZEND_END_ARG_INFO()
>>
>> and a call like:
>>
>> <?php
>> current($foo = array("bar"));
>> ?>
>>
>> Presents you with:
>>
>> PHP Strict Standards:  Only 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
bug.
>>
>> 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 = 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) /* {{
>>
>>        if (function_ptr) {
>>                if (ARG_MAY_BE_SENT_BY_REF(function_ptr, (zend_uint)
offset)) {
>> -                       if (param->op_type & (IS_VAR|IS_CV)) {
>> +                       if (param->op_type & (IS_VAR|IS_CV) &&
param->u.EA.type !=
>> ZEND_PARSED_EXPR_NO_PASS_BY_REF) {
>>                                send_by_reference = 1;
>>                                if (op == ZEND_SEND_VAR &&
zend_is_function_or_method_call(param)) {
>>                                        /* 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);
>>  #define ZEND_PARSED_VARIABLE                   (1<<4)
>>  #define ZEND_PARSED_REFERENCE_VARIABLE (1<<5)
>>  #define ZEND_PARSED_NEW                                        (1<<6)
>> +#define ZEND_PARSED_EXPR_NO_PASS_BY_REF        (1<<7)
>>
>>
>>  /* 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:
>>
>>  expr_without_variable:
>>                T_LIST '(' { zend_do_list_init(TSRMLS_C); }
assignment_list ')' '='
>> expr { zend_do_list_end(&$$, &$7 TSRMLS_CC); }
>> -       |       variable '=' expr               {
zend_check_writable_variable(&$1);
>> zend_do_assign(&$$, &$1, &$3 TSRMLS_CC); }
>> +       |       variable '=' expr               {
zend_check_writable_variable(&$1);
>> zend_do_assign(&$$, &$1, &$3 TSRMLS_CC); $$.u.EA.type =
>> ZEND_PARSED_EXPR_NO_PASS_BY_REF; }
>>        |       variable '=' '&' variable {
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); }
>>        |       variable '=' '&' T_NEW class_name_reference {
>> zend_error(E_DEPRECATED, "Assigning the return value of new by reference
>> is deprecated");  zend_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 =
>> ZEND_PARSED_NEW; zend_do_assign_ref(&$$, &$1, &$3 TSRMLS_CC); }
>>        |       T_NEW class_name_reference {
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
>>
>>
>
>
>
> --
> Etienne Kneuss
> http://www.colder.ch
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>

--0016363b9f42346fc304ad9c4aa5--