Skip to content

Commit

Permalink
Reworked iterator handling in next/prior helpers.
Browse files Browse the repository at this point in the history
The new implementation tries to detect if the incremented/decremented type
is an iterator first and if not falls back to operator probing. This way
iterators that are not SFINAE-friendly (i.e. unconditionally define
arithmetic operators regardless of the iterator category) are still treated
as iterators through std::advance and do not fail the compilation.

The iterator detection is based on probing for the nested iterator_category
type that is expected to be present in class-type iterators. This heuristic
is not flawless since iterators are not required to defined this type.
User-defined iterators may not have it and instead specialize
std::iterator_traits. This use case is not covered by the current implementation
and will likely fail to compile. With C++17 SFINAE-friendly std::iterator_traits
this can be fixed, but currently Boost.Config lacks the macro to detect
availability of this feature. Support for it can be added by a later commit.

Also simplified boost::prior for iterators, removing the possibility of
integer overflow caused by negation of the distance value.
  • Loading branch information
Lastique committed Jul 9, 2017
1 parent 21261a8 commit 6cf9c22
Showing 1 changed file with 71 additions and 55 deletions.
126 changes: 71 additions & 55 deletions include/boost/next_prior.hpp
@@ -1,8 +1,11 @@
// Boost next_prior.hpp header file ---------------------------------------//

// (C) Copyright Dave Abrahams and Daniel Walker 1999-2003. Distributed under the Boost
// Software License, Version 1.0. (See accompanying file
// LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
// (C) Copyright Dave Abrahams and Daniel Walker 1999-2003.
// Copyright (c) Andrey Semashev 2017
//
// Distributed under the Boost Software License, Version 1.0.
// (See accompanying file LICENSE_1_0.txt or copy at
// http://www.boost.org/LICENSE_1_0.txt)

// See http://www.boost.org/libs/utility for documentation.

Expand All @@ -13,13 +16,7 @@
#define BOOST_NEXT_PRIOR_HPP_INCLUDED

