From 855f511db4fc5b910ab8228f7eb4334d590105b5 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Thu, 9 Jul 2026 20:18:28 +0200 Subject: [PATCH] Fix stale Clang -Weverything suppression comments; drop -Wno-missing-noreturn (#5250) * Fix stale Clang -Weverything suppression comments; eliminate -Wno-missing-noreturn cmake/clang_flags.cmake claimed -Wno-unsafe-buffer-usage was needed only for Doctest and that -Wno-missing-noreturn had "no way to silence... otherwise" (PR #4871, which never actually attempted a source fix). Neither held up under investigation (todo 130): - -Wno-unsafe-buffer-usage is pervasive (208 distinct sites across 19 files measured with clang trunk in silkeh/clang:dev), spanning the library's own low-level numeric/buffer code (to_chars, serializer, lexer, binary reader/writer, input adapters, json_pointer) as well as vendored Doctest itself (96 of the 208 sites). A source-level fix is not feasible at this scale; the comment now says so instead of blaming Doctest alone. - -Wno-missing-noreturn had exactly two real trigger sites, both genuinely and unconditionally non-returning: a test-only throwing allocator (tests/src/unit-allocator.cpp) and, previously undiscovered, wide_string_input_adapter::get_elements() in include/nlohmann/detail/input/input_adapters.hpp. Verified this isn't a wider pattern by checking all 160 JSON_THROW call sites in the library for functions whose entire body is an unconditional throw. Annotated both ([[noreturn]] in the test file, since JSON_HEDLEY_NO_RETURN is #undef'd by the time test code runs; JSON_HEDLEY_NO_RETURN in the library file, its first real use anywhere in the codebase) and dropped the suppression entirely. single_include/nlohmann/json.hpp regenerated via `make amalgamate`; `make check-amalgamation` passes. Verified in Docker (silkeh/clang:dev, matching the ci_static_analysis_clang CI job): baseline builds clean, and the full 194-target test suite builds with zero warnings under the corrected CLANG_CXXFLAGS (-Wno-missing-noreturn no longer in the list). Also sanity-compiled and ran unit-allocator.cpp and unit-wstring.cpp on host Apple Clang to confirm behavior is unchanged. Signed-off-by: Niels Lohmann * Fix MSVC C4702 warning caused by JSON_HEDLEY_NO_RETURN on get_elements() PR #5250 annotated wide_string_input_adapter::get_elements() with JSON_HEDLEY_NO_RETURN (it unconditionally throws). On MSVC this expands to __declspec(noreturn), and MSVC correctly determined that the code following its call in binary_reader.hpp is unreachable for that instantiation, firing C4702 under /W4 /WX in the msvc, msvc-vs2026, and msvc-arm64 Debug jobs. Clang doesn't flag this case, so the Docker verification for #5250 (which only checked Clang -Weverything) didn't catch it. This is the same warning class already tolerated for Release builds since PR #5216, where MSVC's optimizer independently found the same dead code after /Od was removed. Extend that existing /wd4702 suppression to Debug builds too, instead of reverting the noreturn annotation. Signed-off-by: Niels Lohmann --------- Signed-off-by: Niels Lohmann --- cmake/clang_flags.cmake | 8 +++++--- include/nlohmann/detail/input/input_adapters.hpp | 2 +- single_include/nlohmann/json.hpp | 2 +- tests/CMakeLists.txt | 6 +++++- tests/src/unit-allocator.cpp | 2 +- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/cmake/clang_flags.cmake b/cmake/clang_flags.cmake index b3dfa3144..aea17820c 100644 --- a/cmake/clang_flags.cmake +++ b/cmake/clang_flags.cmake @@ -5,8 +5,11 @@ # -Wno-extra-semi-stmt The library uses assert which triggers this warning. # -Wno-padded We do not care about padding warnings. # -Wno-covered-switch-default All switches list all cases and a default case. -# -Wno-unsafe-buffer-usage Otherwise Doctest would not compile. -# -Wno-missing-noreturn We found no way to silence this warning otherwise, see PR #4871 +# -Wno-unsafe-buffer-usage Pervasive: the library's own low-level numeric/buffer code +# (to_chars, serializer, lexer, binary reader/writer, input +# adapters, json_pointer) plus vendored Doctest itself (~208 +# distinct sites measured 2026-07-08 on clang trunk) all use +# raw pointer arithmetic / libc string calls by necessity. set(CLANG_CXXFLAGS -Werror @@ -18,5 +21,4 @@ set(CLANG_CXXFLAGS -Wno-padded -Wno-covered-switch-default -Wno-unsafe-buffer-usage - -Wno-missing-noreturn ) diff --git a/include/nlohmann/detail/input/input_adapters.hpp b/include/nlohmann/detail/input/input_adapters.hpp index 94af90202..920a26adc 100644 --- a/include/nlohmann/detail/input/input_adapters.hpp +++ b/include/nlohmann/detail/input/input_adapters.hpp @@ -430,7 +430,7 @@ class wide_string_input_adapter // parsing binary with wchar doesn't make sense, but since the parsing mode can be runtime, we need something here template - std::size_t get_elements(T* /*dest*/, std::size_t /*count*/ = 1) + JSON_HEDLEY_NO_RETURN std::size_t get_elements(T* /*dest*/, std::size_t /*count*/ = 1) { JSON_THROW(parse_error::create(112, 1, "wide string type cannot be interpreted as binary data", nullptr)); } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 806b02ca9..0b034892e 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -7267,7 +7267,7 @@ class wide_string_input_adapter // parsing binary with wchar doesn't make sense, but since the parsing mode can be runtime, we need something here template - std::size_t get_elements(T* /*dest*/, std::size_t /*count*/ = 1) + JSON_HEDLEY_NO_RETURN std::size_t get_elements(T* /*dest*/, std::size_t /*count*/ = 1) { JSON_THROW(parse_error::create(112, 1, "wide string type cannot be interpreted as binary data", nullptr)); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 317ada91f..4383b582c 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -68,7 +68,11 @@ target_compile_options(test_main PUBLIC # Disable warning C4566: character represented by universal-character-name '\uFF01' # cannot be represented in the current code page (1252) # Disable warning C4996: 'nlohmann::basic_json<...>::operator <<': was declared deprecated - $<$:/W4;/wd4566;/wd4996;$<$:/wd4702>> + # Disable warning C4702: unreachable code; wide_string_input_adapter::get_elements() + # is annotated JSON_HEDLEY_NO_RETURN (it always throws), which + # makes MSVC flag the code following its call in binary_reader.hpp + # as unreachable for that instantiation, in both Debug and Release + $<$:/W4;/wd4566;/wd4996;/wd4702> # https://github.com/nlohmann/json/issues/1114 $<$:/bigobj> $<$:-Wa,-mbig-obj> diff --git a/tests/src/unit-allocator.cpp b/tests/src/unit-allocator.cpp index 6f2c70148..2dbb746b0 100644 --- a/tests/src/unit-allocator.cpp +++ b/tests/src/unit-allocator.cpp @@ -24,7 +24,7 @@ struct bad_allocator : std::allocator template bad_allocator(const bad_allocator& /*unused*/) { } template - void construct(T* /*unused*/, Args&& ... /*unused*/) // NOLINT(cppcoreguidelines-missing-std-forward) + [[noreturn]] void construct(T* /*unused*/, Args&& ... /*unused*/) // NOLINT(cppcoreguidelines-missing-std-forward) { throw std::bad_alloc(); }