Move numerous components from `<xstddef>` to other headers and reduce inclusion by frederick-vs-ja · Pull Request #3623 · microsoft/STL · GitHub
Skip to content

Move numerous components from <xstddef> to other headers and reduce inclusion#3623

Merged
StephanTLavavej merged 16 commits into
microsoft:mainfrom
frederick-vs-ja:include-opt
Apr 14, 2023
Merged

Move numerous components from <xstddef> to other headers and reduce inclusion#3623
StephanTLavavej merged 16 commits into
microsoft:mainfrom
frederick-vs-ja:include-opt

Conversation

@frederick-vs-ja

@frederick-vs-ja frederick-vs-ja commented Apr 4, 2023

Copy link
Copy Markdown
Contributor

This PR is trying to reduce inclusion dependency. Towards #3599.

The most signification changes are about <xstddef>. I believe most of the contents of <xstddef> should be moved to other headers (especially <type_traits> and <xutility>), which is probably "pure win" for throughput.

Unfortunately, __builtin_bit_cast is used in <compare> instead of a wrapper function (or the standard function std::bit_cast), because I didn't find the right place for the real function.

I previously tried to remove <cwchar> dependency for <limits> in #3482, but that PR was blocked by some compiler bugs.

Edit: this PR is now only focusing on <xstddef>. I've reported DevCom-10331466 due to the failed/skipped libc++test for <stdlib.h>.

Driven-by change: reordering the entry for LWG-3865 in expected_results.txt.

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner April 4, 2023 18:10
@StephanTLavavej StephanTLavavej added the throughput Must compile faster label Apr 4, 2023
@StephanTLavavej

This comment was marked as resolved.

@frederick-vs-ja

This comment was marked as resolved.

@frederick-vs-ja frederick-vs-ja changed the title <xstddef> etc.: Reduce inclusion dependency Move numerous components from <xstddef> to other headers and reduce inclusion Apr 5, 2023
Comment thread stl/inc/functional Outdated
Comment thread stl/inc/functional Outdated
@StephanTLavavej StephanTLavavej removed their assignment Apr 9, 2023
@StephanTLavavej

StephanTLavavej commented Apr 10, 2023

Copy link
Copy Markdown
Member

@strega-nil-ms strega-nil-ms self-assigned this Apr 11, 2023
@StephanTLavavej

Copy link
Copy Markdown
Member

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

@StephanTLavavej StephanTLavavej merged commit 90d87a5 into microsoft:main Apr 14, 2023
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for improving the STL's throughput and helping users to be more disciplined about inclusion! This ended up being more source-breaking than I originally expected, but hopefully we can power through the resulting breaks and ship this in VS 2022 17.7. 🚀 🚢 😹

@kiwidev

kiwidev commented Aug 10, 2023

Copy link
Copy Markdown
Member

Given that this breaks code when updating to VS17.7, could a note be included inside the release notes for VS17.7 discussing the breaking change?
Having to hunt down and find this when a team is broken (and it will take time to move code changes through repos etc) is not a good experience for a component that is usually treated as stable and is automatically updated.

@frederick-vs-ja

Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

throughput Must compile faster

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants