fmtlib / fmt

A modern formatting library
https://fmt.dev
Other
19.84k stars 2.42k forks source link

Added else branches to if constexpr statements to silence MSVC #3971

Closed matt77hias closed 1 month ago

vitaut commented 1 month ago

Thanks for the PR but could you provide a godbolt repro?

matt77hias commented 1 month ago

It is basically MSVC logging warnings about logically dead code. I'll see if the new compiler version is already available and I can throw something small together.

Edit: I cannot provide a godbolt repro.

I cannot reproduce it on a release build locally. It only happens for debug builds:

...External\fmt\base.h(2713,1): warning C4702: unreachable code
...External\fmt\base.h(2724,1): warning C4702: unreachable code

And then like 100+ of this pair of warnings.

vitaut commented 1 month ago

This is clearly a false positive since the code is reachable for some values of template parameters so I recommend reporting to Microsoft. Because of this and since it's level 4 warning, I suggest using other means to suppress it (e.g. FMT_SYSTEM) unless it can be done in a cleaner way.

matt77hias commented 1 month ago

This is clearly a false positive since the code is reachable for some values of template parameters so I recommend reporting to Microsoft.

Yes, but that is not what MSVC complains about. If the if constexpr condition is true and the if-branch unconditionally returns, the code following the if constexpr statement that is not in an else-branch is dead for that template specialization. It is not complaining for the template specializations for which the if constexpr condition is false.

Imho I would also have thought early-outing from an if constexpr wasn't possible, and one would also need else branches as well. But no idea why MSVC is inconsistent between release and debug builds.