[PHP-DEV] Re: 5.4 does not support Digest WWW-Authenticate anymore?
Hi:
this bug has been fixed, see
http://svn.php.net/viewvc?view=revisionrevision=317172
it was introduced by a mistake when removing PG(safe_mod):
if (PG(saf_mod)) {
#if PCRE
{
}
#else
{
//this should removed too, that is what does the fixing done
}
}
thanks
2011/9/23 Laruence larue...@php.net:
Hi:
if no ideas, I will be going to revert this.
thanks
2011/9/22 Laruence larue...@php.net:
+kalle
Hi :
I found this was introduce in r298625,and seems to be a mistake, kalle ,
can you verify this? Thanks
http://svn.php.net/viewvc/php/php-src/trunk/main/SAPI.c?r1=298625r2=298624pathrev=298625
在 2011年9月22日星期四，Laruence larue...@php.net 写道：
Hi:
I have filed a bug about this #55758,
thanks
2011/9/21 Laruence larue...@php.net:
Hi:
RT,
in main/SAPI.c sapi_header_op, all WWW-Authenticae will send a
Basic header.
was this removed intentionally? or just by mis-take?
thanks
--
Laruence Xinchen Hui
http://www.laruence.com/
--
Laruence Xinchen Hui
http://www.laruence.com/
--
惠新宸 laruence
Senior PHP Engineer
http://www.laruence.com
--
惠新宸 laruence
Senior PHP Engineer
http://www.laruence.com
--
Laruence Xinchen Hui
http://www.laruence.com/
--
Laruence Xinchen Hui
http://www.laruence.com/
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] __constructor parameter limitations.
On Tue, Sep 20, 2011 at 9:35 PM, Stas Malyshev smalys...@sugarcrm.com wrote:
Hi!
On 9/20/11 12:07 PM, Reindl Harald wrote:
but it is not logical
foo() in B can do anything with $a before or after parent::foo()
and the caller does not need to know at any point that B
has anything to do with the class A
Consider this code:
function doFoo(A $a) {
$a-foo();
}
It's fine, right? Since A::foo() signature fits.
Now consider this code:
$a = new B(); // B extends A, it's OK, right?
doFoo($a); // oops, $a-foo() breaks!
This code would break since B::foo() requires a parameter.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
A little bit off-topic, but maybe we could also discuss/fix this:
https://bugs.php.net/bug.php?id=43200
http://groups.google.com/group/symfony-devs/browse_thread/thread/3fc16ba601045551
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] __constructor parameter limitations.
Hi 2011/9/23 Ferenc Kovacs tyr...@gmail.com: A little bit off-topic, but maybe we could also discuss/fix this: https://bugs.php.net/bug.php?id=43200 http://groups.google.com/group/symfony-devs/browse_thread/thread/3fc16ba601045551 I don't see the bug in that matter, because the interface defines the prototype of methods that classes who inherit must implement. Abstract classes who implement the interface but not all the methods don't need to say to the child class that the method must be implemented twice, as the implementation requirement is inherited in the child class since its parent is abstract. Sure the warning could go away if both the interface and abstract class signatures match, but that would seem inconsistent to me. -- regards, Kalle Sommer Nielsen ka...@php.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
On 09/23/2011 01:37 AM, Alan Knowles wrote: This patch adds an extra parameter 'allow_string' to is_a (default off) and is_subclass_of (default on) , https://bugs.php.net/patch-display.php?bug_id=55475patch=Is_a_with_allow_string_argument_v3revision=latest https://bugs.php.net/patch-display.php?bug_id=55475patch=Is_a_with_allow_string_argument_v3revision=latest It also explains why their behaviour is different. This is a clean, no BC break, solution. let's move on and just fix this. Ok, executive decision made. Patch committed. We obviously screwed up in making this change in the 5_3 branch and since at least one distro has held off in pushing out newer 5.3 builds in order to not break every existing PEAR package, it is completely clear to me that this needed to be fixed in 5.3. So, a couple of ideas to address cases like this in the future. 1. Should we work up a basic PEAR test case that we can add to our tests? 2. Maybe we should think bigger and put more focus on having large PHP frameworks and apps test every RC. Currently we notify them of RCs and just hope someone will test and report back, but that obviously isn't working. We need a Daniel Brown-like approach to this. Someone who is really annoyingly persistent and will hunt down people to test RCs and keep a sign-off checklist of projects that have given a thumbs-up on an RC. Oh, and what do we do in 5.4? Philosophically I think Dmitry's original change was correct, but none of us realized all the code relying (arguably incorrectly) on the original behaviour. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
2011/9/23 Rasmus Lerdorf ras...@lerdorf.com: On 09/23/2011 01:37 AM, Alan Knowles wrote: This patch adds an extra parameter 'allow_string' to is_a (default off) and is_subclass_of (default on) , https://bugs.php.net/patch-display.php?bug_id=55475patch=Is_a_with_allow_string_argument_v3revision=latest https://bugs.php.net/patch-display.php?bug_id=55475patch=Is_a_with_allow_string_argument_v3revision=latest It also explains why their behaviour is different. This is a clean, no BC break, solution. let's move on and just fix this. Ok, executive decision made. Patch committed. We obviously screwed up in making this change in the 5_3 branch and since at least one distro has held off in pushing out newer 5.3 builds in order to not break every existing PEAR package, it is completely clear to me that this needed to be fixed in 5.3. So, a couple of ideas to address cases like this in the future. 1. Should we work up a basic PEAR test case that we can add to our tests? 2. Maybe we should think bigger and put more focus on having large PHP frameworks and apps test every RC. Currently we notify them of RCs and just hope someone will test and report back, but that obviously isn't working. We need a Daniel Brown-like approach to this. Someone who is really annoyingly persistent and will hunt down people to test RCs and keep a sign-off checklist of projects that have given a thumbs-up on an RC. Solution 2: +1 Having a Jenkins instance which would run major framework testsuites against the different versions of PHP? Oh, and what do we do in 5.4? Philosophically I think Dmitry's original change was correct, but none of us realized all the code relying (arguably incorrectly) on the original behaviour. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
hi Rasmus, On Fri, Sep 23, 2011 at 12:06 PM, Rasmus Lerdorf ras...@lerdorf.com wrote: 1. Should we work up a basic PEAR test case that we can add to our tests? 2. Maybe we should think bigger and put more focus on having large PHP frameworks and apps test every RC. Currently we notify them of RCs and just hope someone will test and report back, but that obviously isn't working. We need a Daniel Brown-like approach to this. Someone who is really annoyingly persistent and will hunt down people to test RCs and keep a sign-off checklist of projects that have given a thumbs-up on an RC. We do 2) already (while we are working on increasing the amount of apps and frameworks being tested), as I was asking to revert this patch between 5.3.7 and 5.3.8 back then pointing to our tests results and numerous reports. The problem was not in the QA but in the decision process. QA should have a kind of veto power in this case to avoid arguing and still have BC breaks landing in stable releases. Oh, and what do we do in 5.4? Philosophically I think Dmitry's original change was correct, but none of us realized all the code relying (arguably incorrectly) on the original behaviour. It is not an easy decision, I would prefer to revert it there too as it will break BC in 5.4 as well, obviously. Cheers, -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
On 09/23/2011 12:13 PM, Patrick ALLAERT wrote: 2011/9/23 Rasmus Lerdorf ras...@lerdorf.com 2. Maybe we should think bigger and put more focus on having large PHP frameworks and apps test every RC. Currently we notify them of RCs and just hope someone will test and report back, but that obviously isn't working. We need a Daniel Brown-like approach to this. Someone who is really annoyingly persistent and will hunt down people to test RCs and keep a sign-off checklist of projects that have given a thumbs-up on an RC. Solution 2: +1 Having a Jenkins instance which would run major framework testsuites against the different versions of PHP? That would be cool, but a lot of work to maintain since every framework/app has different ways of testing and we'll want to test different versions. It seems like the best bet is to get the people who know the code best to maintain the tests. If we could get all of them to set up *and maintain* their stuff on the Jenkins instance it would be ideal, but that's probably dreaming in technicolor. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
On 09/23/2011 12:17 PM, Pierre Joye wrote: We do 2) already (while we are working on increasing the amount of apps and frameworks being tested), as I was asking to revert this patch between 5.3.7 and 5.3.8 back then pointing to our tests results and numerous reports. The problem was not in the QA but in the decision process. QA should have a kind of veto power in this case to avoid arguing and still have BC breaks landing in stable releases. Well, 5.3.7 - 5.3.8 wasn't really the problem. That release was rushed to fix my crypt screwup. This should have been caught in the 5.3.6 - 5.3.7 testing which lasted much longer than the short window to 5.3.8. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
On Fri, Sep 23, 2011 at 12:23 PM, Rasmus Lerdorf ras...@lerdorf.com wrote: On 09/23/2011 12:17 PM, Pierre Joye wrote: We do 2) already (while we are working on increasing the amount of apps and frameworks being tested), as I was asking to revert this patch between 5.3.7 and 5.3.8 back then pointing to our tests results and numerous reports. The problem was not in the QA but in the decision process. QA should have a kind of veto power in this case to avoid arguing and still have BC breaks landing in stable releases. Well, 5.3.7 - 5.3.8 wasn't really the problem. That release was rushed to fix my crypt screwup. This should have been caught in the 5.3.6 - 5.3.7 testing which lasted much longer than the short window to 5.3.8. Right, also reported back then with the same success ;) Anyway, that's past, the BC break has been fixed and we are working to get these appsframeworks tests result as part of our release process (symfony and doctrine are on the road, must see with zf if we can share resources as they do it anyway internally). Cheers, -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH] Test-case for bug #55754
The following is a test-case for bug #55754 The patch is on top of php-5.3.8. Daniel K. diff --git a/tests/lang/bug55754.phpt b/tests/lang/bug55754.phpt new file mode 100644 index 000..179d475 --- /dev/null +++ b/tests/lang/bug55754.phpt @@ -0,0 +1,11 @@ +--TEST-- +Bug #55754 (Only variables should be passed by reference for ZEND_SEND_PREFER_REF params) +--FILE-- +?php + +current($arr = array(0 = a)); +current(array(0 = a)); +current($arr); + +? +--EXPECT-- -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
2011/9/23 Rasmus Lerdorf ras...@lerdorf.com: On 09/23/2011 12:13 PM, Patrick ALLAERT wrote: 2011/9/23 Rasmus Lerdorf ras...@lerdorf.com 2. Maybe we should think bigger and put more focus on having large PHP frameworks and apps test every RC. Currently we notify them of RCs and just hope someone will test and report back, but that obviously isn't working. We need a Daniel Brown-like approach to this. Someone who is really annoyingly persistent and will hunt down people to test RCs and keep a sign-off checklist of projects that have given a thumbs-up on an RC. Solution 2: +1 Having a Jenkins instance which would run major framework testsuites against the different versions of PHP? That would be cool, but a lot of work to maintain since every framework/app has different ways of testing and we'll want to test different versions. It seems like the best bet is to get the people who know the code best to maintain the tests. If we could get all of them to set up *and maintain* their stuff on the Jenkins instance it would be ideal, but that's probably dreaming in technicolor. Most serious PHP libraries/frameworks have a CI server. But they are mainly setup to discover when someone breaks something rather than testing various releases of PHP (and certainly not RC ones). Open up an Jenkins instance with some guidelines on how to add a new project which will be automatically launched with different releases of PHP and I am sure project leaders will be glad to setup their project(s) to know if their libraries/framework break with a specific release of PHP. Note that the libraries/frameworks wouldn't be supposed to be updated against their VCS. Ideally key releases of those libs/frameworks would be setup. Davide Mendolia (in CC) has started such a project in the past. Patrick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH] Fix for bug #55754
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 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 (14)
#define ZEND_PARSED_REFERENCE_VARIABLE (15)
#define ZEND_PARSED_NEW(16)
+#define ZEND_PARSED_EXPR_NO_PASS_BY_REF(17)
/* 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
Re: [PHP-DEV] [PATCH] Fix for bug #55754
Hi,
On Fri, Sep 23, 2011 at 14:06, Daniel K. d...@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 (14)
#define ZEND_PARSED_REFERENCE_VARIABLE (15)
#define ZEND_PARSED_NEW (16)
+#define ZEND_PARSED_EXPR_NO_PASS_BY_REF (17)
/* 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
Re: [PHP-DEV] [PATCH] Fix for bug #55754
On 09/23/2011 02:28 PM, Etienne Kneuss wrote:
On Fri, Sep 23, 2011 at 14:06, Daniel K. d...@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.
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... ?
foo(array(..)) is not a problem, as the op_type of 'array(..)' in this
context is IS_TMP_VAR, and thus not a candidate for passing by
reference, as far as zend_do_pass_param() is concerned.
For foo(funcNotReturningARef()) the lexer sets EA.type for the value of
funcNotReturningARef() to ZEND_PARSED_FUNCTION_CALL which in turn gets
checked in zend_do_pass_param(), just a few lines below where I added
the check to avoid the warning for assignment expressions (see below).
If ZEND_PARSED_FUNCTION_CALL is found, ZEND_ARG_SEND_SILENT is
eventually added to the opline, which in turn is checked for in
zend_vm_execute.c (ZEND_SEND_VAR_NO_REF_SPEC_VAR_HANDLER) where the
Strict warning is suppressed if ZEND_ARG_SEND_SILENT is set.
We could of course use the same mechanism, to just silence the warning,
but then the temporary variable holding the result of the assignment
expression would still be sent by reference, which I consider a bug.
I believe the right thing would be to pass it by value, to avoid nasty
surprises if any C code were to check if the argument was sent by
reference, and try to act accordingly.
To me it makes not much sense to distinguish different non-ref
expressions in that regard, they should all be handled the same.
It makes very much sense to make the distinction, as not all expressions
are made the same. Only IS_VAR and IS_CV ones are considered candidates
for passing by reference in zend_do_pass_param()
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.
This might very well be, PHP internals is not my first language, and I
might be completely off the track here. Do you by any chance have an
opinion as to where this 'totally different level' might be?
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 */
op = ZEND_SEND_VAR_NO_REF;
send_function =
ZEND_ARG_SEND_FUNCTION | ZEND_ARG_SEND_SILENT;
zend_is_function_or_method_call(param) returns true if
ZEND_PARSED_FUNCTION_CALL is set by the lexer, and then the warning is
silenced by virtue of adding ZEND_ARG_SEND_SILENT to send_function.
I think my method, to set ZEND_PARSED_EXPR_NO_PASS_BY_REF is in the same
vein as the similar existing method, and sufficient to avoid the buggy
behaviour.
Daniel K.
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] Fix for bug #55754
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 col...@php.net wrote:
Hi,
On Fri, Sep 23, 2011 at 14:06, Daniel K. d...@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 (14)
#define ZEND_PARSED_REFERENCE_VARIABLE (15)
#define ZEND_PARSED_NEW(16)
+#define ZEND_PARSED_EXPR_NO_PASS_BY_REF(17)
/* 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 {
Re: [PHP-DEV] Re: is_a() - again - a better fix
Hi Rasmus: On Fri, Sep 23, 2011 at 12:06:28PM +0200, Rasmus Lerdorf wrote: Ok, executive decision made. Patch committed. Thanks. 1. Should we work up a basic PEAR test case that we can add to our tests? Funny you mention that. I wrote an extensive email to the PEAR QA team a month ago (http://news.php.net/php.pear.qa/5890) proposing I clean up the PEAR CI code and that they contact PHP QA to get it running on gcov.php.net. Alas, the people on PEAR QA team seem preoccupied with life, so hadn't substantively responded. Though Daniel O'Connor, who put together PEAR's CI stuff, said offlist that he's +1 on my getting karma. Thanks, --Dan -- T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y data intensive web and database programming http://www.AnalysisAndSolutions.com/ 4015 7th Ave #4, Brooklyn NY 11232 v: 718-854-0335 f: 718-854-0409 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
Funny you mention that. I wrote an extensive email to the PEAR QA team a month ago (http://news.php.net/php.pear.qa/5890) proposing I clean up the PEAR CI code and that they contact PHP QA to get it running on gcov.php.net. Alas, the people on PEAR QA team seem preoccupied with life, so hadn't substantively responded. Though Daniel O'Connor, who put together PEAR's CI stuff, said offlist that he's +1 on my getting karma. I would be also glad to help setting that up, if help is needed. As I mentioned previously, I have experience setting up PHPUnderControl and Hudson/Jenkins for PHP and C apps, -- Ferenc Kovács @Tyr43l - http://tyrael.hu -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
On 2011-09-23, Rasmus Lerdorf ras...@lerdorf.com wrote: On 09/23/2011 12:13 PM, Patrick ALLAERT wrote: 2011/9/23 Rasmus Lerdorf ras...@lerdorf.com 2. Maybe we should think bigger and put more focus on having large PHP frameworks and apps test every RC. Currently we notify them of RCs and just hope someone will test and report back, but that obviously isn't working. We need a Daniel Brown-like approach to this. Someone who is really annoyingly persistent and will hunt down people to test RCs and keep a sign-off checklist of projects that have given a thumbs-up on an RC. Solution 2: +1 Having a Jenkins instance which would run major framework testsuites against the different versions of PHP? That would be cool, but a lot of work to maintain since every framework/app has different ways of testing and we'll want to test different versions. It seems like the best bet is to get the people who know the code best to maintain the tests. If we could get all of them to set up *and maintain* their stuff on the Jenkins instance it would be ideal, but that's probably dreaming in technicolor. I've made the decision that my team will test against RCs as soon as they are out (and we're going to be trying to do each beta as well). If we run into issues, we'll of course report back here. That said, I think it would be good to have a notification system whereby framework leads are all pinged on new betas and RCs, and a wiki page where they can indicate that they've run tests (and whether or not they had issues). That way, you could have a targetted nag list -- Hey, I don't see an update from you -- RUN THE TESTS!, and a deadline whereby if they haven't run them, they accept the consequences. :) I could also see this being an interesting peer-pressure move -- First to test!, We tested last week; how come _you_ haven't?, etc. -- Matthew Weier O'Phinney Project Lead| matt...@zend.com Zend Framework | http://framework.zend.com/ PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
On 23.09.11 18:15, Matthew Weier O'Phinney wrote: That said, I think it would be good to have a notification system whereby framework leads are all pinged on new betas and RCs, and a wiki Something could probably monitor http://svn.php.net/repository/php/php-src/tags/ for changes and alert people then. There would certainly be false positives along the way, but it should work fine most of the time. Given sufficient interest I can look into this. - Martin -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
On Fri, 2011-09-23 at 12:15 -0400, Matthew Weier O'Phinney wrote: On 2011-09-23, Rasmus Lerdorf ras...@lerdorf.com wrote: On 09/23/2011 12:13 PM, Patrick ALLAERT wrote: 2011/9/23 Rasmus Lerdorf ras...@lerdorf.com 2. Maybe we should think bigger and put more focus on having large PHP frameworks and apps test every RC. Currently we notify them of RCs and just hope someone will test and report back, but that obviously isn't working. We need a Daniel Brown-like approach to this. Someone who is really annoyingly persistent and will hunt down people to test RCs and keep a sign-off checklist of projects that have given a thumbs-up on an RC. Solution 2: +1 Having a Jenkins instance which would run major framework testsuites against the different versions of PHP? That would be cool, but a lot of work to maintain since every framework/app has different ways of testing and we'll want to test different versions. It seems like the best bet is to get the people who know the code best to maintain the tests. If we could get all of them to set up *and maintain* their stuff on the Jenkins instance it would be ideal, but that's probably dreaming in technicolor. I've made the decision that my team will test against RCs as soon as they are out (and we're going to be trying to do each beta as well). If we run into issues, we'll of course report back here. Good are also success reports so we know tests were run and succeeded. That said, I think it would be good to have a notification system whereby framework leads are all pinged on new betas and RCs, and a wiki page where they can indicate that they've run tests (and whether or not they had issues). That way, you could have a targetted nag list -- Hey, I don't see an update from you -- RUN THE TESTS!, and a deadline whereby if they haven't run them, they accept the consequences. :) We send out mails like this: http://news.php.net/php.qa/65903 to maintainers of different PHP projects who have opted in for every RC. I usually get one response and lots of black-hole void reactions. For 5.3.9 I'll make more responses a release requirement. (have to check the current recipient list and probably update that to define that closer, will also work w/ Stas/dsp on that to have it identical for 5.4) I could also see this being an interesting peer-pressure move -- First to test!, We tested last week; how come _you_ haven't?, etc. This also means that this list has more traffic. Which makes it more likely to be ignored ... johannes -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
On 09/23/2011 09:15 AM, Matthew Weier O'Phinney wrote: I could also see this being an interesting peer-pressure move -- First to test!, We tested last week; how come _you_ haven't?, etc. Speaking of testing, the commit http://svn.php.net/viewvc?view=revisionrevision=317183 doesn't have any... Who amongst the vocal supporters of the change is volunteering for this task? Chris -- Email: christopher.jo...@oracle.com Tel: +1 650 506 8630 Blog: http://blogs.oracle.com/opal/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
On 09/23/2011 07:13 PM, Christopher Jones wrote: On 09/23/2011 09:15 AM, Matthew Weier O'Phinney wrote: I could also see this being an interesting peer-pressure move -- First to test!, We tested last week; how come _you_ haven't?, etc. Speaking of testing, the commit http://svn.php.net/viewvc?view=revisionrevision=317183 doesn't have any... Who amongst the vocal supporters of the change is volunteering for this task? I committed a test about 2 hours before you posted this message: http://svn.php.net/viewvc?view=revisionrevision=317208 -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
On 09/23/2011 11:17 AM, Rasmus Lerdorf wrote: On 09/23/2011 07:13 PM, Christopher Jones wrote: On 09/23/2011 09:15 AM, Matthew Weier O'Phinney wrote: I could also see this being an interesting peer-pressure move -- First to test!, We tested last week; how come _you_ haven't?, etc. Speaking of testing, the commit http://svn.php.net/viewvc?view=revisionrevision=317183 doesn't have any... Who amongst the vocal supporters of the change is volunteering for this task? I committed a test about 2 hours before you posted this message: http://svn.php.net/viewvc?view=revisionrevision=317208 -Rasmus I don't see is_a or is_subclass_of in that test. Did I miss something? Chris -- Email: christopher.jo...@oracle.com Tel: +1 650 506 8630 Blog: http://blogs.oracle.com/opal/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
On 09/23/2011 08:20 PM, Christopher Jones wrote: On 09/23/2011 11:17 AM, Rasmus Lerdorf wrote: On 09/23/2011 07:13 PM, Christopher Jones wrote: On 09/23/2011 09:15 AM, Matthew Weier O'Phinney wrote: I could also see this being an interesting peer-pressure move -- First to test!, We tested last week; how come _you_ haven't?, etc. Speaking of testing, the commit http://svn.php.net/viewvc?view=revisionrevision=317183 doesn't have any... Who amongst the vocal supporters of the change is volunteering for this task? I committed a test about 2 hours before you posted this message: http://svn.php.net/viewvc?view=revisionrevision=317208 -Rasmus I don't see is_a or is_subclass_of in that test. Did I miss something? Ah, oops, wrong commit. I thought you meant the lack of a test in my initial commit to fix 55767 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
On 09/23/2011 08:20 PM, Christopher Jones wrote: On 09/23/2011 11:17 AM, Rasmus Lerdorf wrote: On 09/23/2011 07:13 PM, Christopher Jones wrote: On 09/23/2011 09:15 AM, Matthew Weier O'Phinney wrote: I could also see this being an interesting peer-pressure move -- First to test!, We tested last week; how come _you_ haven't?, etc. Speaking of testing, the commit http://svn.php.net/viewvc?view=revisionrevision=317183 doesn't have any... Who amongst the vocal supporters of the change is volunteering for this task? I committed a test about 2 hours before you posted this message: http://svn.php.net/viewvc?view=revisionrevision=317208 -Rasmus I don't see is_a or is_subclass_of in that test. Did I miss something? Yeah, got the wrong commit when I brought up that link. Had a cached copy of my fix for 55767 and the wifi is wonky here. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: is_a() - again - a better fix
On 09/23/2011 11:33 AM, Rasmus Lerdorf wrote: On 09/23/2011 08:20 PM, Christopher Jones wrote: On 09/23/2011 11:17 AM, Rasmus Lerdorf wrote: On 09/23/2011 07:13 PM, Christopher Jones wrote: On 09/23/2011 09:15 AM, Matthew Weier O'Phinney wrote: I could also see this being an interesting peer-pressure move -- First to test!, We tested last week; how come _you_ haven't?, etc. Speaking of testing, the commit http://svn.php.net/viewvc?view=revisionrevision=317183 doesn't have any... Who amongst the vocal supporters of the change is volunteering for this task? I committed a test about 2 hours before you posted this message: http://svn.php.net/viewvc?view=revisionrevision=317208 -Rasmus I don't see is_a or is_subclass_of in that test. Did I miss something? Yeah, got the wrong commit when I brought up that link. Had a cached copy of my fix for 55767 and the wifi is wonky here. No probs. Let's see if the people who speak about PHP testing compatibility actually do care enough about it to submit some tests. Chris -- Email: christopher.jo...@oracle.com Tel: +1 650 506 8630 Blog: http://blogs.oracle.com/opal/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] Fix for bug #55754
On 09/23/2011 03:41 PM, Daniel K. wrote:
On 09/23/2011 02:28 PM, Etienne Kneuss wrote:
On Fri, Sep 23, 2011 at 14:06, Daniel K. d...@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.
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... ?
[*SNIP Lengthy description of why the patch is right*]
To me it makes not much sense to distinguish different non-ref
expressions in that regard, they should all be handled the same.
It makes very much sense to make the distinction, as not all expressions
are made the same. Only IS_VAR and IS_CV ones are considered candidates
for passing by reference in zend_do_pass_param()
However, it can be done with less complexity, we only have to make sure
that the function arguments that the lexer sends to zend_do_pass_param
is not promoted to be sent by reference if the lexer already has decided
that you did not specify a variable. See: Zend/zend_language_parser.y -
non_empty_function_call_parameter_list:
In other words, only consider promoting to send by reference if the
argument passed to the function is not of type ZEND_SEND_VAL.
--- 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) original_op !=
ZEND_SEND_VAL) {
send_by_reference = 1;
if (op == ZEND_SEND_VAR
zend_is_function_or_method_call(param)) {
/* Method call */
Thanks for questioning my patch, and making me recheck what I was doing,
so that this minimal patch could come to life.
This still passes the test-suite with flying colours.
As a bonus this version of the fix applies to both 5.3-latest and
5.4-latest.
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.
I think I misunderstood you last time around.
I am of the opinion that the strict warning in question is completely
bogus. If the arginfo says: ZEND_SEND_PREFER_REF, Zend had better shut
up when a _value_ is passed.
Daniel K.
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index c325a7e..d361c64 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) original_op != ZEND_SEND_VAL) {
send_by_reference = 1;
if (op == ZEND_SEND_VAR zend_is_function_or_method_call(param)) {
/* Method call */
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
