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

Jump to bottom

Add support for deserialization of STL containers of non-default constructable types (fixes #2574). #2576

Merged
merged 18 commits into from
Apr 25, 2021
Merged

Add support for deserialization of STL containers of non-default constructable types (fixes #2574). #2576

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
1e825e4
Add support for deserialization of STL containers of non-default cons…
AnthonyVH Jan 8, 2021
c0a8b45
Renamed template parameter and added some comments.
AnthonyVH Jan 9, 2021
1b113f7
Added extra tests to improve coverage.
AnthonyVH Jan 9, 2021
23f462b
Reduced code duplication, renamed tag to identity_tag.
AnthonyVH Jan 10, 2021
672e8bf
Fixed std::pair trying to deserialize via array functions.
AnthonyVH Jan 11, 2021
6ebf274
Add internal version of is_default_constructible to work around LWG 2…
AnthonyVH Jan 12, 2021
6278f31
Simplify from_json overloads.
AnthonyVH Jan 12, 2021
6ef1614
Refactor json::get() to use priority_tag.
AnthonyVH Jan 12, 2021
fc8c584
Regenerated single include.
AnthonyVH Jan 12, 2021
fbf6df6
Enable member function calls in trailing return decltype expressions …
AnthonyVH Jan 12, 2021
d7c0f15
Merged from_json for pair and tuple to try to fix C2995 error in old …
AnthonyVH Jan 13, 2021
6eb37e9
Only add conditional constexpr to get() for >= C++14 to work around e…
AnthonyVH Jan 14, 2021
848927a
Updated comments as requested.
AnthonyVH Jan 23, 2021
130382f
Remove comment about GCC commit which didn't really relate to the code.
AnthonyVH Jan 24, 2021
333612c
Merge remote-tracking branch 'upstream/develop' into non_default_cons…
AnthonyVH Mar 24, 2021
322bc99
Reran amalgamate.
AnthonyVH Mar 24, 2021
8e79917
Fix clang-tidy complaints.
AnthonyVH Mar 24, 2021
2b86513
Fixing CI errors.
AnthonyVH Mar 24, 2021
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
42 changes: 33 additions & 9 deletions include/nlohmann/adl_serializer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
#pragma once

#include <type_traits>
#include <utility>

#include <nlohmann/detail/conversions/from_json.hpp>
#include <nlohmann/detail/conversions/to_json.hpp>
#include <nlohmann/detail/meta/identity_tag.hpp>
#include <nlohmann/detail/meta/type_traits.hpp>

namespace nlohmann
{

template<typename, typename>
template<typename ValueType, typename>
struct adl_serializer
{
/*!
Expand All @@ -17,17 +20,39 @@ struct adl_serializer
This function is usually called by the `get()` function of the
@ref basic_json class (either explicit or via conversion operators).

@note This function is chosen for value types which can be default constructed.
AnthonyVH marked this conversation as resolved.
Show resolved Hide resolved

@param[in] j JSON value to read from
@param[in,out] val value to write to
*/
template<typename BasicJsonType, typename ValueType>
static auto from_json(BasicJsonType&& j, ValueType& val) noexcept(
template<typename BasicJsonType, typename TargetType = ValueType>
static auto from_json(BasicJsonType && j, TargetType& val) noexcept(
noexcept(::nlohmann::from_json(std::forward<BasicJsonType>(j), val)))
-> decltype(::nlohmann::from_json(std::forward<BasicJsonType>(j), val), void())
{
::nlohmann::from_json(std::forward<BasicJsonType>(j), val);
}

/*!
@brief convert a JSON value to any value type

This function is usually called by the `get()` function of the
@ref basic_json class (either explicit or via conversion operators).

@note This function is chosen for value types which can not be default constructed.
AnthonyVH marked this conversation as resolved.
Show resolved Hide resolved

@param[in] j JSON value to read from

@return copy of the JSON value, converted to @a ValueType
*/
template<typename BasicJsonType, typename TargetType = ValueType>
static auto from_json(BasicJsonType && j) noexcept(
noexcept(::nlohmann::from_json(std::forward<BasicJsonType>(j), detail::identity_tag<TargetType> {})))
-> decltype(::nlohmann::from_json(std::forward<BasicJsonType>(j), detail::identity_tag<TargetType> {}))
{
return ::nlohmann::from_json(std::forward<BasicJsonType>(j), detail::identity_tag<TargetType> {});
}

/*!
@brief convert any value type to a JSON value

Expand All @@ -37,13 +62,12 @@ struct adl_serializer
@param[in,out] j JSON value to write to
@param[in] val value to read from
*/
template<typename BasicJsonType, typename ValueType>
static auto to_json(BasicJsonType& j, ValueType&& val) noexcept(
noexcept(::nlohmann::to_json(j, std::forward<ValueType>(val))))
-> decltype(::nlohmann::to_json(j, std::forward<ValueType>(val)), void())
template<typename BasicJsonType, typename TargetType = ValueType>
static auto to_json(BasicJsonType& j, TargetType && val) noexcept(
noexcept(::nlohmann::to_json(j, std::forward<TargetType>(val))))
-> decltype(::nlohmann::to_json(j, std::forward<TargetType>(val)), void())
{
::nlohmann::to_json(j, std::forward<ValueType>(val));
::nlohmann::to_json(j, std::forward<TargetType>(val));
}
};

} // namespace nlohmann
79 changes: 66 additions & 13 deletions include/nlohmann/detail/conversions/from_json.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <nlohmann/detail/exceptions.hpp>
#include <nlohmann/detail/macro_scope.hpp>
#include <nlohmann/detail/meta/cpp_future.hpp>
#include <nlohmann/detail/meta/identity_tag.hpp>
#include <nlohmann/detail/meta/type_traits.hpp>
#include <nlohmann/detail/value_t.hpp>

Expand Down Expand Up @@ -187,7 +188,10 @@ auto from_json_array_impl(const BasicJsonType& j, std::array<T, N>& arr,
}
}

template<typename BasicJsonType, typename ConstructibleArrayType>
template<typename BasicJsonType, typename ConstructibleArrayType,
enable_if_t<
std::is_assignable<ConstructibleArrayType&, ConstructibleArrayType>::value,
int> = 0>
auto from_json_array_impl(const BasicJsonType& j, ConstructibleArrayType& arr, priority_tag<1> /*unused*/)
-> decltype(
arr.reserve(std::declval<typename ConstructibleArrayType::size_type>()),
Expand All @@ -208,7 +212,10 @@ auto from_json_array_impl(const BasicJsonType& j, ConstructibleArrayType& arr, p
arr = std::move(ret);
}

template<typename BasicJsonType, typename ConstructibleArrayType>
template<typename BasicJsonType, typename ConstructibleArrayType,
enable_if_t<
std::is_assignable<ConstructibleArrayType&, ConstructibleArrayType>::value,
int> = 0>
void from_json_array_impl(const BasicJsonType& j, ConstructibleArrayType& arr,
priority_tag<0> /*unused*/)
{
Expand Down Expand Up @@ -248,6 +255,26 @@ void())
from_json_array_impl(j, arr, priority_tag<3> {});
}

template < typename BasicJsonType, typename T, std::size_t... Idx >
std::array<T, sizeof...(Idx)> from_json_inplace_array_impl(BasicJsonType&& j,
identity_tag<std::array<T, sizeof...(Idx)>> /*unused*/, index_sequence<Idx...> /*unused*/)
{
return { std::forward<BasicJsonType>(j).at(Idx).template get<T>()... };
}

template < typename BasicJsonType, typename T, std::size_t N >
auto from_json(BasicJsonType&& j, identity_tag<std::array<T, N>> tag)
-> decltype(from_json_inplace_array_impl(std::forward<BasicJsonType>(j), tag, make_index_sequence<N> {}))
{
if (JSON_HEDLEY_UNLIKELY(!j.is_array()))
{
JSON_THROW(type_error::create(302, "type must be array, but is " +
std::string(j.type_name())));
}

return from_json_inplace_array_impl(std::forward<BasicJsonType>(j), tag, make_index_sequence<N> {});
}

template<typename BasicJsonType>
void from_json(const BasicJsonType& j, typename BasicJsonType::binary_t& bin)
{
Expand Down Expand Up @@ -323,22 +350,48 @@ void from_json(const BasicJsonType& j, ArithmeticType& val)
}
}

template<typename BasicJsonType, typename... Args, std::size_t... Idx>
std::tuple<Args...> from_json_tuple_impl_base(BasicJsonType&& j, index_sequence<Idx...> /*unused*/)
{
return std::make_tuple(std::forward<BasicJsonType>(j).at(Idx).template get<Args>()...);
}

template < typename BasicJsonType, class A1, class A2 >
std::pair<A1, A2> from_json_tuple_impl(BasicJsonType&& j, identity_tag<std::pair<A1, A2>> /*unused*/, priority_tag<0> /*unused*/)
{
return {std::forward<BasicJsonType>(j).at(0).template get<A1>(),
std::forward<BasicJsonType>(j).at(1).template get<A2>()};
}

template<typename BasicJsonType, typename A1, typename A2>
void from_json(const BasicJsonType& j, std::pair<A1, A2>& p)
void from_json_tuple_impl(BasicJsonType&& j, std::pair<A1, A2>& p, priority_tag<1> /*unused*/)
{
p = {j.at(0).template get<A1>(), j.at(1).template get<A2>()};
p = from_json_tuple_impl(std::forward<BasicJsonType>(j), identity_tag<std::pair<A1, A2>> {}, priority_tag<0> {});
}

template<typename BasicJsonType, typename Tuple, std::size_t... Idx>
void from_json_tuple_impl(const BasicJsonType& j, Tuple& t, index_sequence<Idx...> /*unused*/)
template<typename BasicJsonType, typename... Args>
std::tuple<Args...> from_json_tuple_impl(BasicJsonType&& j, identity_tag<std::tuple<Args...>> /*unused*/, priority_tag<2> /*unused*/)
{
t = std::make_tuple(j.at(Idx).template get<typename std::tuple_element<Idx, Tuple>::type>()...);
return from_json_tuple_impl_base<BasicJsonType, Args...>(std::forward<BasicJsonType>(j), index_sequence_for<Args...> {});
}

template<typename BasicJsonType, typename... Args>
void from_json(const BasicJsonType& j, std::tuple<Args...>& t)
void from_json_tuple_impl(BasicJsonType&& j, std::tuple<Args...>& t, priority_tag<3> /*unused*/)
{
t = from_json_tuple_impl_base<BasicJsonType, Args...>(std::forward<BasicJsonType>(j), index_sequence_for<Args...> {});
}

template<typename BasicJsonType, typename TupleRelated>
auto from_json(BasicJsonType&& j, TupleRelated&& t)
-> decltype(from_json_tuple_impl(std::forward<BasicJsonType>(j), std::forward<TupleRelated>(t), priority_tag<3> {}))
{
from_json_tuple_impl(j, t, index_sequence_for<Args...> {});
if (JSON_HEDLEY_UNLIKELY(!j.is_array()))
{
JSON_THROW(type_error::create(302, "type must be array, but is " +
std::string(j.type_name())));
}

return from_json_tuple_impl(std::forward<BasicJsonType>(j), std::forward<TupleRelated>(t), priority_tag<3> {});
}

template < typename BasicJsonType, typename Key, typename Value, typename Compare, typename Allocator,
Expand Down Expand Up @@ -384,11 +437,11 @@ void from_json(const BasicJsonType& j, std::unordered_map<Key, Value, Hash, KeyE
struct from_json_fn
{
template<typename BasicJsonType, typename T>
auto operator()(const BasicJsonType& j, T& val) const
noexcept(noexcept(from_json(j, val)))
-> decltype(from_json(j, val), void())
auto operator()(const BasicJsonType& j, T&& val) const
noexcept(noexcept(from_json(j, std::forward<T>(val))))
-> decltype(from_json(j, std::forward<T>(val)))
{
return from_json(j, val);
return from_json(j, std::forward<T>(val));
}
AnthonyVH marked this conversation as resolved.
Show resolved Hide resolved
};
} // namespace detail
Expand Down
10 changes: 10 additions & 0 deletions include/nlohmann/detail/meta/identity_tag.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#pragma once

namespace nlohmann
{
namespace detail
{
// dispatching helper struct
template <class T> struct identity_tag {};
} // namespace detail
} // namespace nlohmann
76 changes: 57 additions & 19 deletions include/nlohmann/detail/meta/type_traits.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,7 @@ struct is_getable
};

