From d0de6a9111aa9aed9c4f881846ec2a05ee9b2501 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Thu, 9 Jul 2026 19:03:49 +0200 Subject: [PATCH] Document std::optional direct construction limitation (#5247) * Document std::optional direct-init/copy-init limitation with null Add regression test pinning current behavior (CHECK_THROWS_AS) in the null section of unit-conversions.cpp with detailed comment explaining the C++ language-level cause (std::optional's own converting constructor wins overload resolution over basic_json::operator T()). Add a warning callout in conversions.md documenting that direct construction/ assignment of std::optional from JSON null throws type_error 302, with a clear workaround (use get>() or get_to() instead, which correctly produce std::nullopt). This is a limitation at the language level: there is no SFINAE path to distinguish "called from inside std::optional's own constructor" from "direct call", so fixing it would require breaking changes to operator ValueType(). A permanent fix belongs in the 4.0 type-strictness redesign (#3453). Signed-off-by: Niels Lohmann Co-Authored-By: Claude Code * Fix issue reference in std::optional test comment Update the comment in the null section test to reference #5246 instead of placeholder #XXXX, clarifying where the direct-init/copy-init limitation is tracked. Signed-off-by: Niels Lohmann * Use CHECK_THROWS_AS_WITH for std::optional test assertions Update the regression tests to use CHECK_THROWS_AS_WITH instead of CHECK_THROWS_AS to verify both the exception type and the error message. Signed-off-by: Niels Lohmann * Fix CI: use CHECK_THROWS_WITH_AS, the macro that actually exists CHECK_THROWS_AS_WITH is not a doctest macro; the correct one used throughout this test suite is CHECK_THROWS_WITH_AS(expr, message, exception_type&), with the message before the type and the type as a reference. The previous commit didn't catch this because it only compiled the file standalone with default settings; this TEST_CASE only compiles under `#if !JSON_USE_IMPLICIT_CONVERSIONS`, which is why ci_test_noimplicitconversions was the job that failed. Verified by building and running the test in that exact configuration (JSON_USE_IMPLICIT_CONVERSIONS=0): 14/14 assertions pass. Signed-off-by: Niels Lohmann * Run std::optional test under default implicit-conversions build too TEST_CASE("std::optional") was guarded by #if !JSON_USE_IMPLICIT_CONVERSIONS, so it only ever compiled in the non-default build with implicit conversions disabled. This traces back to commit 1d7688aef (fixes #3859), which changed a previously dead #ifndef JSON_USE_IMPLICIT_CONVERSIONS guard (the macro is always defined by that point, so it never held) to #if !JSON_USE_IMPLICIT_CONVERSIONS -- making the test compile for the first time, but only in the disabled-conversions build. As a result, std::optional support had zero test coverage in the default configuration almost every user builds with. Verified the entire test case (all sections: null, string, bool, number, array, object) compiles and passes identically with JSON_USE_IMPLICIT_CONVERSIONS both on (default) and off -- nothing in it actually depends on the setting. Removing the guard closes the coverage gap with no behavior change: 285 assertions pass with implicit conversions on, 232 with them off (the difference comes from other, unrelated conditionally-compiled tests in this file). Signed-off-by: Niels Lohmann * :rotating_light: fix warning Signed-off-by: Niels Lohmann --------- Signed-off-by: Niels Lohmann Co-authored-by: Claude Code --- docs/mkdocs/docs/features/conversions.md | 18 ++++++++++++++++++ tests/src/unit-conversions.cpp | 18 ++++++++++++++---- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/docs/mkdocs/docs/features/conversions.md b/docs/mkdocs/docs/features/conversions.md index eaa213324..7f63587fd 100644 --- a/docs/mkdocs/docs/features/conversions.md +++ b/docs/mkdocs/docs/features/conversions.md @@ -66,6 +66,24 @@ which forces the explicit `get` form and can catch unintended conversions at com floating-point value as an integer truncates it, and narrowing conversions may overflow. See [number conversion](types/number_handling.md#number-conversion) for details and how to guard against it. +!!! warning "std::optional direct construction from JSON null throws" + + Constructing or assigning `std::optional` directly from a JSON value does not correctly produce + `std::nullopt` for a JSON `null`: + + ```cpp + json j_null; + std::optional opt = j_null; // ❌ throws type_error 302 + ``` + + This is due to C++ language rules: `std::optional` has its own converting constructor that is chosen over + `basic_json::operator T()` when both are viable. Use `get>()` or `get_to()` instead: + + ```cpp + auto opt = j_null.get>(); // ✅ std::nullopt + j_null.get_to(opt); // ✅ std::nullopt + ``` + ## Putting values in The reverse direction works the same way: assigning or constructing a `json` from a C++ value converts it to JSON. diff --git a/tests/src/unit-conversions.cpp b/tests/src/unit-conversions.cpp index 7542ef630..ad6623401 100644 --- a/tests/src/unit-conversions.cpp +++ b/tests/src/unit-conversions.cpp @@ -1761,16 +1761,27 @@ TEST_CASE("std::filesystem::path") } #endif -#if !JSON_USE_IMPLICIT_CONVERSIONS TEST_CASE("std::optional") { SECTION("null") { - json j_null; - std::optional opt_null; + const json j_null; + const std::optional opt_null; CHECK(json(opt_null) == j_null); CHECK(j_null.get>() == std::nullopt); + + // Constructing std::optional directly from JSON null throws because + // std::optional's own converting constructor is chosen over basic_json's + // operator T(). This is a language-level limitation (std::optional is + // constructible from T, and T is constructible from basic_json via the + // operator); there is no SFINAE path that distinguishes "call from inside + // std::optional's constructor" from "direct call". Use get>() + // or get_to() instead for correct null handling. See #4864 and #5246. + CHECK_THROWS_WITH_AS(std::optional(j_null), + "[json.exception.type_error.302] type must be string, but is null", json::type_error&); + CHECK_THROWS_WITH_AS(std::optional(j_null), + "[json.exception.type_error.302] type must be number, but is null", json::type_error&); } SECTION("string") @@ -1819,7 +1830,6 @@ TEST_CASE("std::optional") } } #endif -#endif #ifdef JSON_HAS_CPP_17 #undef JSON_HAS_CPP_17