Correct typo in bitwise AND operands

[jaredhoberock]

Hi, thanks for providing this implementation of threefry4x64.

I think I may have noticed typos in the implementation of extract4x64_impl::nth. The original code has expressions like these:

return (_output[n>>3] >> ((n&111)<<3)) & 0xFF; }

I believe the n&111 is a typo. The intention seems to be a bitwise AND with three set bits. But this is integer 111, not 0b111. In practice, this can produce a right shift that is too large for the type of n and generate undefined behavior. I noticed this when comparing the behavior of programs produced by different compilers.

Correcting this to n&7 seems to produce the correct results.

Are these expressions typos?

[sitmo]

Hi Jared, Yes, that must be a typo, good catch. I unfortunately don't have access to this repo anymore to merge your pull request.

…

On Sat, Feb 16, 2019, 00:54 Jared Hoberock ***@***.*** wrote: Hi, thanks for providing this implementation of threefry4x64. I think I may have noticed typos in the implementation of extract4x64_impl::nth. The original code has expressions like these: return (_output[n>>3] >> ((n&111)<<3)) & 0xFF; } I believe the n&111 is a typo. The intention seems to be a bitwise AND with three set bits. But this is integer 111, not 0b111. In practice, this can produce a right shift that is too large for the type in question and generate undefined behavior. I noticed this when comparing results between different compilers. Correcting this to n&7 seems to produce the correct results. Are these expressions typos? ------------------------------ You can view, comment on, or merge this pull request online at: #1 Commit Summary - Correct typo in bitwise AND operands File Changes - *M* random/include/boost/random/threefry4x64.hpp <https://github.com/sitmo/threefry/pull/1/files#diff-0> (4) Patch Links: - https://github.com/sitmo/threefry/pull/1.patch - https://github.com/sitmo/threefry/pull/1.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABC0wxlBo8OO6nk0rHZoRtqyjCX1_Bv1ks5vN0irgaJpZM4a-ovj> .

[jaredhoberock]

Ah, too bad.

Anyway, thanks again for putting in the effort for this implementation. I'll leave the PR open in case anyone else finds the diff useful.

[sitmo]

Hi Jared, Yes, it’s unfortunate,.. but indeed please leave the pull request up so that other people can benefit from your bug fix!

…

On 16 Feb 2019, at 22:23, Jared Hoberock ***@***.***> wrote: Ah, too bad. Anyway, thanks again for putting in the effort for this implementation. I'll leave the PR open in case anyone else finds the diff useful. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABC0w-Y5WwAw1ME5f-ybhwPoF0PC7Imsks5vOHbbgaJpZM4a-ovj>.