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

Fix MSVC Win32 count_digits #2341

Merged
merged 1 commit into from
Jun 4, 2021
Merged

Fix MSVC Win32 count_digits #2341

merged 1 commit into from
Jun 4, 2021

Conversation

upsj
Copy link
Contributor

@upsj upsj commented Jun 4, 2021

We are shifting past 32 bit since the integer literals are 32 bits wide.

Additionally, it looks like having the constexpr table inside the function leads to pretty bad codegen on MSVC, so it might make sense to move it outside the function, though I didn't include that here since I don't know how you deal with constexpr globals

@vitaut vitaut merged commit 7c8b35f into fmtlib:master Jun 4, 2021
@vitaut
Copy link
Contributor

vitaut commented Jun 4, 2021

Thank you!

As for the table, an easy fix is to make it static on MSVC (it cannot be made static unconditionally because of some clang quirk).

@upsj
Copy link
Contributor Author

upsj commented Jun 4, 2021

Yeah, I noticed the same thing. Not sure whether the standard allows it, but constexpr static would be what I would ideally want to use

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants