nlohmann / json

JSON for Modern C++
https://json.nlohmann.me
MIT License
42.76k stars 6.7k forks source link

Add a customization point for user-defined types #328

Closed theodelrieu closed 7 years ago

theodelrieu commented 8 years ago

Hi, I think it would be a cool addition to the library if there was a way to specify a conversion method, from a type to basic_json, which would be called by the lib.

Here a sample code of the behaviour I'm thinking about:

// library code
template <typename UserDefinedType>
struct json_traits;

// user code
template <>
struct json_traits<MyType>
{
  static basic_json convert(MyType const& val)
  {
    return {{"first_row", val.first_row}, {"some_value", 42}};
  }
};

There could also be a conversion method when get is called

auto obj = json::parse(buffer);
auto val = obj["user_defined_type_value"].get<MyType>();

These are just thoughts for now, but that would remove a LOT of boilerplate in my code. Plus, this would add some compile-time checks (if the struct is not defined for example) What's your opinion on that?

gregmarr commented 8 years ago

That would be cool. I created to_json and from_json functions for a lot of my types, and use to_json/from_json exclusively even for types it already supports. Making them work automatically out of the box would be even cooler.

nlohmann commented 8 years ago

This issue is related to #298 and #280.

nlohmann commented 8 years ago

I have not understood the role of json_traits completely. Shouldn't there be a constructor in the basic_json class for the json_traits template? Would this constructor then be usable for any class that is specialized by json_traits?

gregmarr commented 8 years ago

It's all static functions, the class is never instantiated.

nlohmann commented 8 years ago

I still do not understand what to add to the basic_json class.

gregmarr commented 8 years ago

Maybe something like this:

template<typename T> basic_json(T&& val) :
    basic_json(json_traits<T>::convert(std::forward<T>(val)))
{
    assert_invariant();
}

There might also need to be some enable_if code to keep the current constructors working, or maybe the current constructors become specializations of the json_traits class instead. (Though that using specializations part might get into cyclical dependencies.)

gregmarr commented 8 years ago

Yeah, this is basically what @gnzlbg and @d-frey described in #298

theodelrieu commented 8 years ago

Indeed, looking at taocpp value.hh and traits.hh can help to understand the idea

theodelrieu commented 8 years ago

I started a POC on my fork. (I didn't modify the re2c file, but I will !)

theodelrieu commented 8 years ago

There is an issue with that ctor however...

// not equality-comparable
struct S{};

int main()
{
// this will call operator==(basic_json, basic_json) if json_traits<S> was specialized
// and it will do so for every operator defined for basic_json ...
  S() == S();
}

I'm looking for a way to bypass this, without making the constructor explicit

gnzlbg commented 8 years ago

(json_traits::convert(std::forward(val)))

What is the advantage of json_traits<T>::convert over a simpler json_convert free function that is found via ADL ?

theodelrieu commented 8 years ago

@gnzlbg one thing that comes to mind is the ability to partially specialize the struct

We could imagine a specialization for boost::optional for example


template <typename T>
struct json_traits<boost::optional<T>>
{
   // ...
};
gnzlbg commented 8 years ago

What's wrong with just using overloading?

template <typename T>
auto json_convert(boost::optional<T> const&) -> basic_json { ... }

(note: json could include these overloads in some separate header, but they would add a dependency on boost::optional).

I find that for most people free functions and overloading are easier to understand than trait classes and partial template specialization. One thing overloading allows is to implement json_convert once for all Optional-like types (given an OptionalLike concept that requires .value() and explicit operator bool) using enable_if (behind a CONCEPT_REQUIRES_ macro like in, e.g., range-v3):

template <typename Optional, CONCEPT_REQUIRES_(OptionalLike<Optional>{})>
auto json_convert(Optional const&) -> basic_json { ... }

(note: since boost/std/...::optional is not directly used here, json could actually add this overload without introducing a dependency on any of these libraries).

EDIT: Doing the same with partial template specialization is a bit "more hacky" since it would require not only requires and an OptionalLike concept but also void_t (which while simple to write, is a bit harder to understand why it works).

theodelrieu commented 8 years ago

Free functions are definitely easier I agree. I'll look into that!

gnzlbg commented 8 years ago

This comment provides a rough idea about how to implement it to make it as easy as possible for users to serialize their own types. In a nutshell it just proposed adding two customization points (to_json/from_json) to the library in the same way that range-v3 does, and then add some default implementations of these for common types (like std::array, std::pair, std::tuple, ranges, ...).

theodelrieu commented 8 years ago

I remember this paper, I'll read it more closely, thanks for your input!

d-frey commented 8 years ago

I have 10+ years of experience with using traits classes in multiple areas. I'd go for a traits class anytime over a free function. Some reasons:

One final remark for now: Make sure you declare the traits class with an additional parameter defaulted to void:

template< typename T, typename = void >
struct traits;

That way the user can easily specialize with SFINAE when needed, i.e.,

template<typename E>
struct traits<E, std::enable_if_t<std::is_enum_v<E>>>
{ ... };
frenzisys commented 8 years ago

Hi, I do not know if I understand correctly, but maybe this main.cpp file will be helpful. you just declare a MAKE_JSON(name_struct, member1, etc...) inside your struct/class test_json.zip

gnzlbg commented 8 years ago

Where do you get the template parameters from for the return type if you are using a free function?

Template argument deduction? No need to do anything fancy here:

template<typename T, typename... Args>
void to_json(T const&, basic_json<Args...>& j) {
  ...
}

Using a traits class allows you to control the conversion between different areas of your code. basic_json is different from basic_json (as I basically do it in taocpp/json) and hence you can explicitly control how values are converted between the two areas. They don't mix by accident.

I agree that this functionality is useful, but it comes with some big costs that we should be aware of:

While the first concern might seem more serious, I actually like this library because it is very ergonomic to use, while giving you the power to do something else if you want to.

I think we could find a solution that let's you do both, by having one of the template parameters of basic_json be a trait that lets you fully control what can be serialized from/to different json object types and how, but at the same time giving it a reasonable default type argument that uses ADL for those cases/programs in which you don't care such that everything "just works".

For example, something like:

template <..., typename <class, class> Serializer = JSONDefaultSerializer> 
class basic_json { ... }

template<class /*unused*/, class = void>
struct JSONDefaultSerializer {

  // the default serializer just relies on ADL
  // to/from_json free functions are the default ways / fallback 
  // to serializing something to json
  template<typename T, typename JSON>
  void to_json(T const& from, JSON& to) {
    json::to_json(from, to);  // uses ADL
   }
   template<typename JSON, typename T>
   void from_json(JSON const& from, T& to) { 
     json::from_json(from, to);
   }
};

That way if you want to write your own "JSONSerializer" trait to have full control you can:

template <typename T>
struct MySerializer<boost::optional<T>> {
  template <typename JSON>
  void to_json(boost::optional<T> const& from, JSON& to) {
    from? to << from.value() : to << "-";  // do something custom here
   }
   template <typename JSON>
   void from_json(JSON const& from, boost::optional<T>& to) { 
     json::from_json(from, to);  // but you still can use ADL here
   }
};

They don't mix by accident.

Yes but as mentioned above this comes at the cost of having different types for your json objects (and thus different ABI), and at the cost of having to put all your conversions into a single trait class that you pass to the basic_json type. With ADL "everything just works" by default. If I suddenly add new type that has the corresponding free functions I don't have to change anything in any of my basic_json types for it to work with them.

You can not "stack" the free functions. With a traits class, you can derive from it and specialize for some type

Of course you can:

template<typename JSON>
void float_to_json(float const& f, JSON& to) { ... }

template <typename JSON>
void int_to_json(int const& i, JSON& to) { ... }

template <typename T, typename JSON, CONCEPT_REQUIRES(is_float<T>{} or is_integral<T>{})>
void to_json(T const& t, JSON& to) {
  // c++17: selects the best overload
  std::overload(float_to_json, int_to_json)(t, to);
}

I mean you could also use SFIANE or tag dispatching, but doing this in C++17 with std::overload (or std::first_overload) is trivial.

Anyhow I agree that having a robust way of allowing multiple, different, serializations, of a particular type, to a basic_json object, in the same program (even if its header only) is something worth pursuing. I just don't want to complicate things more than necessary for users that do not need this, and I think it is possible to do so.

gnzlbg commented 8 years ago

I also would wish that the "Serializer" type that is passed to the basic_json object would be a concrete type and not a higher-kinded one. So that users cannot specialize (but still could reuse or inherit from) the default serializer. Doing what @d-frey wants should still be possible:

template <..., typename Serializer = JSONDefaultSerializer> 
class basic_json { ... }

struct MySerializer;

using my_json = basic_json<..., MySerializer>;

struct MySerializer {
  template<typename T>
  void to_json(T const& t, my_json& o) {
    my_trait<T>::convert(t, o);  // you can just use your current trait class here
  }
  // from_json is analogous
};

EDIT: this adds a bit of boiler plate for the case that you want to use a trait class, but it simplifies the type constructor of basic_json (no more template <class, class>) and in this case whether you want to use void_t or not is up to you (if a user doesn't need it it doesn't "pay" for it).

theodelrieu commented 8 years ago

Hi, I've committed a first version with a json_traits struct. I added a constructor, a get_impl overload, and a unit-constructors3.cpp file in the test suite.

I tried to implement the customization-point design proposed by Eric Niebler, unfortunately variable templates are not supported in VS 2015, so I decided to begin with the traits struct.

I'm sure I missed quite a lot of things, so I'm looking for your feedbacks.

gnzlbg commented 8 years ago

Range-v3 implementation does not require variable templates (see include/range/v3/utility/static_const.hpp).

On Monday, 17 October 2016, Théo DELRIEU notifications@github.com wrote:

Hi, I've committed a first version https://github.com/theodelrieu/json/commit/a2a8fec577eff7392f124570f3286d6a2e30f0b6 with a json_traits struct. I added a constructor, a get_impl overload, and a unit-constructors3.cpp file in the test suite.

I tried to implement the customization-point design proposed by Eric Niebler, unfortunately variable templates are not supported in VS 2015, so I decided to begin with the traits struct.

I'm sure I missed quite a lot of things, so I'm looking for your feedbacks.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nlohmann/json/issues/328#issuecomment-254174615, or mute the thread https://github.com/notifications/unsubscribe-auth/AA3Nppi9q2mgW4UExUy_4l8Q_HnYCbBrks5q01I_gaJpZM4KU7Jf .

theodelrieu commented 8 years ago

I see, I tried to use the example code from the paper, but the code you linked compiles on VS2015. I will try to implement that too

theodelrieu commented 8 years ago

Hi, I've implemented the from_json/to_json free function support on my fork.

I'd really appreciate all your feedbacks on the diff I linked. (especially you @gnzlbg, since you're more aware than me about the subject of ADL free-function style!).

