fix: treat single-element brace-init as copy/move instead of wrapping in array (#5074)#5090
Conversation
|
See the first entry in the FAQ. https://json.nlohmann.me/home/faq/#brace-initialization-yields-arrays |
🔴 Amalgamation check failed! 🔴The source code has not been amalgamated. @ssam18 |
ssam18
force-pushed
the
fix/issue-5074-brace-init-semantics
branch
from
bee5b16 to
6ba0665
Compare
🔴 Amalgamation check failed! 🔴The source code has not been amalgamated. @ssam18 |
1 similar comment
🔴 Amalgamation check failed! 🔴The source code has not been amalgamated. @ssam18 |
|
There is currently no way to fix this behavior without breaking existing code. |
ssam18
force-pushed
the
fix/issue-5074-brace-init-semantics
branch
from
8986a1d to
336a8f7
Compare
ssam18
force-pushed
the
fix/issue-5074-brace-init-semantics
branch
from
6647c53 to
9563808
Compare
|
@nlohmann - Can you please take a look? |
|
I am not sure about this. Yet another macro adds another moving part to the library... |
|
It might also need to be encoded in the namespace so you don't accidentally link against a version with the wrong behavior. |
ssam18
force-pushed
the
fix/issue-5074-brace-init-semantics
branch
from
363a2ba to
58de969
Compare
🔴 Amalgamation check failed! 🔴The source code has not been amalgamated. @ssam18 |
1 similar comment
🔴 Amalgamation check failed! 🔴The source code has not been amalgamated. @ssam18 |
ssam18
force-pushed
the
fix/issue-5074-brace-init-semantics
branch
from
5bb8c14 to
feb3dad
Compare
Given how often this is reported as a bug, I think having a better answer than "We know, sorry, don't do that." is a good thing. I think this is one of those things that should become the only method in 4.0 and be selectable until then. |
ssam18
force-pushed
the
fix/issue-5074-brace-init-semantics
branch
from
8578552 to
187c763
Compare
|
@nlohmann - I have addressed your review comment. Please review |
When passing a json value using brace initialization with a single element
(e.g., `json j{someObj}` or `foo({someJson})`), C++ always prefers the
initializer_list constructor over the copy/move constructor. This caused
the value to be unexpectedly wrapped in a single-element array.
This bug was previously compiler-dependent (GCC wrapped, Clang did not),
but Clang 20 started matching GCC behavior, making it a universal issue.
Fix: In the initializer_list constructor, when type deduction is enabled
and the list has exactly one element, copy/move it directly instead of
creating a single-element array.
Before:
json obj = {{"key", 1}};
json j{obj}; // -> [{"key":1}] (wrong: array)
foo({obj}); // -> [{"key":1}] (wrong: array)
After:
json j{obj}; // -> {"key":1} (correct: copy)
foo({obj}); // -> {"key":1} (correct: copy)
To explicitly create a single-element array, use json::array({value}).
Fixes the issue nlohmann#5074
Signed-off-by: Samaresh Kumar Singh <[email protected]>
- Add missing comment from include/nlohmann/json.hpp explaining the single-element brace-init fix (issue nlohmann#5074) - Fix extra 4-space indentation in embedded json_fwd.hpp section Regenerated by running: make amalgamate Signed-off-by: Samaresh Kumar Singh <[email protected]>
The single-element brace-init change was a breaking change that cannot be accepted upstream. Reverted all related source, test, and doc changes, then regenerated single_include with correct indentation to pass the amalgamation CI check. Signed-off-by: Samaresh Kumar Singh <[email protected]>
…nn#5074 Single-element brace initialization wrapping in an array cannot be fixed without breaking existing code. Added JSON_BRACE_INIT_COPY_SEMANTICS as an opt-in macro (default 0) so users can enable copy/move semantics for single-element brace init without affecting anyone relying on the current behavior. Signed-off-by: Samaresh Kumar Singh <[email protected]>
…_COPY_SEMANTICS Signed-off-by: Samaresh Kumar Singh <[email protected]>
…on test Signed-off-by: Samaresh Kumar Singh <[email protected]>
…y -Wundef Signed-off-by: Samaresh Kumar Singh <[email protected]>
…tyle check Signed-off-by: Samaresh Kumar Singh <[email protected]>
… order Signed-off-by: Samaresh Kumar Singh <[email protected]>
ssam18
force-pushed
the
fix/issue-5074-brace-init-semantics
branch
from
06f4313 to
214e4cf
Compare
|
@nlohmann - Can we merge this PR? |
|
@nlohmann Have all your concerns been addressed? |
yes, that is true. |
|
Thanks! |
3 participants
Summary
When a value is passed using single-element brace initialization e.g.,
json j{someObj}or via a by-value parametervoid foo(json j)called asfoo({someObj}). C++ always prefers theinitializer_listconstructor over the copy/move constructor. This caused the value to be silently wrapped in a single-element array.This bug was previously compiler-dependent: GCC wrapped the value while Clang did not. As of Clang 20, Clang now matches GCC behavior, making this a universal regression.
Validation
All existing unit tests pass with the fix applied.
Fixes the issue #5074.