Fix pre-PHP 8.2 compatibility for php_mt_rand_range() with MT_RAND_PHP#9839
Merged
TimWolla merged 3 commits intophp:PHP-8.2from Oct 28, 2022
Merged
Fix pre-PHP 8.2 compatibility for php_mt_rand_range() with MT_RAND_PHP#9839TimWolla merged 3 commits intophp:PHP-8.2from
TimWolla merged 3 commits intophp:PHP-8.2from
Conversation
As some left-over comments indicated: > Legacy mode deliberately not inside php_mt_rand_range() > to prevent other functions being affected The broken scaler was only used for `php_mt_rand_common()`, not `php_mt_rand_range()`. The former is only used for `mt_rand()`, whereas the latter is used for `array_rand()` and others. With the refactoring for the introduction of ext/random `php_mt_rand_common()` and `php_mt_rand_range()` were accidentally unified, thus introducing a behavioral change that was reported in FakerPHP/Faker#528. This commit moves the checks for `MT_RAND_PHP` from the general-purpose `range()` function back into `php_mt_rand_common()` and also into `Randomizer::getInt()` for drop-in compatibility with `mt_rand()`.
cmb69
approved these changes
Oct 27, 2022
Member
cmb69
left a comment
There was a problem hiding this comment.
Thank you! This looks good to me.
8 tasks
zeriyoshi
approved these changes
Oct 28, 2022
TimWolla
added a commit
to TimWolla/php-src
that referenced
this pull request
Oct 28, 2022
* PHP-8.2: Fix pre-PHP 8.2 compatibility for php_mt_rand_range() with MT_RAND_PHP (php#9839)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Example: https://3v4l.org/0gEEY
As some left-over comments indicated:
The broken scaler was only used for
php_mt_rand_common(), notphp_mt_rand_range(). The former is only used formt_rand(), whereas the latter is used forarray_rand()and others.With the refactoring for the introduction of ext/random
php_mt_rand_common()andphp_mt_rand_range()were accidentally unified, thus introducing a behavioral change that was reported in FakerPHP/Faker#528.This commit moves the checks for
MT_RAND_PHPfrom the general-purposerange()function back intophp_mt_rand_common()and also intoRandomizer::getInt()for drop-in compatibility withmt_rand().