Avoid MSVC C4702 unreachable code warning in print/println#4814
Avoid MSVC C4702 unreachable code warning in print/println#4814natedemoss wants to merge 1 commit into
Conversation
When `detail::use_utf8` is a compile-time constant, the `if FMT_CONSTEXPR20` branch in `print` and `println` always returns, leaving the trailing statement unreachable. Under C++20 this is an `if constexpr`, so MSVC reports the dead statement as warning C4702 at high warning levels (e.g. /Wall). Move the fallthrough into an `else` branch so it becomes part of the discarded `if constexpr` branch, which MSVC does not flag. The behavior is unchanged. Closes fmtlib#4810 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR refines control flow in fmt printing helpers by explicitly pairing fall-through logic with else, improving clarity and avoiding “uncontrolled statement”/misleading-indentation style diagnostics around early return paths.
Changes:
- Add explicit
elsebranches afterif FMT_CONSTEXPR20 (...) return ...inprint(...)overloads. - Add explicit
elsebranch inprintln(...)to clearly separate UTF-8 vs mojibake paths.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| detail::is_locking<T...>() ? vprint_buffered(f, fmt.str, va) | ||
| : vprint(f, fmt.str, va); | ||
| else | ||
| detail::is_locking<T...>() ? vprint_buffered(f, fmt.str, va) |
There was a problem hiding this comment.
Would 'return' in else branch follow code style as in if constexpr path?
| detail::is_locking<T...>() ? vprint_buffered(stdout, fmt.str, va) | ||
| : vprint(fmt.str, va); | ||
| else | ||
| detail::is_locking<T...>() ? vprint_buffered(stdout, fmt.str, va) |
There was a problem hiding this comment.
Would 'return' in else branch follow code style as in if constexpr path?
There was a problem hiding this comment.
I prefer removing the return from the if branch
There was a problem hiding this comment.
Either way is good to me, the proposal is to follow one pattern/style
| vargs<T...> va = {{args...}}; | ||
| if FMT_CONSTEXPR20 (detail::use_utf8) return vprintln(f, fmt.str, va); | ||
| detail::vprint_mojibake(f, fmt.str, va, true); | ||
| else detail::vprint_mojibake(f, fmt.str, va, true); |
There was a problem hiding this comment.
Would 'return' in else branch follow code style as in if constexpr path?
Problem
fmt::printandfmt::printlninbase.hdispatch on the compile-timeconstant
detail::use_utf8:When the condition is constant the
ifbranch always returns, so thetrailing statement is unreachable. In C++20
FMT_CONSTEXPR20expands toconstexpr, and MSVC reports the dead statement as warning C4702(unreachable code) at high warning levels.
Reproducer from #4810 (MSVC 19.51,
/std:c++20 /Wall, header-only):Depending on the value of
use_utf8for a given build, the same patternalso affects the two
printoverloads.Fix
Move the fallthrough statement into an
elsebranch so it becomes part ofthe discarded
if constexprbranch, which MSVC does not flag. Behavior isunchanged — exactly one branch ran before and runs now.
Testing
No compiler with MSVC was available locally, but the change was verified
to compile cleanly and behave correctly with GCC 15.2 under
-Wall -Wextra -Werrorin C++17 (plainif/else), C++20, and C++23(
if constexpr/else), including the wide-char path viafmt/xchar.h.The
elseform is the standard idiom for suppressing C4702 in a discardedif constexprbranch.Closes #4810