#include <iterator>
#if defined(_MSC_VER) && _MSC_VER <= 1310
#include <boost/mpl/and.hpp>
#include <boost/type_traits/is_integral.hpp>
#endif
#include <boost/type_traits/is_unsigned.hpp>
#include <boost/type_traits/integral_promotion.hpp>
#include <boost/type_traits/make_signed.hpp>
#include <boost/config.hpp>
#include <boost/type_traits/has_plus.hpp>
#include <boost/type_traits/has_plus_assign.hpp>
#include <boost/type_traits/has_minus.hpp>
Expand All @@ -39,34 +36,55 @@ namespace boost {

namespace next_prior_detail {

template< typename T, typename Distance, bool HasPlus = has_plus< T, Distance >::value >
struct next_impl2
// The trait attempts to detect if the T type is an iterator. Class-type iterators are assumed
// to have the nested type iterator_category. Strictly speaking, this is not required to be the
// case (e.g. a user can specialize iterator_traits for T without defining T::iterator_category).
// Still, this is a good heuristic in practice, and we can't do anything better anyway.
// Since C++17 we can test for iterator_traits<T>::iterator_category presence instead as it is
// required to be only present for iterators.
template< typename T >
struct is_iterator
{
static T call(T x, Distance n)
{
std::advance(x, n);
return x;
}
private:
typedef char yes_type;
typedef char (&no_type)[2];

template< typename U >
static yes_type check_iterator_category(typename U::iterator_category*);
template< typename U >
static no_type check_iterator_category(...);

public:
static BOOST_CONSTEXPR_OR_CONST bool value = sizeof(is_iterator< T >::BOOST_NESTED_TEMPLATE check_iterator_category< T >(0)) == sizeof(yes_type);
};

template< typename T >
struct is_iterator< T* >
{
static BOOST_CONSTEXPR_OR_CONST bool value = true;
};


template< typename T, typename Distance, bool HasPlus = has_plus< T, Distance >::value >
struct next_plus_impl;

template< typename T, typename Distance >
struct next_impl2< T, Distance, true >
struct next_plus_impl< T, Distance, true >
{
static T call(T x, Distance n)
{
return x + n;
}
};


template< typename T, typename Distance, bool HasPlusAssign = has_plus_assign< T, Distance >::value >
struct next_impl1 :
public next_impl2< T, Distance >
struct next_plus_assign_impl :
public next_plus_impl< T, Distance >
{
};

template< typename T, typename Distance >
struct next_impl1< T, Distance, true >
struct next_plus_assign_impl< T, Distance, true >
{
static T call(T x, Distance n)
{
Expand All @@ -75,63 +93,43 @@ struct next_impl1< T, Distance, true >
}
};


template<
typename T,
typename Distance,
typename PromotedDistance = typename integral_promotion< Distance >::type,
#if !defined(_MSC_VER) || _MSC_VER > 1310
bool IsUInt = is_unsigned< PromotedDistance >::value
#else
// MSVC 7.1 has problems with applying is_unsigned to non-integral types
bool IsUInt = mpl::and_< is_integral< PromotedDistance >, is_unsigned< PromotedDistance > >::value
#endif
>
struct prior_impl3
template< typename T, typename Distance, bool IsIterator = is_iterator< T >::value >
struct next_advance_impl :
public next_plus_assign_impl< T, Distance >
{
static T call(T x, Distance n)
{
std::advance(x, -n);
return x;
}
};

template< typename T, typename Distance, typename PromotedDistance >
struct prior_impl3< T, Distance, PromotedDistance, true >
template< typename T, typename Distance >
struct next_advance_impl< T, Distance, true >
{
static T call(T x, Distance n)
{
typedef typename make_signed< PromotedDistance >::type signed_distance;
std::advance(x, -static_cast< signed_distance >(static_cast< PromotedDistance >(n)));
std::advance(x, n);
return x;
}
};


template< typename T, typename Distance, bool HasMinus = has_minus< T, Distance >::value >
struct prior_impl2 :
public prior_impl3< T, Distance >
{
};
struct prior_minus_impl;

template< typename T, typename Distance >
struct prior_impl2< T, Distance, true >
struct prior_minus_impl< T, Distance, true >
{
static T call(T x, Distance n)
{
return x - n;
}
};


template< typename T, typename Distance, bool HasMinusAssign = has_minus_assign< T, Distance >::value >
struct prior_impl1 :
public prior_impl2< T, Distance >
struct prior_minus_assign_impl :
public prior_minus_impl< T, Distance >
{
};

template< typename T, typename Distance >
struct prior_impl1< T, Distance, true >
struct prior_minus_assign_impl< T, Distance, true >
{
static T call(T x, Distance n)
{
Expand All @@ -140,6 +138,24 @@ struct prior_impl1< T, Distance, true >
}
};

template< typename T, typename Distance, bool IsIterator = is_iterator< T >::value >
struct prior_advance_impl :
public prior_minus_assign_impl< T, Distance >
{
};

template< typename T, typename Distance >
struct prior_advance_impl< T, Distance, true >
{
static T call(T x, Distance n)
{
// Avoid negating n to sidestep possible integer overflow
std::reverse_iterator< T > rx(x);
std::advance(rx, n);
return rx.base();
}
};

} // namespace next_prior_detail

template <class T>
Expand All @@ -148,7 +164,7 @@ inline T next(T x) { return ++x; }
template <class T, class Distance>
inline T next(T x, Distance n)
{
return next_prior_detail::next_impl1< T, Distance >::call(x, n);
return next_prior_detail::next_advance_impl< T, Distance >::call(x, n);
}

template <class T>
Expand All @@ -157,7 +173,7 @@ inline T prior(T x) { return --x; }
template <class T, class Distance>
inline T prior(T x, Distance n)
{
return next_prior_detail::prior_impl1< T, Distance >::call(x, n);
return next_prior_detail::prior_advance_impl< T, Distance >::call(x, n);
}

} // namespace boost
Expand Down

0 comments on commit 6cf9c22

Please sign in to comment.