Skip to content

PHP 8.2.6RC1 endless foreach #11171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
usarise opened this issue May 1, 2023 · 25 comments
Closed

PHP 8.2.6RC1 endless foreach #11171

usarise opened this issue May 1, 2023 · 25 comments

Comments

@usarise
Copy link
Contributor

usarise commented May 1, 2023

Description

The following code:

<?php

$all = ['test'];

foreach ($all as &$item) {
  $all += [$item];
}

var_dump($all);

Resulted in this output:

endless foreach

But I expected this output instead:

array(1) {
  [0]=>
  &string(4) "test"
}

PHP Version

PHP 8.2.6RC1

Operating System

Debian 11.7

@iluuu1994
Copy link
Member

The + on arrays works by only setting the keys that have not been set by the previous array. This includes integer keys. Thus, your array never grows.

https://3v4l.org/4RIXD

What you want is array_merge or [...$a, ...$b].

@iluuu1994 iluuu1994 closed this as not planned Won't fix, can't repro, duplicate, stale May 1, 2023
@usarise
Copy link
Contributor Author

usarise commented May 1, 2023

This is great, of course, but it doesn't work because of the new behavior.
for example phpstan or rector

@usarise
Copy link
Contributor Author

usarise commented May 1, 2023

@iluuu1994
Copy link
Member

@usarise It has worked this way since PHP 5.0 🙂 https://3v4l.org/7eHmFH

@usarise
Copy link
Contributor Author

usarise commented May 1, 2023

@usarise It has worked this way since PHP 5.0 🙂 https://3v4l.org/7eHmFH

starting with PHP 8.2.6RC1 endless foreach, before array(1) { [0]=> &string(4) "test" }

@iluuu1994
Copy link
Member

Oh. You mixed up "Expected" and "Actual" in your report.

@usarise
Copy link
Contributor Author

usarise commented May 1, 2023

Oh right sorry

@iluuu1994
Copy link
Member

I can reproduce it now. Thanks for the report!

@iluuu1994
Copy link
Member

@ramsey @saundefined @adoy

This is a pretty critical issue. It's caused by #10975. I will revert it for PHP-8.1+. Should I cherry-pick it for the given release branches or will you take care of that?

@adoy
Copy link
Member

adoy commented May 1, 2023

@iluuu1994 I took care of it for PHP 8.2.6. Thanks
/cc @saundefined

@iluuu1994
Copy link
Member

@adoy Thank you! 8.1 will also need those changes, I forgot to ping @patrickallaert.

@saundefined
Copy link
Member

@iluuu1994 @adoy Thank you both! 💙

@iluuu1994
Copy link
Member

@ramsey @patrickallaert Could this be merged for PHP 8.1.19? I'd like to avoid potential issues here.

@usarise
Copy link
Contributor Author

usarise commented May 10, 2023

@iluuu1994 PHP 8.1.19 non fixed!

https://3v4l.org/4ifDB

@iluuu1994
Copy link
Member

@patrickallaert I see you tagged 8.1.19 without cherry-picking this...

@patrickallaert
Copy link
Contributor

@patrickallaert I see you tagged 8.1.19 without cherry-picking this...

Yeah, sorry, not following github notices a lot, always better to reach me via e-mail in such cases. I have been reached by @remicollet.

@iluuu1994
Copy link
Member

@patrickallaert Sorry, I should've sent an e-mail. There's no dedicated list for RM things, right? Thanks for re-tagging with the fix!

@patrickallaert
Copy link
Contributor

@patrickallaert Sorry, I should've sent an e-mail. There's no dedicated list for RM things, right? Thanks for re-tagging with the fix!

Yes, there is: release-managers [at] php.net

@iluuu1994
Copy link
Member

Thanks! My bad, I'll send a mail to the list next time.

@nielsdos
Copy link
Member

nielsdos commented May 12, 2023

Sorry to bring this up again...
But I don't see the revert commit in https://github.com/php/php-src/commits/PHP-8.1.19
Was this reverted in the actual release?

EDIT: Ah, but the changelog entry is reverted on the release web page. So it might be okay in the release itself, just not reverted on the github branch.

@iluuu1994
Copy link
Member

@nielsdos This confused me too. It's not a part of the branch, but it is a part of the tag. See:
https://github.com/php/php-src/commits/php-8.1.19

@martin-cod
Copy link

martin-cod commented May 12, 2023

I spent hours trying to define this bug only on version 8.1.19 when I was using rector.
https://onlinephp.io/c/f6fd8

@iluuu1994
Copy link
Member

@martin-cod I assume they've built PHP right after the first tag was created instead of using the official zip.

@usarise
Copy link
Contributor Author

usarise commented May 12, 2023

@iluuu1994 the bug was also in the archives, for example I tested
https://www.php.net/distributions/php-8.1.19.tar.bz2

@nielsdos
Copy link
Member

Just to be sure, I just downloaded the archive and the bug is no longer in the archives right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants