Skip to content

Conversation

@TimWolla
Copy link
Member

This fixes an incompatibility when wrapping native 32-bit engines with a userland
engine. The latter always used the 64-bit range function which then used two
32-bit numbers from the underlying engine to fill the 64-bit range, whereas the
native implementation used only one.

Now the selection of the range variant only depends on the requested range. A
32-bit range uses the 32-bit variant (even for 64-bit engines), whereas a
larger range uses the 64-bit variant.

This was found in #9410 (comment)

This fixes an incompatibility when wrapping native 32-bit engines with a userland
engine. The latter always used the 64-bit range function which then used two
32-bit numbers from the underlying engine to fill the 64-bit range, whereas the
native implementation used only one.

Now the selection of the range variant only depends on the requested range. A
32-bit range uses the 32-bit variant (even for 64-bit engines), whereas a
larger range uses the 64-bit variant.

This was found in php#9410 (comment)
@TimWolla
Copy link
Member Author

Intentionally not adding / changing a test here to make #9410 not more of a mess than it already is. The correct behavior was verified manually and will be correctly asserted with #9410. For this one I'm happy if the existing tests continue to pass (which they should).

Copy link
Contributor

@zeriyoshi zeriyoshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thank you!

@TimWolla TimWolla merged commit cbb024c into php:master Aug 25, 2022
@TimWolla TimWolla deleted the rand-range-selection branch August 25, 2022 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants