Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assigning between different json types #977

Closed
yosagi opened this issue Feb 21, 2018 · 11 comments · Fixed by #986
Closed

Assigning between different json types #977

yosagi opened this issue Feb 21, 2018 · 11 comments · Fixed by #986
Assignees
Labels
solution: duplicate the issue is a duplicate; refer to the linked issue instead solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@yosagi
Copy link

yosagi commented Feb 21, 2018

I have 2 different json types, like following:

using foo_json = nlohmann::basic_json<std::map, std::vector, std::string, bool, std::int64_t,
 std::uint64_t, double, std::allocator,ns::foo_serializer>;

using json = nlohmann::json

Basically, they are same type except their JSONSerializer, and I want to copy/assign them one another. But when I copy one type to the other, the program receives SIGSEGV. It seems stack overflow.
How can I make it work?

Demonstration code:

#include <nlohmann/json.hpp>
#include <iostream>
#include <type_traits>

namespace ns{
struct foo{
  int x;
};

template <typename, typename SFINAE = void>
struct foo_serializer;

template<typename T>
struct foo_serializer<T,typename std::enable_if<std::is_same<foo, T>::value>::type> {
  template <typename BasicJsonType>
  static void to_json(BasicJsonType& j, const T& value) {
    j=BasicJsonType{{"x",value.x}};
  }
  template <typename BasicJsonType>
  static void from_json(const BasicJsonType& j, foo& value) {
    nlohmann::from_json(j.at("x"),value.x);
  }
};

template<typename T>
struct foo_serializer<T, typename std::enable_if<!std::is_same<foo, T>::value>::type> {
  template <typename BasicJsonType>
  static void to_json(BasicJsonType& j, const T& value) {
    ::nlohmann::to_json(j, value);
  }
  template <typename BasicJsonType>
  static void from_json(const BasicJsonType& j, foo& value) {
    ::nlohmann::from_json(j, value);
  }
};
}

using foo_json = nlohmann::basic_json<std::map, std::vector, std::string, bool, std::int64_t, 
std::uint64_t, double, std::allocator,ns::foo_serializer>;

int main(){
  foo_json lj=ns::foo{3};
  ns::foo ff=lj;
  std::cout<<lj<<std::endl;
  std::cout<<ff.x<<std::endl;
  nlohmann::json nj=lj;             // !!! Crash here !!!
}

Tested compilers

  • g++ 5.4
  • clang 3.8
@nlohmann
Copy link
Owner

Related: #972

@nlohmann nlohmann added the solution: duplicate the issue is a duplicate; refer to the linked issue instead label Feb 21, 2018
@theodelrieu
Copy link
Contributor

I haven't tried with #972 yet, but this issue is because of the to/from_json mechanism.

Trying to convert from foo_json to nlohmann::json calls the CompatibleArrayType overload, which cause an infinite recursion.

In order to fix it, we have to decide if we want to support conversions between different basic_json types.
Once we have a defined behavior it should be relatively easy to make it work.

@gregmarr
Copy link
Contributor

gregmarr commented Feb 21, 2018

I would say that it should be supported as much as possible, as long as the data storage types are compatible. If not, then it should probably be a compile error instead of a runtime error.
At that point, it's basically equivalent to assigning a vector with one allocator to a vector with a different one, or a map with one comparator to a map with a different one.

@nlohmann
Copy link
Owner

I think having a conversion between different basic_json types is very important, because the status quo is very surprising.

@yosagi
Copy link
Author

yosagi commented Feb 26, 2018

Thanks for replies and a discussion.
I want to use different json types in the same code base to switch serialization policy, such as a struct should be mapped to json array or json object. I have 2 set of serializers, but resulting json types are exchangeable only via from_msgpack(to_msgpack(j)) or similar calls.
If the problem is difficult to avoid by some external code, please support this kind of operation by the library.

@theodelrieu
Copy link
Contributor

I'll work on it this week.

@theodelrieu
Copy link
Contributor

@yosagi Could you try the fix present in #986?

@yosagi
Copy link
Author

yosagi commented Feb 28, 2018

It worked! Thanks.
The demonstration code shown above no longer compiles.

In file included from /home/yos/work/draft/json/testconv.cc:1:
/home/yos/work/draft/json/json/single_include/nlohmann/json.hpp:10885:78: error: 
      no matching member function for call to 'get'
  ...val.template get<other_boolean_t>());
     ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
/home/yos/work/draft/json/testconv.cc:45:21: note: in instantiation of function
      template specialization 'nlohmann::basic_json<std::map, std::vector,
      std::__cxx11::basic_string<char>, bool, long, unsigned long, double,
      std::allocator, adl_serializer>::basic_json<nlohmann::basic_json<std::map,
      std::vector, std::__cxx11::basic_string<char>, bool, long, unsigned long,
      double, std::allocator, ns::foo_serializer>, 0>' requested here
  nlohmann::json nj=lj;        // Crash here

