-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
Add a customization point for user-defined types #328
Comments
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. |
I have not understood the role of |
It's all static functions, the class is never instantiated. |
I still do not understand what to add to the |
Maybe something like this:
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.) |
I started a POC on my fork. (I didn't modify the re2c file, but I will !) |
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 |
What is the advantage of |
@gnzlbg one thing that comes to mind is the ability to partially specialize the struct We could imagine a specialization for template <typename T>
struct json_traits<boost::optional<T>>
{
// ...
}; |
What's wrong with just using overloading? template <typename T>
auto json_convert(boost::optional<T> const&) -> basic_json { ... } (note: 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 template <typename Optional, CONCEPT_REQUIRES_(OptionalLike<Optional>{})>
auto json_convert(Optional const&) -> basic_json { ... } (note: since EDIT: Doing the same with partial template specialization is a bit "more hacky" since it would require not only |
Free functions are definitely easier I agree. I'll look into that! |
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 ( |
I remember this paper, I'll read it more closely, thanks for your input! |
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:
That way the user can easily specialize with SFINAE when needed, i.e.,
|
Hi, I do not know if I understand correctly, but maybe this main.cpp file will be helpful. |
Template argument deduction? No need to do anything fancy here: template<typename T, typename... Args>
void to_json(T const&, basic_json<Args...>& j) {
...
}
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 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
}
};
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
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 Anyhow I agree that having a robust way of allowing multiple, different, serializations, of a particular type, to a |
I also would wish that the "Serializer" type that is passed to the 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 |
Hi, I've committed a first version with a 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. |
Range-v3 implementation does not require variable templates (see On Monday, 17 October 2016, Théo DELRIEU [email protected] wrote:
|
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 |
Hi, I've implemented the 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 EDIT: Should I start a PR now? This would make it easier to review/comment. |
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. |
Some things:
That is, to serialize something in 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 |
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! |
@theodelrieu Instead of |
As @d-frey says, 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 So what @theodelrieu actually wants is typically called 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 If you want the implementation of the "default" 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 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. |
This feature is implemented, see #435. |
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:
There could also be a conversion method when
get
is calledThese 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?
The text was updated successfully, but these errors were encountered: