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

Added support for string_view in C++17 #1028

Merged
merged 11 commits into from
Jun 23, 2018
Merged

Added support for string_view in C++17 #1028

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8165707
basic_json now supports getting many type of strings
gracicot Mar 15, 2018
2a5506e
Amalgamated headers
gracicot Mar 16, 2018
53d8d57
Amalgamate single include
gracicot Mar 29, 2018
c78dbc3
Added test for conversion to string_view
gracicot Mar 29, 2018
aaee18c
Added test for string conversion with string_view
gracicot Apr 9, 2018
714c592
Disabled implicit conversion to string_view on MSVC 15.13 and older
gracicot Jun 1, 2018
4778c02
Set MSVC version from 1514 and older
gracicot Jun 1, 2018
e0e7fa3
Re-added external_constructor with string compatible types
gracicot Jun 4, 2018
5676a2a
Aligned template declaration
gracicot Jun 4, 2018
7acd90b
Fixed check for compatible string type
gracicot Jun 4, 2018
14e6278
Merge branch 'develop' of github.com:gracicot/json into develop
gracicot Jun 4, 2018
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
19 changes: 19 additions & 0 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 @@ -70,6 +70,25 @@ void from_json(const BasicJsonType& j, typename BasicJsonType::string_t& s)
s = *j.template get_ptr<const typename BasicJsonType::string_t*>();
}

template <
typename BasicJsonType, typename CompatibleStringType,
enable_if_t <
is_compatible_string_type<BasicJsonType, CompatibleStringType>::value and
not std::is_same<typename BasicJsonType::string_t,
CompatibleStringType>::value and
std::is_constructible <
BasicJsonType, typename CompatibleStringType::value_type >::value,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this check relevant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, it's not necessary. I also missed a check elsewhere, let me fix that.

int > = 0 >
void from_json(const BasicJsonType& j, CompatibleStringType& s)
{
if (JSON_UNLIKELY(not j.is_string()))
{
JSON_THROW(type_error::create(302, "type must be string, but is " + std::string(j.type_name())));
}

s = *j.template get_ptr<const typename BasicJsonType::string_t*>();
}

template<typename BasicJsonType>
void from_json(const BasicJsonType& j, typename BasicJsonType::number_float_t& val)
{
Expand Down
10 changes: 10 additions & 0 deletions include/nlohmann/detail/conversions/to_json.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,16 @@ struct external_constructor<value_t::string>
j.m_value = std::move(s);
j.assert_invariant();
}

template<typename BasicJsonType, typename CompatibleStringType,
enable_if_t<not std::is_same<CompatibleStringType, typename BasicJsonType::string_t>::value,
int> = 0>
Copy link
Contributor

Choose a reason for hiding this comment

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

This SFINAE check is already done in the to_json method, maybe we can only do it once:

  • Adding a to_json(const BasicJson::string_t&) overload (only BasicJson::string_t&& is handled right now)
  • Then, in the to_json overload where the std::is_constructible<...> check is performed, construct a string_t and move it to the external_constructor.

Additionally, I'd rather keep external_constructor function count low (Maybe the array_t specialization could get cleaned a bit).

This is mostly a style preference though.

static void construct(BasicJsonType& j, const CompatibleStringType& str)
{
j.m_type = value_t::string;
j.m_value.string = j.template create<typename BasicJsonType::string_t>(str);
j.assert_invariant();
}
};

template<>
Expand Down
19 changes: 19 additions & 0 deletions include/nlohmann/detail/meta.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,16 @@ struct is_compatible_object_type_impl<true, RealType, CompatibleObjectType>
std::is_constructible<typename RealType::mapped_type, typename CompatibleObjectType::mapped_type>::value;
};

template<bool B, class RealType, class CompatibleStringType>
struct is_compatible_string_type_impl : std::false_type {};

template<class RealType, class CompatibleStringType>
struct is_compatible_string_type_impl<true, RealType, CompatibleStringType>
{
static constexpr auto value =
std::is_same<typename RealType::value_type, typename CompatibleStringType::value_type>::value;
};

template<class BasicJsonType, class CompatibleObjectType>
struct is_compatible_object_type
{
Expand All @@ -130,6 +140,15 @@ struct is_compatible_object_type
typename BasicJsonType::object_t, CompatibleObjectType >::value;
};

template<class BasicJsonType, class CompatibleStringType>
struct is_compatible_string_type
{
static auto constexpr value = is_compatible_string_type_impl <
conjunction<negation<std::is_same<void, CompatibleStringType>>,
has_value_type<CompatibleStringType>>::value,
typename BasicJsonType::string_t, CompatibleStringType >::value;
};

template<typename BasicJsonType, typename T>
struct is_basic_json_nested_type
{
Expand Down
4 changes: 2 additions & 2 deletions include/nlohmann/json.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2821,9 +2821,9 @@ class basic_json
not detail::is_basic_json<ValueType>::value
#ifndef _MSC_VER // fix for issue #167 operator<< ambiguity under VS2015
and not std::is_same<ValueType, std::initializer_list<typename string_t::value_type>>::value
#endif
#if defined(JSON_HAS_CPP_17)
#if defined(JSON_HAS_CPP_17) && _MSC_VER <= 1914
and not std::is_same<ValueType, typename std::string_view>::value
#endif
#endif
, int >::type = 0 >
operator ValueType() const
Expand Down
52 changes: 50 additions & 2 deletions single_include/nlohmann/json.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,16 @@ struct is_compatible_object_type_impl<true, RealType, CompatibleObjectType>
std::is_constructible<typename RealType::mapped_type, typename CompatibleObjectType::mapped_type>::value;
};

template<bool B, class RealType, class CompatibleStringType>
struct is_compatible_string_type_impl : std::false_type {};

template<class RealType, class CompatibleStringType>
struct is_compatible_string_type_impl<true, RealType, CompatibleStringType>
{
static constexpr auto value =
std::is_same<typename RealType::value_type, typename CompatibleStringType::value_type>::value;
};

template<class BasicJsonType, class CompatibleObjectType>
struct is_compatible_object_type
{
Expand All @@ -363,6 +373,15 @@ struct is_compatible_object_type
typename BasicJsonType::object_t, CompatibleObjectType >::value;
};

template<class BasicJsonType, class CompatibleStringType>
struct is_compatible_string_type
{
static auto constexpr value = is_compatible_string_type_impl <
conjunction<negation<std::is_same<void, CompatibleStringType>>,
has_value_type<CompatibleStringType>>::value,
typename BasicJsonType::string_t, CompatibleStringType >::value;
};

template<typename BasicJsonType, typename T>
struct is_basic_json_nested_type
{
Expand Down Expand Up @@ -977,6 +996,25 @@ void from_json(const BasicJsonType& j, typename BasicJsonType::string_t& s)
s = *j.template get_ptr<const typename BasicJsonType::string_t*>();
}

template <
typename BasicJsonType, typename CompatibleStringType,
enable_if_t <
is_compatible_string_type<BasicJsonType, CompatibleStringType>::value and
not std::is_same<typename BasicJsonType::string_t,
CompatibleStringType>::value and
std::is_constructible <
BasicJsonType, typename CompatibleStringType::value_type >::value,
int > = 0 >
void from_json(const BasicJsonType& j, CompatibleStringType& s)
{
if (JSON_UNLIKELY(not j.is_string()))
{
JSON_THROW(type_error::create(302, "type must be string, but is " + std::string(j.type_name())));
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a test case checking for this exception? If not, please add one.

}

s = *j.template get_ptr<const typename BasicJsonType::string_t*>();
}

template<typename BasicJsonType>
void from_json(const BasicJsonType& j, typename BasicJsonType::number_float_t& val)
{
Expand Down Expand Up @@ -1283,6 +1321,16 @@ struct external_constructor<value_t::string>
j.m_value = std::move(s);
j.assert_invariant();
}

template<typename BasicJsonType, typename CompatibleStringType,
enable_if_t<not std::is_same<CompatibleStringType, typename BasicJsonType::string_t>::value,
int> = 0>
static void construct(BasicJsonType& j, const CompatibleStringType& str)
{
j.m_type = value_t::string;
j.m_value.string = j.template create<typename BasicJsonType::string_t>(str);
j.assert_invariant();
}
};

template<>
Expand Down Expand Up @@ -12432,9 +12480,9 @@ class basic_json
not detail::is_basic_json<ValueType>::value
#ifndef _MSC_VER // fix for issue #167 operator<< ambiguity under VS2015
and not std::is_same<ValueType, std::initializer_list<typename string_t::value_type>>::value
#endif
#if defined(JSON_HAS_CPP_17)
#if defined(JSON_HAS_CPP_17) && _MSC_VER <= 1914
and not std::is_same<ValueType, typename std::string_view>::value
#endif
#endif
, int >::type = 0 >
operator ValueType() const
Expand Down
54 changes: 54 additions & 0 deletions test/src/unit-conversions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ using nlohmann::json;
#include <unordered_set>
#include <valarray>

#if (defined(__cplusplus) && __cplusplus >= 201703L) || (defined(_HAS_CXX17) && _HAS_CXX17 == 1) // fix for issue #464
#define JSON_HAS_CPP_17
#define JSON_HAS_CPP_14
#elif (defined(__cplusplus) && __cplusplus >= 201402L) || (defined(_HAS_CXX14) && _HAS_CXX14 == 1)
#define JSON_HAS_CPP_14
#endif

#if defined(JSON_HAS_CPP_17)
#include <string_view>
#endif

TEST_CASE("value conversion")
{
SECTION("get an object (explicit)")
Expand Down Expand Up @@ -292,6 +303,13 @@ TEST_CASE("value conversion")
std::string s = j.get<std::string>();
CHECK(json(s) == j);
}
#if defined(JSON_HAS_CPP_17)
SECTION("std::string_view")
{
std::string_view s = j.get<std::string_view>();
CHECK(json(s) == j);
}
#endif

SECTION("exception in case of a non-string type")
{
Expand All @@ -318,6 +336,34 @@ TEST_CASE("value conversion")
CHECK_THROWS_WITH(json(json::value_t::number_float).get<json::string_t>(),
"[json.exception.type_error.302] type must be string, but is number");
}

#if defined(JSON_HAS_CPP_17)
SECTION("exception in case of a non-string type using string_view")
{
CHECK_THROWS_AS(json(json::value_t::null).get<std::string_view>(), json::type_error&);
CHECK_THROWS_AS(json(json::value_t::object).get<std::string_view>(), json::type_error&);
CHECK_THROWS_AS(json(json::value_t::array).get<std::string_view>(), json::type_error&);
CHECK_THROWS_AS(json(json::value_t::boolean).get<std::string_view>(), json::type_error&);
CHECK_THROWS_AS(json(json::value_t::number_integer).get<std::string_view>(), json::type_error&);
CHECK_THROWS_AS(json(json::value_t::number_unsigned).get<std::string_view>(), json::type_error&);
CHECK_THROWS_AS(json(json::value_t::number_float).get<std::string_view>(), json::type_error&);

CHECK_THROWS_WITH(json(json::value_t::null).get<std::string_view>(),
"[json.exception.type_error.302] type must be string, but is null");
CHECK_THROWS_WITH(json(json::value_t::object).get<std::string_view>(),
"[json.exception.type_error.302] type must be string, but is object");
CHECK_THROWS_WITH(json(json::value_t::array).get<std::string_view>(),
"[json.exception.type_error.302] type must be string, but is array");
CHECK_THROWS_WITH(json(json::value_t::boolean).get<std::string_view>(),
"[json.exception.type_error.302] type must be string, but is boolean");
CHECK_THROWS_WITH(json(json::value_t::number_integer).get<std::string_view>(),
"[json.exception.type_error.302] type must be string, but is number");
CHECK_THROWS_WITH(json(json::value_t::number_unsigned).get<std::string_view>(),
"[json.exception.type_error.302] type must be string, but is number");
CHECK_THROWS_WITH(json(json::value_t::number_float).get<std::string_view>(),
"[json.exception.type_error.302] type must be string, but is number");
}
#endif
}

SECTION("get a string (implicit)")
Expand All @@ -331,6 +377,14 @@ TEST_CASE("value conversion")
CHECK(json(s) == j);
}

#if defined(JSON_HAS_CPP_17)
SECTION("std::string_view")
{
std::string_view s = j;
CHECK(json(s) == j);
}
#endif

SECTION("std::string")
{
std::string s = j;
Expand Down