diff --git a/docs/mkdocs/docs/features/arbitrary_types.md b/docs/mkdocs/docs/features/arbitrary_types.md index 7ea01ee55..662175138 100644 --- a/docs/mkdocs/docs/features/arbitrary_types.md +++ b/docs/mkdocs/docs/features/arbitrary_types.md @@ -79,6 +79,7 @@ Some important things: * When using `get()`, `your_type` **MUST** be [DefaultConstructible](https://en.cppreference.com/w/cpp/named_req/DefaultConstructible). (There is a way to bypass this requirement described later.) * In function `from_json`, use function [`at()`](../api/basic_json/at.md) to access the object values rather than `operator[]`. In case a key does not exist, `at` throws an exception that you can handle, whereas `operator[]` exhibits undefined behavior. * You do not need to add serializers or deserializers for STL types like `std::vector`: the library already implements these. +* If you control the type, consider defining `to_json`/`from_json` as `friend` functions inside the class ("hidden friends"). Argument-dependent lookup then only finds them for your type, which also avoids a [GCC < 11 compilation error](../home/faq.md#incomplete-detector-type-with-gcc-11). ## Simplify your life with macros diff --git a/docs/mkdocs/docs/home/faq.md b/docs/mkdocs/docs/home/faq.md index 8b3602bd1..f16b6314a 100644 --- a/docs/mkdocs/docs/home/faq.md +++ b/docs/mkdocs/docs/home/faq.md @@ -307,6 +307,51 @@ APP_CPPFLAGS += -frtti -fexceptions The code compiles successfully with [Android NDK](https://developer.android.com/ndk/index.html?hl=ml), Revision 9 - 11 (and possibly later) and [CrystaX's Android NDK](https://www.crystax.net/en/android/ndk) version 10. +### Incomplete `detector` type with GCC < 11 + +!!! question + + Why does GCC 10 or older fail with `invalid use of incomplete type 'struct nlohmann::detail::detector<..., to_json_function, ...>'` for a type that holds an `optional` member? + +This happens with GCC 10 and older in C++11/C++14 mode when all of these hold: + +- a class `Holder` has an `optional` member (e.g., `boost::optional`), +- `Dummy` has a constructor taking a `json` value, and +- `to_json` for `Holder` is a free function in the namespace of `Dummy`. + +```cpp +class Dummy { + public: + explicit Dummy(const nlohmann::json& j); +}; + +class Holder { + boost::optional d; +}; + +void to_json(nlohmann::json& j, const Holder& h); // triggers the error +``` + +To decide whether `Dummy` is copyable, the compiler checks whether a `Dummy` can be converted to `json`. That check +looks up `to_json` via argument-dependent lookup, finds the unrelated `to_json` for `Holder`, and eventually asks again +whether `Dummy` is copyable. GCC before version 11 turns this cycle into a hard error; GCC 11 and later, Clang, and +C++17 mode compile the code. The same error shows up without this library whenever a constrained converting constructor +is involved, so the library can't avoid it. + +To work around this, define `to_json` (and `from_json`) as a *hidden friend* inside the class. That way, +argument-dependent lookup only finds it for `Holder`: + +```cpp +class Holder { + boost::optional d; + + friend void to_json(nlohmann::json& j, const Holder& h) { /* ... */ } +}; +``` + +The [`NLOHMANN_DEFINE_TYPE_INTRUSIVE`](../api/macros/nlohmann_define_type_intrusive.md) macros define hidden friends as +well. See [#3669](https://github.com/nlohmann/json/issues/3669) for details. + ### Missing STL function !!! question "Questions" diff --git a/tests/src/unit-regression2.cpp b/tests/src/unit-regression2.cpp index c466958bf..a77f12043 100644 --- a/tests/src/unit-regression2.cpp +++ b/tests/src/unit-regression2.cpp @@ -241,6 +241,51 @@ class my_allocator : public std::allocator }; }; +///////////////////////////////////////////////////////////////////// +// for #3669 +///////////////////////////////////////////////////////////////////// + +// mimics boost::optional's converting constructor, whose SFINAE check asks +// whether T is constructible from const U& +template +struct issue3669_is_constructible +{ + template()))> + static char test(int); + template + static long test(...); + static constexpr bool value = sizeof(test(0)) == 1; +}; + +template +class issue3669_optional +{ + public: + issue3669_optional() = default; + template + issue3669_optional(const issue3669_optional& /*unused*/, // NOLINT(google-explicit-constructor,hicpp-explicit-conversions) + typename std::enable_if::value, bool>::type /*unused*/ = true) {} +}; + +class Issue3669Dummy +{ + public: + explicit Issue3669Dummy(const json& /*unused*/) {} +}; + +class Issue3669Holder +{ + issue3669_optional d{}; + + // GCC < 11 (C++11/14) rejects a free to_json(json&, const Issue3669Holder&) + // here, because ADL for Issue3669Dummy finds it and closes an instantiation + // cycle; a hidden friend is only visible to ADL for Issue3669Holder + friend void to_json(json& j, const Issue3669Holder& /*unused*/) + { + j = "holder"; + } +}; + TEST_CASE("regression tests 2") { SECTION("issue #1001 - Fix memory leak during parser callback") @@ -766,6 +811,14 @@ TEST_CASE("regression tests 2") CHECK(j == k); } + SECTION("issue #3669 - invalid use of incomplete type with optional member and to_json") + { + const Issue3669Holder h{}; + const Issue3669Holder h2(h); // NOLINT(performance-unnecessary-copy-initialization) + const json j = h2; + CHECK(j == "holder"); + } + } TEST_CASE("regression test - parser callback must not lose a duplicate key's prior value")