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.

Sign up for GitHub

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

Feature/user defined types #338

Closed
wants to merge 14 commits into from
Closed

Feature/user defined types #338

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c136f67
add first version support for user-defined types
Oct 16, 2016
2ee7399
fix compilation on gcc-6
theodelrieu Oct 17, 2016
0001032
anonymous namespace renamed to detail
theodelrieu Oct 17, 2016
63241db
wip: add free function to_json/from_json support
theodelrieu Oct 17, 2016
f805dfc
wip: modify from_json prototype
Oct 18, 2016
733e95f
wip: add get_impl overload that uses free from_json method
Oct 18, 2016
06d60de
add alias templates to reduce boilerplate
theodelrieu Oct 18, 2016
2407833
revert useless formatting
Oct 20, 2016
db4806b
rename __static_const to _static_const (reserved identifier)
Oct 20, 2016
9d290c6
use uncvref_t<T> instead of remove_cv_t<remove_reference_t<T>>>
Oct 20, 2016
5451aa5
remove has_destructor and has_json_traits, use decltype instead
Oct 20, 2016
15507d1
move most SFINAE trickery in to/from_json_fn
Oct 21, 2016
56492ce
fix compilation on vs2015 (taken from MS-ranges)
Oct 22, 2016
b81c2a2
fix compilation on gcc 4.9
theodelrieu Oct 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 149 additions & 23 deletions src/json.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,34 @@ SOFTWARE.
*/
namespace nlohmann
{
// TODO add real documentation before PR

// Traits structure declaration, users can specialize it for their own types
//
// constructing a json object from a user-defined type will call the
// 'json to_json(T)' function
//
// whereas calling json::get<T> will call 'T from_json(json const&)'
template <typename T, typename = void>
struct json_traits;

// alias templates to reduce boilerplate
template <bool B, typename T = void>
using enable_if_t = typename std::enable_if<B, T>::type;

template <typename T>
using remove_cv_t = typename std::remove_cv<T>::type;

template <typename T>
using remove_reference_t = typename std::remove_reference<T>::type;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you probably want uncvref_t here as well.

// TODO update this doc
/*!
@brief unnamed namespace with internal helper functions
@since version 1.0.0
*/
namespace

namespace detail
{
/*!
@brief Helper to determine whether there's a key_type for T.
Expand All @@ -122,6 +143,44 @@ struct has_mapped_type
std::is_integral<decltype(detect(std::declval<T>()))>::value;
};

// taken from http://stackoverflow.com/questions/10711952/how-to-detect-existence-of-a-class-using-sfinae
// used to determine if json_traits is defined for a given type T
template <typename T>
struct has_destructor
{
template <typename U>
static std::true_type detect(decltype(std::declval<U>().~U())*);

template <typename>
static std::false_type detect(...);

static constexpr bool value = decltype(detect<T>(0))::value;
};

template<typename T>
struct has_json_traits
{
static constexpr bool value = has_destructor<json_traits<T>>::value;
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need all this?

I think we can have some member function like this in basic_json:

// Sketch

template <typename T>
auto from_impl(T const& t, long) -> decltype(do_something_without_trait(t));
template <typename T>
auto from_impl(T const& t, int) -> decltype(json_trait<T>::from_json(t, *this));
template <typename T>
auto from(T const& t) -> decltype(from_impl(t, 0));

Notice how from calls from_impl(t, (int)0), which will try to call the overload from_impl(T, int) (which is a better match that the overload with long). If that works, then that is the one that is called. But if in that overload json_trait<T>::from fails, e.g., because there is no specialization for T, that overload will be silently removed, and then the second overload from_impl(T, long) will kick in, because there is an implicit conversion from int to long.

So what that does is "try to call the int overload, and if that fails, fall back to the long overload". You can probably use this "idiom" to easily detect whether json_traits implement something that you need, and fall back to some other solution if it doesn't.

(If you want to check some examples that actually work, a good example is how begin and end are implemented in range-v3, or in STL2).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, the from_impl method aims to replace the two get_impl overloads that I added? I agree this is a lot more readable than what I have now.


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));
}
};

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);
}
};

/*!
@brief helper class to create locales with decimal point

Expand All @@ -144,6 +203,23 @@ struct DecimalSeparator : std::numpunct<char>

}

// taken from ranges-v3
// TODO add doc
template <typename T>
struct __static_const
{
static constexpr T value{};
};

template <typename T>
constexpr T __static_const<T>::value;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember that identifiers starting with an underscore followed by adjacent underscores are reserved for the implementation in any scope (not only the global one), that is, one cannot use them. But the rules for reserved identifiers were more complex than that (this is C++ after all), so might want to check if that is actually allowed in a non-global scope (we are inside a namespace here).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to cppreference : "the identifiers with a double underscore anywhere are reserved"

I'll rename it


inline namespace
{
constexpr auto const& to_json = __static_const<detail::to_json_fn>::value;
constexpr auto const& from_json = __static_const<detail::from_json_fn>::value;
}

/*!
@brief a class to store JSON values

Expand Down Expand Up @@ -1225,6 +1301,25 @@ class basic_json
assert_invariant();
}

// constructor chosen if json_traits is specialized for type T
// note: constructor is marked explicit to avoid the following issue:
//
// struct not_equality_comparable{};
//
// not_equality_comparable{} == not_equality_comparable{};
//
// 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
// 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 = enable_if_t<detail::has_json_traits<
remove_cv_t<remove_reference_t<T>>>::value>>
explicit basic_json(T &&val)
: basic_json(json_traits<remove_cv_t<remove_reference_t<T>>>::to_json(
std::forward<T>(val))) {}
/*!
@brief create a string (explicit)

Expand All @@ -1241,15 +1336,14 @@ class basic_json

@sa @ref basic_json(const typename string_t::value_type*) -- create a
string value from a character pointer
@sa @ref basic_json(const CompatibleStringType&) -- create a string value
@sa @ref basic_json(const CompatibleStringType&) -- create a string
value
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: you should probably revert all unnecessary formatting changes, since they make going through the relevant parts of the diffs harder

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, forgot to check that, my bad

from a compatible string container

@since version 1.0.0
*/
basic_json(const string_t& val)
: m_type(value_t::string), m_value(val)
{
assert_invariant();
basic_json(const string_t &val) : m_type(value_t::string), m_value(val) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intuition says that we would like to use the trait system for constructing basic_json objects from all types that are not basic_json, since in case somebody has a different kind of representation in a string (e.g. XML), they could still define their own trait class to construct basic_json objects from that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, although this would be a huge change to the current implementation, it might be out of the scope of this PR.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I think we probably don't want to do this for string and int, and other primitive types yet, I recall that there was a constructor for "ranges" (types with value_type, begin and end). I think it would be still a good idea to at least move this constructor out to check that ADL works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you talking about the constructor taking CompatibleArrayType& as a parameter?
Could you post a short pseudo-code about what you have in mind?

assert_invariant();
}

/*!
Expand Down Expand Up @@ -2201,7 +2295,7 @@ class basic_json
{
std::stringstream ss;
// fix locale problems
const static std::locale loc(std::locale(), new DecimalSeparator);
const static std::locale loc(std::locale(), new detail::DecimalSeparator);
ss.imbue(loc);

// 6, 15 or 16 digits of precision allows round-trip IEEE 754
Expand Down Expand Up @@ -2584,20 +2678,52 @@ 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);
template <typename T, typename = enable_if_t<detail::has_json_traits<
remove_cv_t<remove_reference_t<T>>>::value>>
auto get_impl(T *) const
-> decltype(json_traits<remove_cv_t<remove_reference_t<T>>>::from_json(
std::declval<basic_json>())) {
return json_traits<remove_cv_t<remove_reference_t<T>>>::from_json(*this);
}

// this one is quite atrocious
// 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(T *) const
-> enable_if_t<not detail::has_json_traits<remove_cv_t<T>>::value,
remove_cv_t<remove_reference_t<decltype(
::nlohmann::from_json(std::declval<basic_json>(),
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)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll think about this, you make good points here.

::nlohmann::from_json(*this, ret);
return ret;
}

/// get an object (explicit)
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>
T get_impl(T*) const
{
if (is_object())
{
return T(m_value.object->begin(), m_value.object->end());
}
else
{
throw std::domain_error("type must be object, but is " + type_name());
}
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>
T get_impl(T *) const {
if (is_object()) {
return T(m_value.object->begin(), m_value.object->end());
} else {
throw std::domain_error("type must be object, but is " + type_name());
}
}

/// get an object (explicit)
Expand All @@ -2619,7 +2745,7 @@ class basic_json
not std::is_same<basic_json_t, typename T::value_type>::value and
not std::is_arithmetic<T>::value and
not std::is_convertible<std::string, T>::value and
not has_mapped_type<T>::value, int>::type = 0>
not detail::has_mapped_type<T>::value, int>::type = 0>
T get_impl(T*) const
{
if (is_array())
Expand Down Expand Up @@ -2664,7 +2790,7 @@ class basic_json
/// get an array (explicit)
template<class T, typename std::enable_if<
std::is_same<basic_json, typename T::value_type>::value and
not has_mapped_type<T>::value, int>::type = 0>
not detail::has_mapped_type<T>::value, int>::type = 0>
T get_impl(T*) const
{
if (is_array())
Expand Down Expand Up @@ -5829,7 +5955,7 @@ class basic_json
o.width(0);

// fix locale problems
const auto old_locale = o.imbue(std::locale(std::locale(), new DecimalSeparator));
const auto old_locale = o.imbue(std::locale(std::locale(), new detail::DecimalSeparator));
// set precision

// 6, 15 or 16 digits of precision allows round-trip IEEE 754
Expand Down
1 change: 1 addition & 0 deletions test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ add_executable(${JSON_UNITTEST_TARGET_NAME}
"src/unit-concepts.cpp"
"src/unit-constructor1.cpp"
"src/unit-constructor2.cpp"
"src/unit-constructor3.cpp"
"src/unit-convenience.cpp"
"src/unit-conversions.cpp"
"src/unit-deserialization.cpp"
Expand Down
Loading