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

Cleanups: Follow Standard order for byteswap in <bit> #3543

Merged

Conversation

StephanTLavavej
Copy link
Member

Coalesce _Byteswap_meow and byteswap, follow Standard order - see WG21-N4928 [bit.syn].

Programmer archaeology: This was introduced by #2235, where we simply didn't mention Standard order while reviewing. It appears that the rationale for separating the helpers was that this grouped Standard functions together. These days, we prefer to follow Standard order unless there's a strong reason not to, and we're comfortable with interleaving helpers along the way (this makes it easier to see what the helpers are doing).

Note: There will be a trivial merge conflict between this PR's code movement and #3539 editing countl_zero's adjacent definition.

This was introduced by GH 2235. We didn't mention Standard order while
reviewing. It appears that the rationale for separating the helpers from
the Standard function was that this grouped Standard functions together.
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Mar 6, 2023
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner March 6, 2023 19:59
@github-actions github-actions bot added this to Initial Review in Code Reviews Mar 6, 2023
@StephanTLavavej StephanTLavavej moved this from Initial Review to Final Review in Code Reviews Mar 6, 2023
@strega-nil-ms strega-nil-ms moved this from Final Review to Ready To Merge in Code Reviews Mar 6, 2023
stl/inc/bit Show resolved Hide resolved
@StephanTLavavej StephanTLavavej self-assigned this Mar 7, 2023
@StephanTLavavej
Copy link
Member Author

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

@StephanTLavavej
Copy link
Member Author

I've resolved the trivial merge conflict.

@StephanTLavavej StephanTLavavej merged commit 1ab51c6 into microsoft:main Mar 7, 2023
Code Reviews automation moved this from Ready To Merge to Done Mar 7, 2023
@StephanTLavavej StephanTLavavej deleted the stl-cleanups-byteswap branch March 7, 2023 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants