Unwrap reference after foreach

php.internals

Nikita Popov

5 years ago
Hi internals, I'd like to address a common footgun when using foreach by reference: https://wiki.php.net/rfc/foreach_unwrap_ref This addresses the issue described in the big red box at https://www.php.net/manual/en/control-structures.foreach.php. While this is "not a bug" (as our bug tracker can regularly attest), it's rather unexpected, and we could easily avoid it... Regards, Nikita

Hans Henrik Bergan

5 years ago
+1 from me, and yeah lets not care about that edge case, i hope the edge gets removed at some point.. (but that's an issue for another thread) FWIW if attempts at getting it in 8.2 fails, i would welcome another attempt at this for PHP9 On Fri, 13 Aug 2021 at 15:29, Nikita Popov <nikita.ppv@gmail.com> wrote:

Tobias Nyholm

5 years ago
I’m happy with this too. I’m +1 for most things that makes a smoother developer experience. //Tobias Nyholm

Trevor Rowbotham

5 years ago
-------- Original Message -------- On Aug 13, 2021, 9:28 AM, Nikita Popov < nikita.ppv@gmail.com> wrote: Hi internals, I'd like to address a common footgun when using foreach by reference: https://wiki.php.net/rfc/foreach_unwrap_ref This addresses the issue described in the big red box at https://www.php.net/manual/en/control-structures.foreach.php. While this is "not a bug" (as our bug tracker can regularly attest), it's rather unexpected, and we could easily avoid it... Regards, Nikita This seems like a good opportunity to go one step further and have loops create a new scope, which would automagically handle the unwrapping with out worrying about the edge cases. Obviously, this would be a bigger BC break, but one worth considering. Trevor

Michael Wallner

5 years ago
> > This seems like a good opportunity to go one step further and have loops create a new scope, which would automagically handle the unwrapping with out worrying about the edge cases. Obviously, this would be a bigger BC break, but one worth considering. >
Sounds reasonable, but this is too much a BC break for my taste. PHP doesn’t have a lot of scopes, and at a quick glance, this would increase them by 33%. I /know/ I wrote code using the last iterated index/key of a loop. That said, I also /know/ I wrote code using the value-by-reference after the loop, so I’m neither keen on voting +1 on the actual proposal. Granted though, it’d be an easy fix if there’s already a condition in the loop breaking its continuation. Regards, Mike

Claude Pache

5 years ago
> Le 13 août 2021 à 15:28, Nikita Popov <nikita.ppv@gmail.com> a écrit : > > Hi internals, > > I'd like to address a common footgun when using foreach by reference: > https://wiki.php.net/rfc/foreach_unwrap_ref > > This addresses the issue described in the big red box at > https://www.php.net/manual/en/control-structures.foreach.php. While this is > "not a bug" (as our bug tracker can regularly attest), it's rather > unexpected, and we could easily avoid it... > > Regards, > Nikita
Hi, I don’t like the split of semantics between simple and complex variables. It makes the language more inconsistent, therefore more difficult to reason about. On the other hand, using a complex variable as reference in that context should be very rare outside obfuscation code contests. Therefore, I suggest to deprecate (and later remove) the ability to use of a complex variable as reference in that context. That would solve the inconsistency issue. —Claude

Flávio Heleno

5 years ago
On Fri, Aug 13, 2021 at 11:20 AM Claude Pache <claude.pache@gmail.com> wrote:
> > > Le 13 août 2021 à 15:28, Nikita Popov <nikita.ppv@gmail.com> a écrit : > > > > Hi internals, > > > > I'd like to address a common footgun when using foreach by reference: > > https://wiki.php.net/rfc/foreach_unwrap_ref > > > > This addresses the issue described in the big red box at > > https://www.php.net/manual/en/control-structures.foreach.php. While > this is > > "not a bug" (as our bug tracker can regularly attest), it's rather > > unexpected, and we could easily avoid it... > > > > Regards, > > Nikita > > > Hi, > > I don’t like the split of semantics between simple and complex variables. > It makes the language more inconsistent, therefore more difficult to reason > about. > > On the other hand, using a complex variable as reference in that context > should be very rare outside obfuscation code contests. > > Therefore, I suggest to deprecate (and later remove) the ability to use of > a complex variable as reference in that context. That would solve the > inconsistency issue. > > —Claude > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > >
Hi, I know I don't have an actual vote (voting karma it is), but it's a +1 from me as well. I like Trevor's suggestion of scoping variables within loops and this would be a first step towards it.

Hossein Baghayi

5 years ago
On Fri, 13 Aug 2021 at 17:59, Nikita Popov <nikita.ppv@gmail.com> wrote:
> I'd like to address a common footgun when using foreach by reference: > https://wiki.php.net/rfc/foreach_unwrap_ref >
Hello, I had a question regarding this. Wouldn't it be possible to limit ```$value```'s scope to only foreach's block then discard it? Unless it was already defined elsewhere. ``` foreach($nice_stuff as &$value) {} //we are done here and no need to keep value around. echo $value; // $value is Undefined here. $value = null; foreach($stuff as &$value) {} //we can keep the value here since it doesn't belong to foreach. echo $value; // prints some fancy pancy stuff ```

Hans Henrik Bergan

5 years ago
Speaking of, i hope that one day we can support javascript-style let in php :) like foreach($it as let &$v){} but that's a discussion for another thread (and i'm sure it has been discussed before, i haven't actually checked though) On Sat, 14 Aug 2021 at 15:23, Hossein Baghayi <hossein.baghayi@gmail.com> wrote:

Hans Henrik Bergan

5 years ago
well today you can do foreach($it as &$value){...} unset($value); - which is pretty close, but it will break with $value="initial";foreach($it as &$value){...}unset($value); echo $value; here $value will not be "initial", it will be undefined, however you *CAN* do $value="initial";(function()use(&$it){foreach($it as &$value){...}unset($value);})(); echo $value; and it will still contain "initial" and here the unset is even unnecessary, but yeah.. usually people do not go to such lengths to get a contained variable scope ^^ On Sat, 14 Aug 2021 at 15:23, Hossein Baghayi <hossein.baghayi@gmail.com> wrote:

Claude Pache

5 years ago
> Le 13 août 2021 à 15:28, Nikita Popov <nikita.ppv@gmail.com> a écrit : > > Hi internals, > > I'd like to address a common footgun when using foreach by reference: > https://wiki.php.net/rfc/foreach_unwrap_ref > > This addresses the issue described in the big red box at > https://www.php.net/manual/en/control-structures.foreach.php. While this is > "not a bug" (as our bug tracker can regularly attest), it's rather > unexpected, and we could easily avoid it... > > Regards, > Nikita
A case that should be mentioned in the RFC is destructuring: ```php foreach ($foo as [ $x, &$y ]) { /* .... */ } ``` https://3v4l.org/A5Vi7 <https://3v4l.org/A5Vi7> I assume that the reference in `$y` would be unwrapped after the loop? —Claude

Nikita Popov

5 years ago
On Sat, Aug 14, 2021 at 6:00 PM Claude Pache <claude.pache@gmail.com> wrote:
> > Le 13 août 2021 à 15:28, Nikita Popov <nikita.ppv@gmail.com> a écrit : > > Hi internals, > > I'd like to address a common footgun when using foreach by reference: > https://wiki.php.net/rfc/foreach_unwrap_ref > > This addresses the issue described in the big red box at > https://www.php.net/manual/en/control-structures.foreach.php. While this > is > "not a bug" (as our bug tracker can regularly attest), it's rather > unexpected, and we could easily avoid it... > > Regards, > Nikita > > > A case that should be mentioned in the RFC is destructuring: > > ```php > foreach ($foo as [ $x, &$y ]) { /* .... */ } > ``` > https://3v4l.org/A5Vi7 > > I assume that the reference in `$y` would be unwrapped after the loop? >
I clarified to the RFC to say that a) unwrapping also occurs if the loop is left using break, continue, goto etc. b) unwrapping occurs for array destructuring targets under the same limitation of "simple variables only". So yes, $y would be unwrapped in your example. Regards, Nikita

Nikita Popov

4 years ago
On Fri, Aug 13, 2021 at 3:28 PM Nikita Popov <nikita.ppv@gmail.com> wrote:
> Hi internals, > > I'd like to address a common footgun when using foreach by reference: > https://wiki.php.net/rfc/foreach_unwrap_ref > > This addresses the issue described in the big red box at > https://www.php.net/manual/en/control-structures.foreach.php. While this > is "not a bug" (as our bug tracker can regularly attest), it's rather > unexpected, and we could easily avoid it... >
As the discussion has died down, I plan to open voting on this RFC soon. I have to admit that I'm less convinced of this than I was originally, because there's a surprising number of edge cases involved. The behavior is more intuitive on the surface, but things get more complicated when you look at detailed language semantics. Regards, Nikita

Nikita Popov

4 years ago
On Wed, Nov 10, 2021 at 10:06 AM Nikita Popov <nikita.ppv@gmail.com> wrote:
> On Fri, Aug 13, 2021 at 3:28 PM Nikita Popov <nikita.ppv@gmail.com> wrote: > >> Hi internals, >> >> I'd like to address a common footgun when using foreach by reference: >> https://wiki.php.net/rfc/foreach_unwrap_ref >> >> This addresses the issue described in the big red box at >> https://www.php.net/manual/en/control-structures.foreach.php. While this >> is "not a bug" (as our bug tracker can regularly attest), it's rather >> unexpected, and we could easily avoid it... >> > > As the discussion has died down, I plan to open voting on this RFC soon. > > I have to admit that I'm less convinced of this than I was originally, > because there's a surprising number of edge cases involved. The behavior is > more intuitive on the surface, but things get more complicated when you > look at detailed language semantics. >
I have ultimately decided to withdraw this proposal. Regards, Nikita

Côme Chilliet

4 years ago
Le 14 novembre 2021 18:13:18 GMT+01:00, Nikita Popov <nikita.ppv@gmail.com> a écrit :
>I have ultimately decided to withdraw this proposal. > >Regards, >Nikita
This is sad to hear, because this is a really big footgun, which has hit me in the past. I ended up adding a codestyle rule forcing byref foreach to be followed by an unset to avoid this. I understand making a special case for one syntax and not the other is unsatisfying but I was hoping for an other outcome. Côme

Rowan Collins

4 years ago
On 14/11/2021 19:49, Côme Chilliet wrote:
> Le 14 novembre 2021 18:13:18 GMT+01:00, Nikita Popov <nikita.ppv@gmail.com> a écrit : >> I have ultimately decided to withdraw this proposal. > This is sad to hear, because this is a really big footgun, which has hit me in the past. I ended up adding a codestyle rule forcing byref foreach to be followed by an unset to avoid this. > > I understand making a special case for one syntax and not the other is unsatisfying but I was hoping for an other outcome.
I think I agree with both of you - I agree that this is a problem worth solving, but also that the proposed solution isn't quite the right one. For one thing, it's just too "magic": although the current behaviour is *surprising*, it's actually a consequence of very straight-forward language rules; the proposed solution introduced a special case that isn't particular easy to explain, with extra edge cases to watch out for. One alternative solution would be to introduce block-scoped variables, which most of the time is what people want (and expect) in *any* foreach loop. Then the special-case for by-ref unsetting goes away, e.g.: foreach ( $array as let $x ) {     var_dump($x); } var_dump($x); //null / undefined foreach ( $array as let &$x ) { // or "&let $x"?     $x++; } var_dump($x); //null / undefined, not a reference Regards,
-- Rowan Tommins [IMSoP]