Skip to content
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

<random>: Fixes subtract_with_carry_engine io #2088

Merged
merged 3 commits into from
Oct 9, 2021

Conversation

yuanhongzhao
Copy link
Contributor

  • Fixes <random>: subtract_with_carry_engine textual representation is incorrect #442. Also fixes <random>: Cannot save/restore a ranlux48 object in a stream #437, which was a slightly different issue. I maintained the textual representation of tr1::subtract_with_carry, breaking the state into 32-bit words, but the streaming operators should be consistent now.
  • Implements part of LWG-3519 by making subtract_with_carry_engine streaming operators hidden friends. tr1 classes continue to go through a free function taking an _Swc_base&.
  • Improves conformance of subtract_with_carry_engine streaming operators [tab:rand.req.eng], specifically the handling of the stream's flags and leaving the state unchanged if reading fails.
  • The now-unskipped libc++ tests cover the streaming operators and textual representation, so I only added coverage for the stream flags.

@yuanhongzhao yuanhongzhao requested a review from a team as a code owner July 31, 2021 01:23
@StephanTLavavej StephanTLavavej added the bug Something isn't working label Aug 3, 2021
@StephanTLavavej StephanTLavavej self-assigned this Aug 4, 2021
Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

Thanks - and apologies for the long wait! All of the issues I found were minor, so I'll validate and push changes.

Comment on lines -831 to +838
for (int _Jx = 1; _Jx <= _Kx; ++_Jx) { // unpack into _Kx words
unsigned int _Word = static_cast<unsigned int>(_Buf._At(_Ix) >> ((_Kx - _Jx) * 32));
for (int _Jx = 0; _Jx < _Kx; ++_Jx) { // unpack into _Kx words
const unsigned int _Word = static_cast<unsigned int>(_Buf._At(_Ix) >> (_Jx * 32));
Copy link
Member

Choose a reason for hiding this comment

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

Note for other reviewers (no change requested): as the runtime behavior here was completely messed up, I am okay with changing the serialized output.

@StephanTLavavej StephanTLavavej removed their assignment Sep 28, 2021
@CaseyCarter CaseyCarter self-assigned this Sep 29, 2021
@CaseyCarter
Copy link
Contributor

I'm mirroring this to an MSVC-internal PR. Please notify me if any further changes are pushed.

@CaseyCarter CaseyCarter merged commit 6aac0c9 into microsoft:main Oct 9, 2021
@CaseyCarter
Copy link
Contributor

Thank you for the bug fix!

@CaseyCarter CaseyCarter removed their assignment Oct 9, 2021
AZero13 pushed a commit to AZero13/STL that referenced this pull request Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants