foreach, assigning to a reference, and E_NOTICE

php.internals

Sean Coates

20 years ago
Hi, A week or two ago, a colleague asked me about a strange behaviour he experienced, with seemingly-duplicate data within a foreach block. Upon further examination, we determined that this was caused by assigning to a referenced variable: <?php $i = array('zero','one','two'); foreach ($i AS &$j) {} foreach ($i AS $j) { echo $j . " "; } ?> (Output is: "zero one one") I asked around about this, and was given the technical explanation (see http://sean.caedmon.net/notsoweird.phps if you're curious), which cleared it up. However, upon further discussion, a number of us agreed that it would be pertinent to raise an E_NOTICE when foreach assigns to a pre-existing reference target. Certainly, the behaviour demonstrated above is non-obvious, and contrary to the normal "PHP way" (simplicity). Developers, at least in my opinion, don't expect foreach to WRITE to the iterated array. Technical explanation aside (yes, we realize that this is "normal" behaviour, as demonstrated in the unroll), I believe this represents a usability problem. Can we throw an E_NOTICE when foreach targets a reference? (The other option we kicked around was implicitly unset()ing the target variable before the first iteration, but this was deemed "too magical".) If so, when? HEAD only? I'd like to see it in 5.2 as well, if that ever exists, but I agree that it's too late for 5.1. S

Xuefer Tinys

20 years ago
try: <?php $i = array('zero','one','two'); foreach ($i AS &$j) {} unset($j) // <-------- this foreach ($i AS $j) { echo $j . " "; } ?> it's known behavior. may app rely on this.

Xuefer Tinys

20 years ago

Sara Golemon

20 years ago
>> Can we throw an E_NOTICE when foreach targets a reference? (The other > there's many way to use reference, not only in foreach. don't do > reference unless u know it's really needed. >
Sean's not so much referring to his own problem as (like you said) the solution is a fairly simple matter of following strict coding practices. He's trying to address the problem of unexperienced developers seeing this behavior and (wrongly) reporting it as a bug, or worse, assuming PHP is just "broken" and therefore shouldn't be used. That said, I'm not convinved this needs a notice, but I'm certainly not against it. Perhaps we should split the difference with an E_STRICT. -Sara

Xuefer Tinys

20 years ago

Sean Coates

20 years ago
> it's not the problem of the second foreach, any usage of $j after the > 1st foreach as &$j will hurt
Yes. I thought it was clear that I understand this. I guess not. My point is that foreach is doing something that isn't immediately obvious. The same is true of your for loop, but to a lesser extent, IMO (as I don't expect your for loop to ONLY read from $i). I don't want to start a discussion on references. I'm just trying to clear up a non-obvious case. S

Lukas Smith

20 years ago
Sean Coates wrote:
>> it's not the problem of the second foreach, any usage of $j after the >> 1st foreach as &$j will hurt > > Yes. I thought it was clear that I understand this. I guess not. > > My point is that foreach is doing something that isn't immediately > obvious. The same is true of your for loop, but to a lesser extent, IMO > (as I don't expect your for loop to ONLY read from $i). > > I don't want to start a discussion on references. I'm just trying to > clear up a non-obvious case.
How would you supress the notice (I know error suppression is ugly), but we need to make it possible for people to quickly adapt to this change if they indeed relied on this. would this work: foreach ($i as @&$j) {} regards, Lukas

Derick Rethans

20 years ago
On Mon, 6 Mar 2006, Sara Golemon wrote:
> > > Can we throw an E_NOTICE when foreach targets a reference? (The other > > there's many way to use reference, not only in foreach. don't do > > reference unless u know it's really needed. > > > Sean's not so much referring to his own problem as (like you said) the > solution is a fairly simple matter of following strict coding practices. He's > trying to address the problem of unexperienced developers seeing this behavior > and (wrongly) reporting it as a bug, or worse, assuming PHP is just "broken" > and therefore shouldn't be used. > > That said, I'm not convinved this needs a notice, but I'm certainly not > against it. Perhaps we should split the difference with an E_STRICT.
It *has* to be an E_STRICT because it's something related to language and possible semantic errors. E_NOTICE is not for that. However, I don't think that we should add this, as it's perfectly normal and expected behavior (albeit a bit weird). And indeed, some people might rely on this "feature". Derick

Sean Coates

20 years ago
> It *has* to be an E_STRICT because it's something related to language > and possible semantic errors. E_NOTICE is not for that.
Isn't it the same as the following? sean@iconoclast:~$ php5 -d"error_reporting=2047" -r '$a=$b;' Notice: Undefined variable: b in Command line code on line 1 According to the manual: E_NOTICE == Run-time notices. Indicate that the script encountered something that could indicate an error, but could also happen in the normal course of running a script. S

Paul Reinheimer

20 years ago
Being the colleague Sean refered to in his first post I thought I might weigh in. While I agree that once I looked at the base case that Sean worked out of my code the problem didn't take too long to recognize, that's not where I first experianced the problem. Problems first rear their head deep within your code, looking at it there I had no idea what was going on, all I could tell was that for some reason I was getting two copies of an object in a foreach loop (I hadn't yet noticed that I was missing one). Once Sean was able to break my code down into the simple base case and replicate the issue I understood what was happening, but just looking at my code as a whole? Not a chance. A warning on strict or or notice would have helped a lot, and that's really what I think a lot of those messages are, notices to the programmer that something odd /may/ happen. as a side note, running the zend code analyzer did not give me any warnings either. paul