foo in the signature of from_json should be T.
This is a working code just in case.

#include <nlohmann/json.hpp>
#include <iostream>
#include <type_traits>

namespace ns{
struct foo{
  int x;
};

template <typename, typename SFINAE = void>
struct foo_serializer;

template<typename T>
struct foo_serializer<T,typename std::enable_if<std::is_same<foo, T>::value>::type> {
  template <typename BasicJsonType>
  static void to_json(BasicJsonType& j, const T& value) {
    j=BasicJsonType{{"x",value.x}};
  }
  template <typename BasicJsonType>
  static void from_json(const BasicJsonType& j, T& value) {   // !!!
    nlohmann::from_json(j.at("x"),value.x);
  }
};

template<typename T>
struct foo_serializer<T, typename std::enable_if<!std::is_same<foo, T>::value>::type> {
  template <typename BasicJsonType>
  static void to_json(BasicJsonType& j, const T& value) {
    ::nlohmann::to_json(j, value);
  }
  template <typename BasicJsonType>
  static void from_json(const BasicJsonType& j, T& value) { //!!!
    ::nlohmann::from_json(j, value);
  }
};
}

using foo_json = nlohmann::basic_json<std::map, std::vector, std::string, bool, std::int64_t, 
std::uint64_t, double, std::allocator,ns::foo_serializer>;

int main(){
  foo_json lj=ns::foo{3};
  ns::foo ff=lj;
  std::cout<<lj<<std::endl;
  std::cout<<ff.x<<std::endl;
  nlohmann::json nj=lj;                  // This line works as expected
}

@theodelrieu
Copy link
Contributor

Great! By the way, I think you could simplify your serializer a bit:

template<typename T, typename = void>
struct foo_serializer {
  template <typename BasicJsonType>
  static void to_json(BasicJsonType& j, const T& value) {
    ::nlohmann::to_json(j, value);
  }
  template <typename BasicJsonType>
  static void from_json(const BasicJsonType& j, T& value) {
    ::nlohmann::from_json(j, value);
  }
};

This is roughly the implementation of the adl_serializer (without perfect forwarding).
Then you could add a full specialization on your type foo. That would remove some enable_if code, and allow you to add specializations for other types than foo in the future.

@yosagi
Copy link
Author

yosagi commented Mar 1, 2018

Thanks for a comment. I'm new to SFINAE and I could not write good example code here.
Also I'm not aware with rvalues. Thank you for mentioning about perfect forwarding.
The actual code I wrote in my project looks like this. (Bit modified just now for perfect forwarding)

using array_json =
    nlohmann::basic_json<std::map, std::vector, std::string, bool, std::int64_t,
                         std::uint64_t, double, std::allocator,
                         array_adl_serializer>;

template <typename T>
struct array_adl_serializer<T, typename std::enable_if<!yos::has_to_json_array<
                                   T, array_json>::value>::type> {
  template <typename BasicJsonType, typename ValueType>
  static void from_json(BasicJsonType&& j, ValueType& t) {
    ::nlohmann::from_json(std::forward<BasicJsonType>(j), t);
  }
  template <typename BasicJsonType, typename ValueType>
  static void to_json(BasicJsonType& j, ValueType&& t) {
    ::nlohmann::to_json(j, std::forward<ValueType>(t));
  }
};
template <typename T>
struct array_adl_serializer<T, typename std::enable_if<yos::has_to_json_array<
                                   T, array_json>::value>::type> {
  template <typename BasicJsonType, typename ValueType>
  static void from_json(BasicJsonType&& j, ValueType& t) {
    t.from_json(std::forward<BasicJsonType>(j));
  }
  template <typename BasicJsonType, typename ValueType>
  static void to_json(BasicJsonType& j, ValueType&& t) {
    // forward rvalue
    std::forward<ValueType>(t).to_json_array(j);
  }
};

The array_adl_serializer look for T::to_json_array and use it.
Full code is available here: https://github.com/furo-org/jsonutil

@theodelrieu
Copy link
Contributor

Looks good, I would still only change the first partial specialization to be the default implementation:

template <typename, typename>
struct array_adl_serializer {
  template <typename BasicJsonType, typename ValueType>
  static void from_json(BasicJsonType&& j, ValueType& t) {
    ::nlohmann::from_json(std::forward<BasicJsonType>(j), t);
  }
  template <typename BasicJsonType, typename ValueType>
  static void to_json(BasicJsonType& j, ValueType&& t) {
    ::nlohmann::to_json(j, std::forward<ValueType>(t));
  }
};

Then, you can keep the second partial specialization as is :)

@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Mar 6, 2018
@nlohmann nlohmann added this to the Release 3.1.2 milestone Mar 6, 2018
@nlohmann nlohmann self-assigned this Mar 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution: duplicate the issue is a duplicate; refer to the linked issue instead solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants