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.

Sign up for GitHub

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

Jump to bottom

Implement P0952R2: 'A new specification for std::generate_canonical' #4740

Merged
merged 31 commits into from
Jun 27, 2024
Merged

Implement P0952R2: 'A new specification for std::generate_canonical' #4740

merged 31 commits into from
Jun 27, 2024

Conversation

MattStephanson
Copy link
Contributor

@MattStephanson MattStephanson commented Jun 20, 2024 •
edited
Loading

Fixes #1074. Fixes #4169.

@MattStephanson MattStephanson requested a review from a team as a code owner June 20, 2024 02:47
MattStephanson
MattStephanson commented Jun 20, 2024
View reviewed changes
stl/inc/random Outdated Show resolved Hide resolved
MattStephanson
MattStephanson commented Jun 20, 2024
View reviewed changes
stl/inc/random Outdated Show resolved Hide resolved
@MattStephanson

This comment was marked as resolved.

Sign in to view
@StephanTLavavej StephanTLavavej changed the title Implement P0952: "A new specification for std::generate_canonical" Implement P0952R2: 'A new specification for std::generate_canonical' Jun 20, 2024
@StephanTLavavej StephanTLavavej added the cxx26 C++26 feature label Jun 20, 2024
@StephanTLavavej StephanTLavavej self-assigned this Jun 20, 2024
@StephanTLavavej

This comment was marked as resolved.

Sign in to view
@StephanTLavavej StephanTLavavej removed their assignment Jun 20, 2024
@StephanTLavavej StephanTLavavej added the bug Something isn't working label Jun 20, 2024
frederick-vs-ja
frederick-vs-ja reviewed Jun 21, 2024
View reviewed changes
stl/inc/random Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej self-assigned this Jun 24, 2024
@StephanTLavavej
Reorder _Urbg_gen_can_params members to follow init order, optimizi…
b176e25 
…ng packing.
@StephanTLavavej
Style: _Urbg_gen_can_params _Params => auto _Params
0ae65e7
@StephanTLavavej
Naming clarity: _Result_int_type => _Result_uint_type
fa6cf54
@StephanTLavavej
Style: static_cast<_Sx_type>(1) => _Sx_type{1}
19f6012
@StephanTLavavej
Initialize _Real _Val{0}; to avoid any potential for warnings.
89d8b9f
@StephanTLavavej
We shouldn't need to suppress warning C4293 "shift count negative or …
a71eecc 
…too big".
@StephanTLavavej
Pre-existing: _Digits is either 24 (float) or 53 (double), drop `_M…
39a4d09 
…inbits <= 64`.

`_Bits` was totally bogus (being 2^32 - 1 or 2^64 - 1) so `_Minbits` was always `_Digits`. Let's update all usage to `_Digits` for clarity, since there's no "min" happening anymore.

`_Minbits <= 64` was always true, so drop it.
@StephanTLavavej
Update comment to say TRANSITION and mention subtract_with_carry_01.
bd3b156
@StephanTLavavej
Style: Flip integral and non-integral tr1 cases.
17736b2
@StephanTLavavej
_STL_INTERNAL_STATIC_ASSERT that k is at least 1.
c736511
@StephanTLavavej
Test: Use range-for.
c35d5ce
@StephanTLavavej
Test: Include <cmath>, but we don't need <utility>.
1b4869d
StephanTLavavej
StephanTLavavej reviewed Jun 26, 2024
View reviewed changes
stl/inc/random Outdated Show resolved Hide resolved
stl/inc/random Outdated Show resolved Hide resolved
tests/libcxx/expected_results.txt Outdated Show resolved Hide resolved
stl/inc/random Outdated Show resolved Hide resolved
stl/inc/random Outdated Show resolved Hide resolved
stl/inc/random Outdated Show resolved Hide resolved
stl/inc/random Outdated Show resolved Hide resolved
tests/std/tests/P0952R2_new_generate_canonical/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P0952R2_new_generate_canonical/test.cpp Show resolved Hide resolved
tests/std/tests/P0952R2_new_generate_canonical/test.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej mentioned this pull request Jun 26, 2024
Open
@StephanTLavavej

This comment was marked as resolved.

Sign in to view
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

statementreply
statementreply reviewed Jun 26, 2024
View reviewed changes
stl/inc/random Show resolved Hide resolved
stl/inc/random Show resolved Hide resolved
@StephanTLavavej StephanTLavavej mentioned this pull request Jun 26, 2024
Closed
@MattStephanson

This comment was marked as resolved.

Sign in to view
@StephanTLavavej

This comment was marked as resolved.

Sign in to view
@StephanTLavavej

This comment was marked as resolved.

Sign in to view
@StephanTLavavej StephanTLavavej merged commit f9b0af8 into microsoft:main Jun 27, 2024
39 checks passed
@StephanTLavavej
Copy link
Member

Thanks for implementing this major overhaul of central <random> machinery! ⚙️ 🛠️ 💚

@StephanTLavavej StephanTLavavej mentioned this pull request Jul 22, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@statementreply statementreply statementreply left review comments

@frederick-vs-ja frederick-vs-ja frederick-vs-ja left review comments

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

Assignees

@StephanTLavavej StephanTLavavej

Labels
bug Something isn't working cxx26 C++26 feature
Projects
STL Code Reviews
Archived in project
Milestone
No milestone
4 participants
@MattStephanson @StephanTLavavej @statementreply @frederick-vs-ja