Skip to content

Use cpp17 constexpr for casting #3113

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

Closed
wants to merge 2 commits into from

Conversation

Skylion007
Copy link
Collaborator

Description

This is an attempt to modernize and revive: #1589 for discussion. Particularly, the part about using constexpr for C++17 > . Closes #1589. This does increase the size of the so by about 224 bytes for C++17, but hopefully that will be worth it. I am using a lot of the STL I am unfamiliar with here. Open to closing this PR if it's determined to have no benefit or causes bizzarre compiler issues. Just wanted to start some machinery to use constexpr more aggressively in more modern C++ dialects that support

Suggested changelog entry:

* Allows constexpr to be used during casting in C++17 or newer.

@Skylion007 Skylion007 requested a review from rwgk July 14, 2021 18:20
@Skylion007 Skylion007 marked this pull request as draft July 14, 2021 18:21
@Skylion007 Skylion007 requested a review from henryiii July 14, 2021 18:31
@rwgk
Copy link
Collaborator

rwgk commented Jul 15, 2021

Your implementation avoiding the copied source code sections is definitely much better, but does it actually do anything?
Did you check if the +224 bytes size difference is reproducible going back and forth compiling from scratch a few times? Maybe it's just noise?

The compiler already knows that the ::value expressions are compile-time constant, the added constexpr seem redundant, the optimizer will probably (certainly?) figure out that it can discard the branch, but it could be that I'm missing something.

IIUC the main benefit of if constexpr is that - inside templates - template expansion is NOT applied to the code inside the if if false, which enables a much nicer and more intuitive code structure compared to the usually mind-bending alternative of SFINAE acrobatics. But that's not the situation in this PR, the if constexpr bodies are just simple enum assignments.

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