template<typename BasicJsonType, typename T>
struct has_from_json < BasicJsonType, T,
enable_if_t < !is_basic_json<T>::value >>
struct has_from_json < BasicJsonType, T, enable_if_t < !is_basic_json<T>::value >>
{
using serializer = typename BasicJsonType::template json_serializer<T, void>;

Expand Down Expand Up @@ -150,6 +149,52 @@ struct has_to_json < BasicJsonType, T, enable_if_t < !is_basic_json<T>::value >>
///////////////////
// is_ functions //
///////////////////
// https://en.cppreference.com/w/cpp/types/conjunction
AnthonyVH marked this conversation as resolved.
Show resolved Hide resolved
template<class...> struct conjunction : std::true_type { };
template<class B1> struct conjunction<B1> : B1 { };
template<class B1, class... Bn>
struct conjunction<B1, Bn...>
: std::conditional<bool(B1::value), conjunction<Bn...>, B1>::type {};

// Reimplementation of is_constructible and is_default_constructible, due to them being broken for
// std::pair and std::tuple until LWG 2367 fix (see https://cplusplus.github.io/LWG/lwg-defects.html#2367).
// This causes compile errors in e.g. clang 3.5 or gcc 4.9.
// Based on commit fixing this in gcc: https://github.com/gcc-mirror/gcc/commit/d3c64041b32b6962ad6b2d879231537a477631fb
Copy link
Owner

Choose a reason for hiding this comment

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

Can you elaborate whether you took code out of that commit? I'm asking because of licensing issues (GPLv2 vs. MIT).

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 looked at the code to see what the "proper" fix looked like. However, that requires modifying std::pair/std::tuple. The code I added is more or less the same as the suggested one in the LWG defect report. Well, the logic is the same, it's not the exact same code. So I don't think there's any issue here.

It's also this report that mentions that this fix is incomplete:

The proposed resolution is incomplete, because it wouldn't work for cv-qualified objects of pair or for references of them during reference-initialization.

I don't think this is an issue, since detail::is_constructible is never called for e.g. std::pair<A, B> volatile nor for a reference to it. I did not add extra specializations to detail::is_constructible for volatile types or references. Currently only std::pair<A, B> and std::pair<A, B> const are specialized. Actually, I don't think that second one will ever be used. Do you prefer I add specializations for volatile, const volatile and references to all specialized types anyway (i.e. 6 extra specializations for std::pair and std::tuple each)?

Copy link
Owner

Choose a reason for hiding this comment

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

This is not what I meant. I was asking whether the code following that comment was written by yourself (and could be MIT licensed in this project) or taken from a different source - e.g., the GCC source code which would be GPLv2 licensed.

Copy link
Contributor Author

@AnthonyVH AnthonyVH Jan 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

No no, as I mentioned the code I added uses the same logic as the suggested solution mentioned in the LWG defect report. No code was copied from anywhere.

Maybe I should just remove the mention to the GCC commit?

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, please. Comments like this make FOSS auditors anxious...

Copy link
Contributor Author

@AnthonyVH AnthonyVH Jan 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Something to keep in mind next time 😄. I've removed the sentence and just pushed it. I guess that's the last required change then?

template <typename T>
struct is_default_constructible : std::is_default_constructible<T> {};

template <typename T1, typename T2>
struct is_default_constructible<std::pair<T1, T2>>
: conjunction<is_default_constructible<T1>, is_default_constructible<T2>> {};

template <typename T1, typename T2>
struct is_default_constructible<const std::pair<T1, T2>>
: conjunction<is_default_constructible<T1>, is_default_constructible<T2>> {};

template <typename... Ts>
struct is_default_constructible<std::tuple<Ts...>>
: conjunction<is_default_constructible<Ts>...> {};

template <typename... Ts>
struct is_default_constructible<const std::tuple<Ts...>>
: conjunction<is_default_constructible<Ts>...> {};


template <typename T, typename... Args>
struct is_constructible : std::is_constructible<T, Args...> {};

template <typename T1, typename T2>
struct is_constructible<std::pair<T1, T2>> : is_default_constructible<std::pair<T1, T2>> {};

template <typename T1, typename T2>
struct is_constructible<const std::pair<T1, T2>> : is_default_constructible<const std::pair<T1, T2>> {};

template <typename... Ts>
struct is_constructible<std::tuple<Ts...>> : is_default_constructible<std::tuple<Ts...>> {};

template <typename... Ts>
struct is_constructible<const std::tuple<Ts...>> : is_default_constructible<const std::tuple<Ts...>> {};


template<typename T, typename = void>
struct is_iterator_traits : std::false_type {};
Expand Down Expand Up @@ -193,9 +238,9 @@ struct is_compatible_object_type_impl <

// macOS's is_constructible does not play well with nonesuch...
static constexpr bool value =
std::is_constructible<typename object_t::key_type,
is_constructible<typename object_t::key_type,
typename CompatibleObjectType::key_type>::value &&
std::is_constructible<typename object_t::mapped_type,
is_constructible<typename object_t::mapped_type,
typename CompatibleObjectType::mapped_type>::value;
};

