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

 ID:                 29992
 Comment by:         paul dot dillinger at gmail dot com
 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 understand that my explanation above isn't 100% accurate (unset really 
doesn't 
have anything to do with it), but that doesn't change that the expected 
behavior 
is not working.

Rasmus said:
        "Ok, simple question then. Do you expect this to output 3?

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

        If you do, then you don't want us to fix this "bug" 
        because fixing it would mean $b is not 3 here."

I say in the example above, would you expect a print_r of the same array to 
return 1,2,2?  My issue was that the content of the entire array.

Honestly I've been programming since I was eight years old.  That was 1985.  If 
this was confusing the hell out of me then something's wrong.

Here's an even simpler example:

<pre>
<?php
$clean = array(1,2,3,4);
foreach($clean as &$item){
  // Nothing
}
echo "A:\n";
/*A*/       print_r($clean);
echo "B:\n";
            foreach($clean as $item){
/*B*/           print_r($clean);
            }

$clean = array(1,2,3,4);
foreach($clean as &$item){
  // Nothing
}
echo "C:\n";
/*C*/       print_r($clean);
echo "D:\n";
            foreach($clean as $not_item){
/*C*/           print_r($clean);
            }
?>
</pre>

A and B output the same array differently with no modification (not expected).
C and D are the same (expected).  The only change was not re-using the name 
$item.

How can we make it so that using &$item in one foreach and then using the same 
variable name ($item) in a different foreach does not change the original array?


Previous Comments:
------------------------------------------------------------------------
[2012-10-25 15:49:27] paul dot dillinger at gmail dot com

I still say this is a bug, so here's all the code you need to re-produce it:

<pre>
<?php
$packages = array();
$library_names = array();
$ext_js =
    array(
        array(
            'name' => 'myname1',
            'attrib' => 'something1',
            'package' => true,
            'library_name' => 'package1'
            ),
        array(
            'name' => 'myname2',
            'attrib' => 'something2',
            'package' => true,
            'library_name' => 'package1'
            ),
        array(
            'name' => 'myname3',
            'attrib' => 'something3',
            'package' => false
            ),
        array(
            'name' => 'myname4',
            'attrib' => 'something4',
            'package' => false
            )
        );

foreach($ext_js as &$item){
        if(!empty($item['package'])){
                $packages[] = $item; // Not using &, so should be a copy, not a 
reference corrent?
                $library_names[] = $item['library_name'];
                unset($item);
        }
}
if(!empty($packages)){
/*A*/       print_r($ext_js);
            foreach($packages as $item){
/*B*/           print_r($ext_js);
            }
}
?>
</pre>

Look at the output on the last item.  Instead of unset removing the item from 
the array $ext_js (which is what I thought it would do), it corrupts the array. 
 
The array is fine though unless you go in to another foreach using another 
$item.

Changing the variable name on the second foreach to something OTHER than $item 
(I used $itemm) fixes it.

Bug.

------------------------------------------------------------------------
[2012-10-25 07:37:08] email at stevemann dot net

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.

------------------------------------------------------------------------
[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

ras...@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.

der...@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.

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


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

Reply via email to