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

<functional> should avoid including <memory> #2998

Merged
merged 3 commits into from
Aug 9, 2022

Conversation

StephanTLavavej
Copy link
Member

Reported by: https://www.reddit.com/r/cpp/comments/wgb8xo/fun_times_with_msvc_functional/

Related to: #2996

<functional> was including <memory> in C++17 mode for unique_ptr and align() used by Boyer-Moore and _Ebco_base used by _Not_fn.

We can deal with align() and _Ebco_base by moving them up from <memory> to <xmemory>. This means that more code will drag them in, but they're quite small so this should have minimal impact.

For unique_ptr, we can replace it with a cut-down _Mini_ptr. I've added a comment mentioning that this has stripped away some of unique_ptr's misuse-resistance (e.g. it doesn't attempt to defend against Derived* being converted to Base* as a constructor parameter and then being array-deleted).

I've uglified _Get() and _Release(), and marked _Release() as _NODISCARD (which we've intentionally refrained from doing for unique_ptr since 10% of usage discards without leaking; here we control all usage).

For TUs (or Standard Library Modules) that include both <functional> and <memory>, the addition of _Mini_ptr is an added cost, but it should be quite small because the class is so cut-down. The savings for TUs that include <functional> without <memory> are substantial. Measuring #include <functional> with cl /EHsc /nologo /W4 /MTd /std:c++latest /P /showIncludes meow.cpp, I found that the preprocessed file shrinks:

@StephanTLavavej StephanTLavavej added the throughput Must compile faster label Aug 5, 2022
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner August 5, 2022 23:02
@StephanTLavavej StephanTLavavej self-assigned this Aug 9, 2022
@StephanTLavavej
Copy link
Member Author

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

@StephanTLavavej
Copy link
Member Author

Resolved a merge conflict with #2806, same as in the mirror PR (keeping this PR's addition at the bottom).

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.

2 participants