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

Remove some #ifdef _CRTBLD blocks #3964

Merged
merged 7 commits into from
Sep 7, 2023

Conversation

achabense
Copy link
Contributor

@achabense achabense commented Aug 14, 2023

Works towards #3941.

  • Remove all #ifdef _CRTBLD blocks from <xthreads.h>.
  • Remove some #ifdef _CRTBLD blocks from <yvals.h>.

@achabense achabense requested a review from a team as a code owner August 14, 2023 02:04
stl/src/cthread.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Aug 14, 2023
@StephanTLavavej StephanTLavavej self-assigned this Aug 14, 2023
@achabense

This comment was marked as outdated.

@StephanTLavavej StephanTLavavej removed their assignment Aug 16, 2023
@achabense
Copy link
Contributor Author

achabense commented Aug 17, 2023

I've reworked and force-pushed this pr. It is ready for review now.

@achabense achabense changed the title Remove #ifdef _CRTBLD blocks from <xthreads.h> Remove some #ifdef _CRTBLD blocks Aug 19, 2023
Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

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

Thanks!

stl/src/_tolower.cpp Outdated Show resolved Hide resolved
stl/src/cthread.cpp Outdated Show resolved Hide resolved
@StephanTLavavej
Copy link
Member

Thanks, these are great cleanups! I'm not sure why we aren't using isleadbyte directly (there may be some semantic difference I'm missing), but this change is a strict, low-risk improvement.

@StephanTLavavej StephanTLavavej self-assigned this Sep 7, 2023
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej StephanTLavavej merged commit 9b468b3 into microsoft:main Sep 7, 2023
35 checks passed
@StephanTLavavej
Copy link
Member

🎉 🧹 😸

@achabense achabense deleted the _CRTBLD_cleanups2 branch September 8, 2023 02:51
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
None yet
Development

Successfully merging this pull request may close these issues.

5 participants