Hi All,
I'm attempting to get involved and hack away at some internals; I figured
the best method to do this would be to pick up a bug and do my best at it.
So I grabbed #37676. The issue asks to implement an E_(NOTICE|WARNING)
when attempting to access scalar values as arrays. cmb alerted me to a
long thread that came up last year (
http://marc.info/?t=143379796900001&r=1&w=2). I've read over it and the
couple of other PR's that attempted to resolve this issue.
As I read, the couple of major complaints about adding an E_NOTICE were
that multiple-accesses in a set (i.e. $a=false;$a[1][2][3];) would produce
too many errors. Matt
http://marc.info/?l=php-internals&m=143384434418881&w=2 raised this
issue, in addition to it only being raised for non-null issues; having NULL
be an 'identity' of sort that any array access on a NULL is NULL. I agree
with him in part, and oppose in others. I think that original access to a
NULL should be treated as accessing any scalar value, however I do agree
that multiple notices would be bad. Rowan
http://marc.info/?l=php-internals&m=143385309121593&w=2 has also
presented his thoughts to the same, and this implementation takes into
account his solution where we don't raise much.
Christoph http://marc.info/?l=php-internals&m=143385339521672&w=2 countered
that: $o->foo->bar would itself raise two notices rather than one. I
would present that that behavior should be pruned itself. Object access on
a null should alert, but subsequent calls ought not to (I don't know how to
prevent that, but then again I haven't learned).
So I want to solicit for more discussion given previous talks. My
first-attempt RFC for this is at:
https://wiki.php.net/rfc/notice-for-non-valid-array-container
--
Dave
ps: this is my first attempt here at an RFC solo, and preemptively
apologize if anything deviates from the norm too far, did my best to follow
all them how-to's.
2016-07-31 21:31 GMT+02:00 David Walker dave@mudsite.com:
Hi All,
I'm attempting to get involved and hack away at some internals; I figured
the best method to do this would be to pick up a bug and do my best at it.
So I grabbed #37676. The issue asks to implement an E_(NOTICE|WARNING)
when attempting to access scalar values as arrays. cmb alerted me to a
long thread that came up last year (
http://marc.info/?t=143379796900001&r=1&w=2). I've read over it and the
couple of other PR's that attempted to resolve this issue.As I read, the couple of major complaints about adding an
E_NOTICEwere
that multiple-accesses in a set (i.e. $a=false;$a[1][2][3];) would produce
too many errors. Matt
http://marc.info/?l=php-internals&m=143384434418881&w=2 raised this
issue, in addition to it only being raised for non-null issues; havingNULL
be an 'identity' of sort that any array access on aNULLis NULL. I agree
with him in part, and oppose in others. I think that original access to a
NULLshould be treated as accessing any scalar value, however I do agree
that multiple notices would be bad. Rowan
http://marc.info/?l=php-internals&m=143385309121593&w=2 has also
presented his thoughts to the same, and this implementation takes into
account his solution where we don't raise much.Christoph http://marc.info/?l=php-internals&m=143385339521672&w=2
countered
that:$o->foo->barwould itself raise two notices rather than one. I
would present that that behavior should be pruned itself. Object access on
a null should alert, but subsequent calls ought not to (I don't know how to
prevent that, but then again I haven't learned).So I want to solicit for more discussion given previous talks. My
first-attempt RFC for this is at:
https://wiki.php.net/rfc/notice-for-non-valid-array-container--
Daveps: this is my first attempt here at an RFC solo, and preemptively
apologize if anything deviates from the norm too far, did my best to follow
all them how-to's.
Big +1. We came across that some days ago again.
Regards, Niklas
Hi David,
So I want to solicit for more discussion given previous talks. My
first-attempt RFC for this is at:
https://wiki.php.net/rfc/notice-for-non-valid-array-container
I've very likely to support this RFC, however the section on 'Backward
Incompatible Changes' needs to be filled out, as this RFC would make
code that currently emits no notices, start emitting notices.
cheers
Dan
Hi Dan,
I left it blank to go along with default production ini-settings where
notices would be excluded. However, you are probably correct that it should
be noted for development environments as something new. I'll toss it in
there in the morning
Thanks
Dave
Hi David,
So I want to solicit for more discussion given previous talks. My
first-attempt RFC for this is at:
https://wiki.php.net/rfc/notice-for-non-valid-array-containerI've very likely to support this RFC, however the section on 'Backward
Incompatible Changes' needs to be filled out, as this RFC would make
code that currently emits no notices, start emitting notices.cheers
Dan
Hi David,
The updated section still doesn't really describe the BC break that well.
Some people will need to change their code to avoid having notices
raised frequently. How difficult that will be needs to be thought
about.
Additionally, although the default ini files may turn off notices in
production environment, I really don't believe that is best practice.
For my machines, in both production and dev all warnings and notices
(except deprecation notices in production) are converted into
exceptions. Anything else allows mistakes in code to go undetected.
cheers
Dan
Dan,
You're right on both accounts; also, as I just notices it could easily have
other BC-breaks (although expecting errors in a certain format is odd)
through calls like error_get_last(), which would regardless of INI settings
include this NOTICE. I will update the RFC with these changes.
--
Dave
Hi David,
The updated section still doesn't really describe the BC break that well.
Some people will need to change their code to avoid having notices
raised frequently. How difficult that will be needs to be thought
about.Additionally, although the default ini files may turn off notices in
production environment, I really don't believe that is best practice.
For my machines, in both production and dev all warnings and notices
(except deprecation notices in production) are converted into
exceptions. Anything else allows mistakes in code to go undetected.cheers
Dan
All,
I updated the RFC, and made a couple updates to tests where this notice is
raising itself (fpm & mysqli). I ended up catching a couple of oddities in
how mine (and previous proposed) changes would have impacted a
foreach(list() = each()), so it won't raise a warning on the last, null,
element of each.
Before I ask for voting, I do have a best-practices question I hope someone
would have an answer to. My dev setup doesn't include all the extensions,
nor do I have a copy of windows for testing. What would be the best way to
ensure all tests are run so I can find other locations where this warning
might be adversely impacting some of the tests? I know Travis will run
tests when I create a PR, but I don't know if it is ensuring all tests are
fully run either.
Thanks
Dave
Dan,
You're right on both accounts; also, as I just notices it could easily
have other BC-breaks (although expecting errors in a certain format is odd)
through calls likeerror_get_last(), which would regardless of INI settings
include this NOTICE. I will update the RFC with these changes.--
DaveHi David,
The updated section still doesn't really describe the BC break that well.
Some people will need to change their code to avoid having notices
raised frequently. How difficult that will be needs to be thought
about.Additionally, although the default ini files may turn off notices in
production environment, I really don't believe that is best practice.
For my machines, in both production and dev all warnings and notices
(except deprecation notices in production) are converted into
exceptions. Anything else allows mistakes in code to go undetected.cheers
Dan