From 4cdc61e49356533c67227d74fffe39588388b0ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Th=C3=A9o=20DELRIEU?= <theo@tanker.io>
Date: Fri, 21 Oct 2016 16:28:01 +0200
Subject: [PATCH] move most SFINAE trickery in to/from_json_fn

---
 src/json.hpp                   | 128 +++++++++++++++++++++------------
 test/src/unit-constructor3.cpp |  68 +++++++++++++++---
 2 files changed, 142 insertions(+), 54 deletions(-)

diff --git a/src/json.hpp b/src/json.hpp
index 390d4e792..8b281d938 100644
--- a/src/json.hpp
+++ b/src/json.hpp
@@ -161,22 +161,70 @@ struct has_mapped_type
         std::is_integral<decltype(detect(std::declval<T>()))>::value;
 };
 
+void to_json();
+void from_json();
+
 struct to_json_fn
 {
-  template <typename T>
-  constexpr auto operator()(T&& val) const -> decltype(to_json(std::forward<T>(val)))
-  {
-    return to_json(std::forward<T>(val));
-  }
+  private:
+    // fallback overload
+    template <typename T>
+    static constexpr auto
+    impl(T &&val, long) noexcept(noexcept(to_json(std::forward<T>(val))))
+        -> decltype(to_json(std::forward<T>(val)))
+    {
+      return to_json(std::forward<T>(val));
+    }
+
+    // preferred overload
+    template <typename T>
+    static constexpr auto impl(T &&val, int) noexcept(
+        noexcept(json_traits<uncvref_t<T>>::to_json(std::forward<T>(val))))
+        -> decltype(json_traits<uncvref_t<T>>::to_json(std::forward<T>(val)))
+    {
+      return json_traits<uncvref_t<T>>::to_json(std::forward<T>(val));
+    }
+
+  public:
+    template <typename T>
+    constexpr auto operator()(T &&val) const
+        noexcept(noexcept(to_json_fn::impl(std::forward<T>(val), 0)))
+            -> decltype(to_json_fn::impl(std::forward<T>(val), 0))
+    {
+      // decltype(0) -> int, so the compiler will try to take the 'preferred overload'
+      // if there is no specialization, the 'fallback overload' will be taken by converting 0 to long
+      return to_json_fn::impl(std::forward<T>(val), 0);
+    }
 };
 
 struct from_json_fn
 {
-  template <typename Json, typename T>
-  constexpr auto operator()(Json const& from, T& to) const -> decltype(from_json(from, to))
-  {
-    return from_json(from, to);
-  }
+  private:
+    template <typename T, typename Json>
+    static constexpr auto impl(Json const &j, T &val,
+                               long) noexcept(noexcept(from_json(j, val)))
+        -> decltype(from_json(j, val))
+    {
+      return from_json(j, val);
+    }
+
+    template <typename T, typename Json>
+    static constexpr auto
+    impl(Json const &j, T &val,
+         int) noexcept(noexcept(json_traits<T>::from_json(j, val)))
+        -> decltype(json_traits<T>::from_json(j, val))
+    {
+      return json_traits<T>::from_json(j, val);
+    }
+
+  public:
+    template <typename T, typename Json>
+    constexpr auto operator()(Json const &j, T &val) const
+        noexcept(noexcept(from_json_fn::impl(j, val, 0)))
+            -> decltype(from_json_fn::impl(j, val, 0))
+    {
+      return from_json_fn::impl(j, val, 0);
+    }
 };
 
 /*!
@@ -1373,7 +1421,13 @@ class basic_json
         assert_invariant();
     }
 
-    // constructor chosen if json_traits is specialized for type T
+    // constructor chosen for user-defined types that either have:
+    // - a to_json free function in their type's namespace
+    // - a json_traits specialization for their type
+    //
+    // If there is both a free function and a specialization, the latter will be chosen,
+    // since it is a more advanced use
+    //
     // note: constructor is marked explicit to avoid the following issue:
     //
     // struct not_equality_comparable{};
@@ -1383,15 +1437,15 @@ class basic_json
     // this will construct implicitely 2 json objects and call operator== on them
     // which can cause nasty bugs on the user's in json-unrelated code
     // 
-    // the trade-off is expressivety in initializer-lists
+    // the trade-off is expressiveness in initializer-lists
     // auto j = json{{"a", json(not_equality_comparable{})}};
     // 
     // we can remove this constraint though, since lots of ctor are not explicit already
-    template <typename T, typename = decltype(json_traits<uncvref_t<T>>::to_json(std::declval<uncvref_t<T>>()))>
+    template <typename T, typename = decltype(::nlohmann::to_json(
+                              std::declval<uncvref_t<T>>()))>
     explicit basic_json(T &&val)
-        : basic_json(json_traits<uncvref_t<T>>::to_json(std::forward<T>(val)))
-    {
-    }
+        : basic_json(::nlohmann::to_json(std::forward<T>(val))) {}
+
     /*!
     @brief create a string (explicit)
 
@@ -2752,32 +2806,6 @@ class basic_json
     // value access //
     //////////////////
 
-    // get_impl overload chosen if json_traits struct is specialized for type T
-    // simply returns json_traits<T>::from_json(*this);
-    // dual argument to avoid conflicting with get_impl overloads taking a pointer
-    template <typename T>
-    auto get_impl(int, int) const -> decltype(json_traits<uncvref_t<T>>::from_json(*this))
-    {
-      return json_traits<uncvref_t<T>>::from_json(*this);
-    }
-
-    // this overload is chosen ONLY if json_traits struct is not specialized, and if the expression nlohmann::from_json(*this, T&) is valid
-    // I chose to prefer the json_traits specialization if it exists, since it's a more advanced use.
-    // But we can of course change this behaviour
-    template <typename T>
-    auto get_impl(long, long) const -> uncvref_t<decltype(::nlohmann::from_json(*this, std::declval<T &>()),
-                                                    std::declval<T>())>
-    {
-      remove_cv_t<T> ret;
-      // I guess this output parameter is the only way to get ADL
-      // Even if users can use the get<T> method to have a more 'functional' behaviour
-      // i.e. having a return type, could there be a way to have the same behaviour with from_json?
-      // e.g. auto t = nlohmann::from_json<T>(json{});
-      // this seems to require variable templates though... (at least it did when I tried to implement it)
-      ::nlohmann::from_json(*this, ret);
-      return ret;
-    }
-
     template<class T, typename std::enable_if<
                  std::is_convertible<typename object_t::key_type, typename T::key_type>::value and
                  std::is_convertible<basic_json_t, typename T::mapped_type>::value, int>::type = 0>
@@ -3082,16 +3110,24 @@ class basic_json
     */
     template<typename ValueType, typename std::enable_if<
                  not std::is_pointer<ValueType>::value, int>::type = 0>
-    auto get() const -> decltype(get_impl(static_cast<ValueType*>(nullptr)))
+    auto get() const -> decltype(this->get_impl(static_cast<ValueType*>(nullptr)))
     {
         return get_impl(static_cast<ValueType*>(nullptr));
     }
 
     template <typename ValueType>
-    auto get() const -> decltype(get_impl<ValueType>(0, 0))
-    {
-        return get_impl<ValueType>(0, 0);
+    auto get() const -> remove_reference_t<
+        decltype(::nlohmann::from_json(*this, std::declval<ValueType &>()),
+                 std::declval<ValueType>())>
+    {
+      static_assert(std::is_default_constructible<ValueType>::value,
+                    "ValueType must be default-constructible when user-defined "
+                    "from_json method is used");
+      ValueType ret;
+      ::nlohmann::from_json(*this, ret);
+      return ret;
     }
+
     /*!
     @brief get a pointer value (explicit)
 
diff --git a/test/src/unit-constructor3.cpp b/test/src/unit-constructor3.cpp
index cfe638673..de52762f6 100644
--- a/test/src/unit-constructor3.cpp
+++ b/test/src/unit-constructor3.cpp
@@ -35,6 +35,9 @@ using nlohmann::json;
 
 namespace udt
 {
+// only used by counter_type
+auto nb_free_function_calls = 0;
+
 struct empty_type {};
 struct pod_type {
   int a;
@@ -48,6 +51,10 @@ struct bit_more_complex_type {
   std::string c;
 };
 
+struct counter_type
+{
+};
+
 // best optional implementation ever
 template <typename T>
 class optional_type
@@ -97,14 +104,18 @@ json to_json(optional_type<T> const& opt)
   using nlohmann::to_json;
   if (!opt)
     return nullptr;
-  return to_json(*opt);
+  return json(*opt);
 }
 
 json to_json(no_json_traits_type const& p)
 {
-  json ret;
-  ret["a"] = p.a;
-  return ret;
+  return {{"a", p.a}};
+}
+
+json to_json(counter_type)
+{
+  ++nb_free_function_calls;
+  return json::object();
 }
 
 void from_json(json const&j, empty_type& t)
@@ -139,6 +150,11 @@ void from_json(json const& j, optional_type<T>& t)
     t = j.get<T>();
 }
 
+void from_json(json const&, counter_type&)
+{
+  ++nb_free_function_calls;
+}
+
 inline bool operator==(pod_type const& lhs, pod_type const& rhs) noexcept
 {
   return std::tie(lhs.a, lhs.b, lhs.c) == std::tie(rhs.a, rhs.b, rhs.c);
@@ -176,7 +192,7 @@ struct json_traits<udt::empty_type>
   {
     return json::object();
   }
-  
+
   static type from_json(json const& j)
   {
     assert(j.is_object() and j.empty());
@@ -193,7 +209,7 @@ struct json_traits<udt::pod_type>
   {
     return {{"a", t.a}, {"b", t.b}, {"c", t.c}};
   }
-  
+
   static type from_json(json const& j)
   {
     assert(j.is_object());
@@ -237,6 +253,25 @@ struct json_traits<udt::optional_type<T>>
     return type{j.get<T>()};
   }
 };
+
+template <>
+struct json_traits<udt::counter_type>
+{
+  using type = udt::counter_type;
+  static int nb_calls;
+
+  static json to_json(type)
+  {
+    ++nb_calls;
+    return json::object();
+  }
+
+  static void from_json(json const&, type&)
+  {
+    ++nb_calls;
+  }
+};
+int json_traits<udt::counter_type>::nb_calls{0};
 }
 
 
@@ -380,6 +415,24 @@ TEST_CASE("get<> for user-defined types", "[udt]")
       CHECK(*v == expected);
     }
   }
+
+  SECTION("no json_traits specialization, use of ADL")
+  {
+    udt::no_json_traits_type val{42};
+    auto const expected = json{{"a", 42}};
+    auto const j = json(val);
+    CHECK(j == expected);
+  }
+
+  SECTION("counter_type")
+  {
+    // check that the traits specialization is chosen
+    auto const j = json{udt::counter_type{}};
+    CHECK(nlohmann::json_traits<udt::counter_type>::nb_calls == 1);
+    auto const elem = j.get<udt::counter_type>();
+    CHECK(nlohmann::json_traits<udt::counter_type>::nb_calls == 2);
+    CHECK(udt::nb_free_function_calls == 0);
+  }
 }
 
 TEST_CASE("to_json free function", "[udt]")
@@ -487,8 +540,7 @@ TEST_CASE("from_json free function", "[udt]")
   {
     udt::no_json_traits_type expected{42};
     udt::no_json_traits_type res;
-    json j;
-    j["a"] = 42;
+    auto const j = json{{"a", 42}};
     nlohmann::from_json(j, res);
     CHECK(res == expected);
 
-- 
GitLab