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

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

Closed
wants to merge 1 commit into from

Conversation

matt77hias
Copy link
Contributor

No description provided.

@vitaut
Copy link
Contributor

vitaut commented May 23, 2024

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

@matt77hias
Copy link
Contributor Author

matt77hias commented May 23, 2024

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.

  • godbolt _MSC_VER == 1939 vs local _MSC_VER == 1940
    Also godbolt only supports fmt + MSVC with vcpkg (2023.08.09), not trunk.

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
Copy link
Contributor

vitaut commented May 25, 2024

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.

@vitaut vitaut closed this May 25, 2024
@matt77hias
Copy link
Contributor Author

matt77hias commented May 25, 2024

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.

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