[nlohmann]

Fixes #5742.

Note

Stacked on #5585 (Create a value before giving it its type), which contains the actual fix. This PR only adds the regression test.

Problem

With JSON_DIAGNOSTICS=1, GCC reports a false -Warray-bounds error at -O3 (so -DCMAKE_BUILD_TYPE=Release + -Werror fails). The warning is located in libstdc++'s stl_tree.h: GCC checks the std::map access in the value_t::object branch of the inlined set_parents() against the 32-byte allocation of a string.

Cause

external_constructor<value_t::string> set the type before allocating the string:

j.m_data.m_type = value_t::string;
j.m_data.m_value.string = j.template create<string_t>(str); // operator new

GCC has to assume that operator new could change any memory, including m_type. So when set_parents() reads m_type again afterwards, GCC cannot rule out the object branch. When the type is set after the value is created, GCC knows the value is a string and drops that branch. #5585 makes exactly this change in every place (for exception safety), so it fixes this warning too, and no pragma is needed.

Rewriting set_parents() itself does not help: an if/else instead of the switch, or an early return via is_structured(), still fails on every affected GCC.

Measured with the #5742 repro at -O3 -Werror=array-bounds (Docker images):

GCC	develop	develop, set_parents() as if/else	#5585 / this PR
11.5	builds	builds	–
12.5	error	error	builds
13.5	error	error	builds
14.4	error	error	builds
15.3	error	error	builds
16.2	error	error	builds

Change

• Add tests/src/unit-diagnostics-optimized.cpp with both known patterns (gcc 14.2 bug: array subscript out of bounds with JSON_DIAGNOSTICS #4819 and Release build fails when JSON diagnostics enabled #5742). On GCC, tests/CMakeLists.txt compiles it with -O3 -Werror=array-bounds, so the test fails to build if the warning returns. Verified with GCC 12 to 16: the test fails to build against develop (GCC 13 to 16) and builds and passes on this branch.
• An earlier version of this PR suppressed -Warray-bounds around set_parents() with a pragma. That is removed again; the history keeps it, so no force push was needed.

Public API

No breaking changes. This PR only adds a test; the library change is in #5585 and does not change the public API either.

---

This PR was written by Claude Code.

🤖 Generated with Claude Code

[IvanPizhenko]

Hi @nlohmann , my additional 5 cents: all these cases starting with case value_t::null can be removed, because they do not introduce any additional logic, just keeping default: here is already good enough.

[nlohmann]

Thanks! The explicit cases are intentional: we compile with -Wswitch-enum (GCC, and via -Weverything on Clang), which warns about every unlisted enumerator even when there is a default:. Listing all value_t cases is the library-wide convention (see cmake/clang_flags.cmake), so that adding a new value type makes the compiler point at every switch that needs a decision — for set_parents(), that would catch a new container type silently losing its parent pointers. It's also unrelated to this fix, so I'd like to keep it as is.

(This reply was written by Claude Code.)

[gregmarr]

I know you say here that you don't want to change it from the switch, but I wonder if using something like this would help avoid the bug:

if (m_data.m_type == value_t::array)
{
  for (auto& element : *m_data.m_value.array)
  {
    element.m_parent = this;
  }
}
else if (m_data.m_type == value_t::object)
{
  for (auto& element : *m_data.m_value.object)
  {
    element.second.m_parent = this;
  }
 }

I really don't like having to disable warnings in code to work around compiler bugs, especially with the repro sample previously working in 15.

[nlohmann]

Thanks for pushing back on the pragma. You were right that there is a better fix, though the if/else rewrite is not it: I tried it and the warning stays on GCC 12.5, 13, 14, 15.3 and 16.2.

The cause is outside set_parents(). external_constructor<value_t::string> stored m_type = value_t::string before calling create<string_t>(). GCC has to assume that operator new could change m_type, so after inlining it couldn't rule out the object branch, however the branch is written, and checked the std::map access against the string's 32-byte allocation. Setting the type after creating the value fixes it on all of these GCC versions. #5585 already makes exactly that change everywhere (for exception safety).

So I've stacked this PR on #5585 and removed the pragma. This PR now only adds the -O3 -Werror=array-bounds regression test. That also answers your question about limiting the pragma to certain versions: there's no pragma left, and the affected range turned out to be GCC 12 to at least 16.2 anyway. Details and the measurements are in the updated description.

(This reply was written by Claude Code.)

[gregmarr]

Excellent.

[gregmarr]

I know you say here that you don't want to change it from the switch, but I wonder if using something like this would help avoid the bug:

if (m_data.m_type == value_t::array)
{
  for (auto& element : *m_data.m_value.array)
  {
    element.m_parent = this;
  }
}
else if (m_data.m_type == value_t::object)
{
  for (auto& element : *m_data.m_value.object)
  {
    element.second.m_parent = this;
  }
 }

I really don't like having to disable warnings in code to work around compiler bugs, especially with the repro sample previously working in 15.