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

Add a SFINAE friendly iterator_traits and use that instead. #1342

Merged
merged 3 commits into from
Dec 19, 2018
+125 −20
Merged

Add a SFINAE friendly iterator_traits and use that instead. #1342

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f665a92
Implement SFINAE friendly iterator_traits and use that instead.
dgavedissian Nov 7, 2018
2c23f0a
Changes requested from code review.
dgavedissian Dec 8, 2018
f1080d7
Code review.
dgavedissian Dec 18, 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
6 changes: 3 additions & 3 deletions include/nlohmann/detail/input/input_adapters.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ class input_adapter
/// input adapter for iterator range with contiguous storage
template<class IteratorType,
typename std::enable_if<
std::is_same<typename std::iterator_traits<IteratorType>::iterator_category, std::random_access_iterator_tag>::value,
std::is_same<typename iterator_traits<IteratorType>::iterator_category, std::random_access_iterator_tag>::value,
int>::type = 0>
input_adapter(IteratorType first, IteratorType last)
{
Expand All @@ -350,7 +350,7 @@ class input_adapter

// assertion to check that each element is 1 byte long
static_assert(
sizeof(typename std::iterator_traits<IteratorType>::value_type) == 1,
sizeof(typename iterator_traits<IteratorType>::value_type) == 1,
"each element in the iterator range must have the size of 1 byte");

const auto len = static_cast<size_t>(std::distance(first, last));
Expand All @@ -374,7 +374,7 @@ class input_adapter
/// input adapter for contiguous container
template<class ContiguousContainer, typename
std::enable_if<not std::is_pointer<ContiguousContainer>::value and
std::is_base_of<std::random_access_iterator_tag, typename std::iterator_traits<decltype(std::begin(std::declval<ContiguousContainer const>()))>::iterator_category>::value,
std::is_base_of<std::random_access_iterator_tag, typename iterator_traits<decltype(std::begin(std::declval<ContiguousContainer const>()))>::iterator_category>::value,
int>::type = 0>
input_adapter(const ContiguousContainer& c)
: input_adapter(std::begin(c), std::end(c)) {}
Expand Down
49 changes: 49 additions & 0 deletions include/nlohmann/detail/iterators/iterator_traits.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#pragma once

#include <iterator> // random_access_iterator_tag

#include <nlohmann/detail/meta/void_t.hpp>
#include <nlohmann/detail/meta/cpp_future.hpp>

namespace nlohmann
{
namespace detail
{
template <typename It, typename = void>
struct iterator_types {};

template <typename It>
struct iterator_types<
It,
void_t<typename It::difference_type, typename It::value_type, typename It::pointer,
typename It::reference, typename It::iterator_category>> {
using difference_type = typename It::difference_type;
using value_type = typename It::value_type;
using pointer = typename It::pointer;
using reference = typename It::reference;
using iterator_category = typename It::iterator_category;
};

// This is required as some compilers implement std::iterator_traits in a way that
// doesn't work with SFINAE. See https://github.com/nlohmann/json/issues/1341.
template <typename T, typename = void>
struct iterator_traits
{
};

template <typename T>
struct iterator_traits<T, enable_if_t<!std::is_pointer<T>::value>>
: iterator_types<T>
{
};

template <typename T>
struct iterator_traits<T*, enable_if_t<std::is_object<T>::value>> {
typedef std::random_access_iterator_tag iterator_category;
typedef T value_type;
typedef ptrdiff_t difference_type;
typedef T* pointer;
typedef T& reference;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer using, but that's a minor thing

Copy link
Owner

Choose a reason for hiding this comment

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

I agree. @davedissian could you please make this last change. Then it should be good to go. Thanks!

};
}
}
9 changes: 5 additions & 4 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 @@ -6,6 +6,7 @@
#include <utility> // declval

#include <nlohmann/json_fwd.hpp>
#include <nlohmann/detail/iterators/iterator_traits.hpp>
#include <nlohmann/detail/meta/cpp_future.hpp>
#include <nlohmann/detail/meta/detected.hpp>
#include <nlohmann/detail/macro_scope.hpp>
Expand Down Expand Up @@ -131,10 +132,10 @@ template <typename T, typename = void>
struct is_iterator_traits : std::false_type {};

template <typename T>
struct is_iterator_traits<std::iterator_traits<T>>
struct is_iterator_traits<iterator_traits<T>>
{
private:
using traits = std::iterator_traits<T>;
using traits = iterator_traits<T>;

public:
static constexpr auto value =
Expand Down Expand Up @@ -251,7 +252,7 @@ struct is_compatible_array_type_impl <
// Therefore it is detected as a CompatibleArrayType.
// The real fix would be to have an Iterable concept.
not is_iterator_traits<
std::iterator_traits<CompatibleArrayType>>::value >>
iterator_traits<CompatibleArrayType>>::value >>
{
static constexpr bool value =
std::is_constructible<BasicJsonType,
Expand Down Expand Up @@ -288,7 +289,7 @@ struct is_constructible_array_type_impl <
// Therefore it is detected as a ConstructibleArrayType.
// The real fix would be to have an Iterable concept.
not is_iterator_traits <
std::iterator_traits<ConstructibleArrayType >>::value and
iterator_traits<ConstructibleArrayType >>::value and

(std::is_same<typename ConstructibleArrayType::value_type, typename BasicJsonType::array_t::value_type>::value or
has_from_json<BasicJsonType,
Expand Down
2 changes: 1 addition & 1 deletion include/nlohmann/json.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ SOFTWARE.
#include <functional> // hash, less
#include <initializer_list> // initializer_list
#include <iosfwd> // istream, ostream
#include <iterator> // iterator_traits, random_access_iterator_tag
#include <iterator> // random_access_iterator_tag
#include <numeric> // accumulate
#include <string> // string, stoi, to_string
#include <utility> // declval, forward, move, pair, swap
Expand Down
78 changes: 66 additions & 12 deletions single_include/nlohmann/json.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ SOFTWARE.
#include <functional> // hash, less
#include <initializer_list> // initializer_list
#include <iosfwd> // istream, ostream
#include <iterator> // iterator_traits, random_access_iterator_tag
#include <iterator> // random_access_iterator_tag
#include <numeric> // accumulate
#include <string> // string, stoi, to_string
#include <utility> // declval, forward, move, pair, swap
Expand Down Expand Up @@ -324,12 +324,10 @@ constexpr T static_const<T>::value;

// #include <nlohmann/json_fwd.hpp>

// #include <nlohmann/detail/meta/cpp_future.hpp>
// #include <nlohmann/detail/iterators/iterator_traits.hpp>

// #include <nlohmann/detail/meta/detected.hpp>


#include <type_traits>
#include <iterator> // random_access_iterator_tag

// #include <nlohmann/detail/meta/void_t.hpp>

Expand All @@ -346,6 +344,62 @@ template <typename ...Ts> using void_t = typename make_void<Ts...>::type;
} // namespace detail
} // namespace nlohmann

// #include <nlohmann/detail/meta/cpp_future.hpp>


namespace nlohmann
{
namespace detail
{
template <typename It, typename = void>
struct iterator_types {};

template <typename It>
struct iterator_types <
It,
void_t<typename It::difference_type, typename It::value_type, typename It::pointer,
typename It::reference, typename It::iterator_category >>
{
using difference_type = typename It::difference_type;
using value_type = typename It::value_type;
using pointer = typename It::pointer;
using reference = typename It::reference;
using iterator_category = typename It::iterator_category;
};

// This is required due to https://github.com/nlohmann/json/issues/1341 - where some
// compilers implement std::iterator_traits in a way that doesn't work with SFINAE.
template <typename T, typename = void>
struct iterator_traits
{
};

template <typename T>
struct iterator_traits < T, enable_if_t < !std::is_pointer<T>::value >>
: iterator_types<T>
{
};

template <typename T>
struct iterator_traits<T*, enable_if_t<std::is_object<T>::value>>
{
typedef std::random_access_iterator_tag iterator_category;
typedef T value_type;
typedef ptrdiff_t difference_type;
typedef T* pointer;
typedef T& reference;
};
}
}
// #include <nlohmann/detail/meta/cpp_future.hpp>

// #include <nlohmann/detail/meta/detected.hpp>


#include <type_traits>

// #include <nlohmann/detail/meta/void_t.hpp>


// http://en.cppreference.com/w/cpp/experimental/is_detected
namespace nlohmann
Expand Down Expand Up @@ -522,10 +576,10 @@ template <typename T, typename = void>
struct is_iterator_traits : std::false_type {};

template <typename T>
struct is_iterator_traits<std::iterator_traits<T>>
struct is_iterator_traits<iterator_traits<T>>
{
private:
using traits = std::iterator_traits<T>;
using traits = iterator_traits<T>;

public:
static constexpr auto value =
Expand Down Expand Up @@ -642,7 +696,7 @@ struct is_compatible_array_type_impl <
// Therefore it is detected as a CompatibleArrayType.
// The real fix would be to have an Iterable concept.
not is_iterator_traits<
std::iterator_traits<CompatibleArrayType>>::value >>
iterator_traits<CompatibleArrayType>>::value >>
{
static constexpr bool value =
std::is_constructible<BasicJsonType,
Expand Down Expand Up @@ -679,7 +733,7 @@ struct is_constructible_array_type_impl <
// Therefore it is detected as a ConstructibleArrayType.
// The real fix would be to have an Iterable concept.
not is_iterator_traits <
std::iterator_traits<ConstructibleArrayType >>::value and
iterator_traits<ConstructibleArrayType >>::value and

(std::is_same<typename ConstructibleArrayType::value_type, typename BasicJsonType::array_t::value_type>::value or
has_from_json<BasicJsonType,
Expand Down Expand Up @@ -2380,7 +2434,7 @@ class input_adapter
/// input adapter for iterator range with contiguous storage
template<class IteratorType,
typename std::enable_if<
std::is_same<typename std::iterator_traits<IteratorType>::iterator_category, std::random_access_iterator_tag>::value,
std::is_same<typename iterator_traits<IteratorType>::iterator_category, std::random_access_iterator_tag>::value,
int>::type = 0>
input_adapter(IteratorType first, IteratorType last)
{
Expand All @@ -2399,7 +2453,7 @@ class input_adapter

// assertion to check that each element is 1 byte long
static_assert(
sizeof(typename std::iterator_traits<IteratorType>::value_type) == 1,
sizeof(typename iterator_traits<IteratorType>::value_type) == 1,
"each element in the iterator range must have the size of 1 byte");

const auto len = static_cast<size_t>(std::distance(first, last));
Expand All @@ -2423,7 +2477,7 @@ class input_adapter
/// input adapter for contiguous container
template<class ContiguousContainer, typename
std::enable_if<not std::is_pointer<ContiguousContainer>::value and
std::is_base_of<std::random_access_iterator_tag, typename std::iterator_traits<decltype(std::begin(std::declval<ContiguousContainer const>()))>::iterator_category>::value,
std::is_base_of<std::random_access_iterator_tag, typename iterator_traits<decltype(std::begin(std::declval<ContiguousContainer const>()))>::iterator_category>::value,
int>::type = 0>
input_adapter(const ContiguousContainer& c)
: input_adapter(std::begin(c), std::end(c)) {}
Expand Down