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

Don't include <cwchar> in <limits> #3631

Conversation

frederick-vs-ja
Copy link
Contributor

Separated from #3482. Towards #3599.

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

This comment was marked as outdated.

@StephanTLavavej StephanTLavavej self-assigned this Apr 7, 2023
@StephanTLavavej

This comment was marked as outdated.

@StephanTLavavej StephanTLavavej added the decision needed We need to choose something before working on this label Apr 7, 2023
@StephanTLavavej StephanTLavavej removed their assignment Apr 7, 2023
@StephanTLavavej
Copy link
Member

This one looks like it's too breaking, sorry. Just within the compiler backend alone, I'm seeing 6 separate files that want the identifiers printf, wprintf, FILE, _SH_DENYNO, sprintf_s, and putchar, all because containers like <vector> include <xmemory> which includes <limits>. Note that <cwchar> includes <cstdio> because of internal VSO-661721 "_FILE_DEFINED dance breaks modules", where the UCRT is doing something weird and the STL has to work around it:

Until then, the STL must include <cstdio> before every inclusion of <cwchar> to make sure this is consistent.

Thus there appears to be an "ambient expectation" that the <cstdio> and <cwchar> machinery is available in C++14 mode - my psychic powers tell me that this will be endlessly breaking and we don't have the capacity to deal with that.

Labeling as decision needed so we can talk about this next Wednesday at the weekly maintainers meeting, but I expect that we will need to close this PR without merging. Thanks for trying though!

@StephanTLavavej StephanTLavavej removed the decision needed We need to choose something before working on this label Apr 12, 2023
@StephanTLavavej
Copy link
Member

We talked about this at the weekly maintainer meeting and have agreed to close this without merging. Thanks again for trying. 😸

@frederick-vs-ja frederick-vs-ja deleted the throughput-limits-cwchar branch April 12, 2023 23:17
@Kojoley
Copy link
Contributor

Kojoley commented Apr 13, 2023

I was wanting to make a similar change, replace <cwchar> with <stdint.h> and move _NATIVE_WCHAR_T_DEFINED over to wchar_t specialization, the statistics I collected from that was:

msvc
was
msvc
now
slow
down
header
0.067 0.038 -43% <limits>
0.101 0.070 -31% <bit>
0.111 0.081 -27% <compare>
0.122 0.090 -26% <typeindex>
0.115 0.085 -26% <coroutine>
0.120 0.090 -25% <utility>
0.137 0.106 -23% <tuple>
0.188 0.158 -16% <span>
0.196 0.166 -15% <cmath>
0.179 0.152 -15% <array>
0.179 0.153 -15% <numeric>
0.189 0.162 -14% <semaphore>
0.190 0.164 -14% <expected>
0.218 0.189 -13% <scoped_allocator>
0.222 0.193 -13% <list>
0.224 0.195 -13% <optional>
0.219 0.191 -13% <any>
0.228 0.200 -12% <set>
0.220 0.193 -12% <forward_list>
0.229 0.201 -12% <vector>
0.230 0.202 -12% <map>
0.221 0.195 -12% <deque>
0.248 0.219 -12% <valarray>
0.236 0.209 -11% <barrier>
0.260 0.232 -11% <charconv>
0.272 0.243 -11% <unordered_set>
0.275 0.246 -11% <variant>
0.245 0.220 -10% <stop_token>
0.272 0.245 -10% <unordered_map>
0.297 0.268 -10% <functional>
0.302 0.273 -10% <algorithm>
0.310 0.302 -3% <string_view>

#3627 made its impact lower, but still significant:

msvc
was
msvc
now
slow
down
header
0.068 0.036 -47% <limits>
0.100 0.069 -31% <bit>
0.191 0.155 -19% <numeric>
0.323 0.279 -14% <algorithm>
0.213 0.184 -14% <scoped_allocator>
0.191 0.165 -14% <semaphore>
0.215 0.187 -13% <any>
0.233 0.204 -12% <vector>
0.225 0.197 -12% <deque>
0.226 0.198 -12% <list>
0.225 0.199 -12% <map>
0.231 0.205 -11% <set>
0.216 0.192 -11% <forward_list>
0.240 0.214 -11% <barrier>
0.250 0.223 -11% <valarray>
0.241 0.215 -11% <stop_token>
0.279 0.249 -11% <unordered_map>
0.276 0.248 -10% <unordered_set>
0.291 0.263 -10% <functional>
0.517 0.497 -4% <codecvt>
0.747 0.728 -3% <execution>

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