Use the #3669 fixture's optional member in to_json

Issue3669Holder::d is never read, so clang's -Weverything -Werror build
(ci_test_clang) fails with -Wunused-private-field. Reference the member
in the hidden-friend to_json; this does not affect the instantiation
cycle the fixture reproduces.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

docs/mkdocs/docs/features/arbitrary_types.md

@@ -79,6 +79,7 @@ Some important things:
 * When using `get<your_type>()`, `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

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<Dummy>` 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<Dummy> 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<Dummy> 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"

tests/src/unit-msgpack.cpp

@@ -2165,13 +2165,6 @@ TEST_CASE("MessagePack with std::byte")
 #endif
 
 // the fake sizes below do not fit into a 32-bit std::size_t
-// with clang and libstdc++ 10, the std::filesystem::path conversion that
-// C++17 builds consider for every string type is ambiguous for a class
-// derived from std::string, so the string case is not tested there
-#if !(defined(__clang__) && defined(_GLIBCXX_RELEASE) && _GLIBCXX_RELEASE < 11)
-    #define JSON_TEST_BEYOND_UINT32_STRING 1
-#endif
-
 #if SIZE_MAX > UINT32_MAX
 template<typename T, typename A = std::allocator<T>>
 struct huge_array : std::vector<T, A>
@@ -2269,12 +2262,11 @@ TEST_CASE("MessagePack Size above uint32 for object")
     object.fake_size = false;
 }
 
-#ifdef JSON_TEST_BEYOND_UINT32_STRING
 struct huge_string : std::string
 {
     using std::string::string;
 
-    std::size_t size() const noexcept // NOLINT(readability-convert-member-functions-to-static)
+    std::size_t size() const noexcept
     {
         return static_cast<std::size_t>(UINT32_MAX) + 1ULL;
     }
@@ -2295,20 +2287,20 @@ using huge_string_json = nlohmann::basic_json <
 
 TEST_CASE("MessagePack Size above uint32 for string")
 {
-    const huge_string_json j = "hello";
+
+    huge_string_json j = "hello";
 
     CHECK_THROWS_WITH_AS(
         huge_string_json::to_msgpack(j),
         "[json.exception.out_of_range.412] MessagePack length 4294967296 exceeds maximum of 4294967295",
         json::out_of_range&);
 }
-#endif
 
 struct huge_binary : std::vector<std::uint8_t>
 {
     using std::vector<std::uint8_t>::vector;
 
-    std::size_t size() const noexcept // NOLINT(readability-convert-member-functions-to-static)
+    std::size_t size() const noexcept
     {
         return static_cast<std::size_t>(UINT32_MAX) + 1ULL;
     }
@@ -2363,6 +2355,13 @@ class beyond_uint32_binary_t : public std::vector<std::uint8_t>
     }
 };
 
+// with clang and libstdc++ 10, the std::filesystem::path conversion that
+// C++17 builds consider for every string type is ambiguous for a class
+// derived from std::string, so the string case is not tested there
+#if !(defined(__clang__) && defined(_GLIBCXX_RELEASE) && _GLIBCXX_RELEASE < 11)
+    #define JSON_TEST_BEYOND_UINT32_STRING 1
+#endif
+
 #ifdef JSON_TEST_BEYOND_UINT32_STRING
 class beyond_uint32_string_t : public std::string
 {

tests/src/unit-regression2.cpp

@@ -241,6 +241,52 @@ class my_allocator : public std::allocator<T>
     };
 };
 
+/////////////////////////////////////////////////////////////////////
+// for #3669
+/////////////////////////////////////////////////////////////////////
+
+// mimics boost::optional's converting constructor, whose SFINAE check asks
+// whether T is constructible from const U&
+template<class T, class Arg>
+struct issue3669_is_constructible
+{
+    template<class T2, class A2, class = decltype(T2(std::declval<A2>()))>
+    static char test(int);
+    template<class, class>
+    static long test(...);
+    static constexpr bool value = sizeof(test<T, Arg>(0)) == 1;
+};
+
+template<class T>
+class issue3669_optional
+{
+  public:
+    issue3669_optional() = default;
+    template<class U>
+    issue3669_optional(const issue3669_optional<U>& /*unused*/, // NOLINT(google-explicit-constructor,hicpp-explicit-conversions)
+                       typename std::enable_if<issue3669_is_constructible<T, const U&>::value, bool>::type /*unused*/ = true) {}
+};
+
+class Issue3669Dummy
+{
+  public:
+    explicit Issue3669Dummy(const json& /*unused*/) {}
+};
+
+class Issue3669Holder
+{
+    issue3669_optional<Issue3669Dummy> 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& h)
+    {
+        static_cast<void>(h.d); // silence -Wunused-private-field
+        j = "holder";
+    }
+};
+
 TEST_CASE("regression tests 2")
 {
     SECTION("issue #1001 - Fix memory leak during parser callback")
@@ -766,6 +812,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")