Bug #29992 [Com]: foreach by reference corrupts the array

From: Date: Thu, 25 Oct 2012 07:37:08 +0000
Subject: Bug #29992 [Com]: foreach by reference corrupts the array
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-174286@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=29992&edit=1

 ID:                 29992
 Comment by:         email at stevemann dot net
 Reported by:        fletch at pobox dot com
 Summary:            foreach by reference corrupts the array
 Status:             Not a bug
 Type:               Bug
 Package:            Scripting Engine problem
 Operating System:   linux
 PHP Version:        5.0.1
 Block user comment: N
 Private report:     N

 New Comment:

I don't think this is going to go anywhere - seems to have reached a stalemate. So I have just
retrained my head to automatically create foreach loops thus:

foreach($array as $item){

}unset($item);

If you need access to the last $item outside the loop, then just do it somewhere before the
unset($item).

Seems to me this thread is being accessed periodically by developers scratching their heads after
discovering similar oddities happening with their foreach loops. My advice would be to do something
similar to the above and just live with it.


Previous Comments:
------------------------------------------------------------------------
[2012-10-24 22:11:44] newms87 at gmail dot com

I understand this functionality, and I do agree that it is not a bug. It seems 
at the core of PHP that this is what would happen, but it does seem very 
unintuitive to me having used a variety of other languages. The result is not 
expected and has  caused several very hard to find bugs for me. 

Would it be possible to have PHP generate a E_NOTICE when using the same $var in 
both a foreach and afterwards when in a higher scope? EG: 

foreach($args as &$a){} 

$a = 'hello';  // this would generate an E_NOTICE

Then maybe have the option to turn off (or on by default) the E_NOTICE warnings 
in the ini settings?

------------------------------------------------------------------------
[2012-06-29 18:52:20] iam4webwork at hotmail dot com

I appreciate the explanation that Rasmus provides -- thank you!
One small but troublesome detail:

The first foreach changes the array by making $a[1] a reference
variable while $a[0] remains a normal variable.

$a = array(1,2);
foreach($a as &$e){}
var_dump($a,$e); // $a[1] == &int 2    $e == 2
foreach($a as $e){} $a[1] == &int 1    $e == 1
var_dump($a,$e); // $a[1] now points to last value of $a which is $a[0]

How about adding a switch so that users who don't want or understand
this behavior can turn it off?  Then it would be up in front of the 
documentation and would be less liable to be overlooked by users who 
fail to scroll down to the colored box.

Even if PHP were to have lexical scope (how hard would that be to 
implement and why can't PHP evolve that way?), that doesn't change 
the fact that the first loop doing seemingly nothing, does change the array.

------------------------------------------------------------------------
[2012-05-04 08:04:56] email at stevemann dot net

rasmus@php.net asked
"Ok, simple question then. Do you expect this to output 3?"

foreach(array(1,2,3) as $b) { }
echo $b;

I would much prefer it not to output 3. Personally I think it would make a lot more sense and be a
lot safer to have the array element references scoped to the foreach block - so effectively being
unset after the block has run. Having the last element of the array floating around outside of the
block is very dangerous in my view and can lead to silent errors. As someone else mentioned, I hate
to think how much incorrect data there is out there because of the last array element being
accidentally changed outside of the block.

derick@php.net rather flippantly said:
"no, we can't unset it by default, as people might use this for some weird reason."

I can think of plenty of non-weird reasons why people might want this behaviour. But if it was unset
by default, it's a simple matter to assign the reference to a variable defined outside of the
block thereby making it available outside the foreach. In other words, like this:

$c = NULL;
foreach(array(1,2,3) as $b) {
	$c = $b;
}
unset($b);// simulates block-scoping of $b
echo $c;

This is not a bug, but I believe it's dangerous behaviour of PHP as it would seem quite logical
to assume that the element references are scoped to the foreach block only - witness the many
comments in this thread to that effect. So my vote would be to change this behaviour to
block-scoping in a future version.

------------------------------------------------------------------------
[2012-03-19 18:51:24] paul dot dillinger at gmail dot com

Rasmus,

Thanks for looking at this.  I found the problem.  I would still call it a bug, 
but I will describe it and you can decide.  You are the man after all.  You were 
right, I was passing a variable by reference in the few lines of code in front 
of my example above: 


        foreach($this->data['external_'.$type] as &$item){
            if(!empty($item['package'])){
                $packages[] = $item;
                $library_names[] = $item['library_name'];
                unset($item);
            }
        }

/* Code in example above goes here */

BUT, where I see this as a bug was: $packages (the array that was getting 
changed) was a new array created from the data of each $item.  $packages was 
never being referenced, though the variable $item it was created from was.  So, 
it should be a copy of the data and not THE data right? 

I fixed it by simply not trying to pass by reference and changing unset($item) 
to unset($this->data['external_'.$type]).  Looking at it, that was the way to do 
it from the beginning.  I see that, but I'm not sure why $packages gets changed 
down the road (it was correct immediately after this, but it and all copies of 
it change inside the next foreach).  Any thoughts?

------------------------------------------------------------------------
[2012-03-01 18:52:14] rasmus@php.net

Paul, my guess is that $item is a reference to an element in the $packages array 
going into this loop. Try using a different variable there.

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=29992


-- 
Edit this bug report at https://bugs.php.net/bug.php?id=29992&edit=1


Thread (43 messages)

« previous php.bugs (#174286) next »