Expand All @@ -216,10 +261,10 @@ struct is_constructible_object_type_impl <
using object_t = typename BasicJsonType::object_t;

static constexpr bool value =
(std::is_default_constructible<ConstructibleObjectType>::value &&
(is_default_constructible<ConstructibleObjectType>::value &&
(std::is_move_assignable<ConstructibleObjectType>::value ||
std::is_copy_assignable<ConstructibleObjectType>::value) &&
(std::is_constructible<typename ConstructibleObjectType::key_type,
(is_constructible<typename ConstructibleObjectType::key_type,
typename object_t::key_type>::value &&
std::is_same <
typename object_t::mapped_type,
Expand Down Expand Up @@ -247,7 +292,7 @@ struct is_compatible_string_type_impl <
value_type_t, CompatibleStringType>::value >>
{
static constexpr auto value =
std::is_constructible<typename BasicJsonType::string_t, CompatibleStringType>::value;
is_constructible<typename BasicJsonType::string_t, CompatibleStringType>::value;
};

template<typename BasicJsonType, typename ConstructibleStringType>
Expand All @@ -265,7 +310,7 @@ struct is_constructible_string_type_impl <
value_type_t, ConstructibleStringType>::value >>
{
static constexpr auto value =
std::is_constructible<ConstructibleStringType,
is_constructible<ConstructibleStringType,
typename BasicJsonType::string_t>::value;
};

Expand All @@ -288,7 +333,7 @@ struct is_compatible_array_type_impl <
iterator_traits<CompatibleArrayType >>::value >>
{
static constexpr bool value =
std::is_constructible<BasicJsonType,
is_constructible<BasicJsonType,
typename CompatibleArrayType::value_type>::value;
};

Expand All @@ -311,7 +356,7 @@ struct is_constructible_array_type_impl <
BasicJsonType, ConstructibleArrayType,
enable_if_t < !std::is_same<ConstructibleArrayType,
typename BasicJsonType::value_type>::value&&
std::is_default_constructible<ConstructibleArrayType>::value&&
is_default_constructible<ConstructibleArrayType>::value&&
(std::is_move_assignable<ConstructibleArrayType>::value ||
std::is_copy_assignable<ConstructibleArrayType>::value)&&
is_detected<value_type_t, ConstructibleArrayType>::value&&
Expand Down Expand Up @@ -355,7 +400,7 @@ struct is_compatible_integer_type_impl <
using CompatibleLimits = std::numeric_limits<CompatibleNumberIntegerType>;

static constexpr auto value =
std::is_constructible<RealIntegerType,
is_constructible<RealIntegerType,
CompatibleNumberIntegerType>::value &&
CompatibleLimits::is_integer &&
RealLimits::is_signed == CompatibleLimits::is_signed;
Expand All @@ -382,17 +427,10 @@ template<typename BasicJsonType, typename CompatibleType>
struct is_compatible_type
: is_compatible_type_impl<BasicJsonType, CompatibleType> {};

// https://en.cppreference.com/w/cpp/types/conjunction
template<class...> struct conjunction : std::true_type { };
template<class B1> struct conjunction<B1> : B1 { };
template<class B1, class... Bn>
struct conjunction<B1, Bn...>
: std::conditional<bool(B1::value), conjunction<Bn...>, B1>::type {};

template<typename T1, typename T2>
struct is_constructible_tuple : std::false_type {};

template<typename T1, typename... Args>
struct is_constructible_tuple<T1, std::tuple<Args...>> : conjunction<std::is_constructible<T1, Args>...> {};
struct is_constructible_tuple<T1, std::tuple<Args...>> : conjunction<is_constructible<T1, Args>...> {};
} // namespace detail
} // namespace nlohmann
Loading