I wrote comments questioning the design choices I've made in json.hpp. I also added some stuff that might be out of this feature's scope, that I should add in a different PR (e.g. alias templates)

EDIT: Should I start a PR now? This would make it easier to review/comment.

gnzlbg commented 8 years ago

I think it is easier to give feedback if you open a PR, just make it clear to @nlohmann that the work is not finished yet and that it should not be merged yet.

gnzlbg commented 8 years ago

Some things:

That is, to serialize something in basic_json you only use JSONSerialization::from/to and be done with it.

If the user doesn't care, ADL will trigger and do "the easy thing".

If the user does care like @d-frey they can write their own trait class that does whatever they want and pass it as a template parameter to, e.g., have different basic_json types that serialize the same objects to and from json in different ways.

theodelrieu commented 8 years ago

I opened a PR.

At first I found the template-argument design a bit annoying to people specializing the traits struct.

But it avoids some confusion, and lets people control serialization. I'll implement that and come back to you, thanks for the feedback!

d-frey commented 8 years ago

@theodelrieu Instead of std::remove_cv_t<std::remove_reference_t<...>> you probably want to use std::decay_t<...>. It does what remove_cv+remove_reference does and more, like taking care of turning const char(&)[N] into const char* - which is what you most likely want. It's almost as if it was made for this kind of use-case... ;)

gnzlbg commented 8 years ago

like taking care of turning const char(&)[N] into const char* - which is what you most likely want.

As @d-frey says, std::decay_t "decays" types like when passed by value to functions arguments (which does array to pointer conversion). However, where @theodelrieu uses remove_cv<remove_reference<T>> he actually doesn't want the arguments to decay like as when passed to functions because that would break both ADL and the trait system for serialization. An user (or probably, this library) can implement to_/from_json for C-arrays using ADL:

template <typename T, std::size_t N, typename JSON>
void to_json(T[N] const&, JSON&);

or by specializing a trait class:

template<typename T, std::size_t N> 
struct json_traits<T[N]> { ... };`

But if you "decay" the types before using ADL or the trait class, the following will be try to be picked instead:

template <typename T, typename JSON>
void to_json(T*, JSON&);

template<typename T> 
struct json_traits<T*> { ... };`

which will fail to pattern-match because T* != T[N] for the trait class, and because T* cannot be converted to T[N] for the free functions.

So what @theodelrieu actually wants is typically called uncvref_t (range-v3, STL2 which also has __uncvref, ...):

template <typename T>
using uncvref_t = std::remove_cv_t<std::remove_reference_t<T>>;

Sadly, this is not in the standard library yet, but while std::decay_t almost does what you want, it breaks in subtle ways. The only safe situation in which one should really use std::decay_t is when one actually really wants to perform decaying of values as when passed to functions as arguments. This happens, for example, when implementing something that calls functions, like std::bind, std::mem_fn, std::result_of, std::invoke... and such. For picking specializations in trait classes independently of reference and const/volatile qualifiers, uncvref_t is what you want.

If you want the implementation of the "default" json_traits to be 200% generic, you actually want to use std::remove_reference_t only, and specialize your trait class like e.g. libc++ does for tuple_size:

template <typename T> struct json_traits;
template <typename T> struct json_traits<const T> : json_traits<T> { ... }
template <typename T> struct json_traits<volatile T> : json_traits<T> { ... }
template <typename T> struct json_traits<const volatile T> : json_traits<T> { ... }

If a user just specializes json_traits for his own T, this will work with a cv qualified T as well. But if a user wants to have different types of json serialization for T than for const volatile T, this allows specializations for cv-qualified types as well. If you would use uncvref_t, cv-qualified specializations would never pattern-match successfully (because uncvref_t removes the cv qualifiers).

Buuuut... I don't think we should support different types of serialization depending on cv-qualifiers, at least not until somebody complains that they need them.

nlohmann commented 7 years ago

This feature is implemented, see #435.