Unwrap reference after foreach

  115702
August 13, 2021 13:28 nikita.ppv@gmail.com (Nikita Popov)
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
  115703
August 13, 2021 13:43 divinity76@gmail.com (Hans Henrik Bergan)
+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 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 >
  115709
August 13, 2021 16:14 tobias.nyholm@gmail.com (Tobias Nyholm)
I’m happy with this too. I’m +1 for most things that makes a smoother developer experience. 

//Tobias Nyholm

> On 13 Aug 2021, at 06:44, Hans Henrik Bergan <divinity76@gmail.com> wrote: > > +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 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 >>
  115704
August 13, 2021 14:04 trevor.rowbotham@protonmail.com (Trevor Rowbotham)
-------- 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
  115758
August 16, 2021 13:41 mike@php.net (Michael Wallner)
> > 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
  115705
August 13, 2021 14:20 claude.pache@gmail.com (Claude Pache)
> Le 13 août 2021 à 15:28, Nikita Popov 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
  115706
August 13, 2021 14:49 flaviohbatista@gmail.com (=?UTF-8?Q?Fl=C3=A1vio_Heleno?=)
On Fri, Aug 13, 2021 at 11:20 AM Claude Pache pache@gmail.com>
wrote:

> > > Le 13 août 2021 à 15:28, Nikita Popov 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.
  115718
August 14, 2021 13:21 hossein.baghayi@gmail.com (Hossein Baghayi)
On Fri, 13 Aug 2021 at 17:59, Nikita Popov 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 ```
  115722
August 14, 2021 14:02 divinity76@gmail.com (Hans Henrik Bergan)
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 baghayi@gmail.com>
wrote:

> On Fri, 13 Aug 2021 at 17:59, Nikita Popov 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 > ``` >
  115725
August 14, 2021 15:17 divinity76@gmail.com (Hans Henrik Bergan)
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 baghayi@gmail.com>
wrote:

> On Fri, 13 Aug 2021 at 17:59, Nikita Popov 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 > ``` >
  115727
August 14, 2021 16:00 claude.pache@gmail.com (Claude Pache)
> Le 13 août 2021 à 15:28, Nikita Popov 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
  115856
August 26, 2021 14:26 nikita.ppv@gmail.com (Nikita Popov)
On Sat, Aug 14, 2021 at 6:00 PM Claude Pache pache@gmail.com> wrote:

> > Le 13 août 2021 à 15:28, Nikita Popov 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
  116312
November 10, 2021 09:06 nikita.ppv@gmail.com (Nikita Popov)
On Fri, Aug 13, 2021 at 3:28 PM Nikita Popov 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
  116347
November 14, 2021 17:13 nikita.ppv@gmail.com (Nikita Popov)
On Wed, Nov 10, 2021 at 10:06 AM Nikita Popov ppv@gmail.com> wrote:

> On Fri, Aug 13, 2021 at 3:28 PM Nikita Popov 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
  116351
November 14, 2021 19:49 come@chilliet.eu (=?ISO-8859-1?Q?C=F4me_Chilliet?=)
Le 14 novembre 2021 18:13:18 GMT+01:00, Nikita Popov 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
  116352
November 14, 2021 21:26 rowan.collins@gmail.com (Rowan Tommins)
On 14/11/2021 19:49, Côme Chilliet wrote:
> Le 14 novembre 2021 18:13:18 GMT+01:00, Nikita Popov 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]