EnumUtils: Add Common::ToUnderlying #11962
Conversation
Mirrors the C++23 <utility> function, std::to_underlying
Minty-Meeo
force-pushed
the
to-underlying
branch
from
f55d2d7
to
07ad75f
Compare
|
Okay like half of your changes are actually changing the casted-to type here. If you want to argue that the casts were previously incorrect, be my guest (probably true for some of these, and some of the enums may have been intended as unsigned in the first place), but this is too many changes for me to casually merge in a single commit, even if it mainly affects logging. |
|
I previously traced these type static_casts back to a commit by @Pokechu22. Seen here: cc56402. These were previously printed in the underlying type. In a way, this is actually a restoration. It's a good demonstration of what makes std::to_underlying / Common::ToUnderlying useful. |
|
Mhm, fair enough, that's a pain in the ass to cross-check but I see your point. Let's see... |
|
Perhaps Pokechu22 can chime in about whether these enums are better represented as a different type. |
| @@ -119,11 +120,11 @@ void WriteVertexLighting(ShaderCode& out, APIType api_type, std::string_view wor | |||
| out.Write(" if ({} != 0u) {{\n", BitfieldExtract<&LitChannel::enablelighting>("alphareg")); | |||
| out.Write(" if ({} != 0u) {{\n", BitfieldExtract<&LitChannel::ambsource>("alphareg")); | |||
| out.Write(" if ((components & ({}u << chan)) != 0u) // VB_HAS_COL0\n", | |||
| static_cast<u32>(VB_HAS_COL0)); | |||
| Common::ToUnderlying(VB_HAS_COL0)); | |||
There was a problem hiding this comment.
These (as well as the ones in UberShaderVertex.cpp) look really sketchy though, this code would clearly break if it actually printed as signed. None of the values are actually big enough for that to happen, but this is clearly intended as unsigned.
There was a problem hiding this comment.
Multiple other places use it to modify u32s. Perhaps the underlying type for this one should be changed to that.
|
Okay, the Ubershader changes look wrong to me, but for everything else this is fine. From a quick glance the enum in |
|
Okay, yes, this looks correct to me now. I'll leave it open for a bit in case Pokechu wants to chime in. |
|
I believe I just defaulted to using u32 because it felt better to me, with no deeper meaning. |
|
Alright then. |
EnumUtils: Add Common::ToUnderlying
Mirrors the C++23 <utility> function,
std::to_underlying.This is an extract of changes from #11932.