-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Use std::size over sizeof/sizeof #3090
Conversation
This is more concise than sizeof/sizeof, and std::size is a constexpr so there shouldn't be any cost.
Thanks - agreed that this is an improvement, and that there's no cost in release mode. (I'm not worried about minor consequences in debug mode.) I've pushed a pre-emptive fix for the MSVC-internal build, plus additional changes to update occurrences in |
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not concerned with the change since none of these uses are performance-critical, but note that this change can in general pessimize debug codegen. MSVC is quite reluctant to evaluate constant expressions that are not manifestly constant (used in a context that requires a constant expression) at compiletime when not optimizing. See https://godbolt.org/z/74eEe98zP for a demonstration.
Again, I'm happy to apply this, but keep in mind if you submit future similar PRs that we may need to sometimes force constant evaluation to avoid pessimizing codegen when adding calls to constexpr functions.
Thanks for this code cleanup! 😸 😸 😸 😸 😸 😸 |
Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
This is more concise than sizeof/sizeof, and std::size is a constexpr so there shouldn't be any cost.