7

I'm implementing a simple std::vector. There are two insert functions:

template <typename T, typename Allocator>
typename Vector<T, Allocator>::iterator
Vector<T, Allocator>::insert(const_iterator pos, size_type count, const T& value)
{
    checkIterator(pos);
    auto p = const_cast<iterator>(pos);
    if (count == 0) {
        return p;
    }
    for (size_type i = 0; i < count; ++i) {
        p = insert(p, value);
    }
    return p;
}

template <typename T, typename Allocator>
template <typename InputIt>
typename Vector<T, Allocator>::iterator
Vector<T, Allocator>::insert(const_iterator pos, InputIt first, InputIt last)
{
    checkIterator(pos);
    auto p = const_cast<iterator>(pos);
    if (first == last) {
        return p;
    }
    for (auto iter = first; iter != last; ++iter) {
        p = insert(p, *iter);
        ++p;
    }
    return p - (last-first);
}

But when I want to use first insert function, the compiler invokes the second one:

Vector<int> vi = {1, 2, 3};
vi.insert(vi.begin(), 3, 4); // get compile error, using insert(const_iterator pos, InputIt first, InputIt last).

Why compiler chooses the second function, and how to modify my code to make it right?

Rishi
  • 1,387
  • 10
  • 14
Sam
  • 103
  • 9
  • 2
    The specification of `std::vector` relies on "compiler magic" for this case: it says that the iterator version is not selected unless the deduced type would comply with `InputIterator` requirements. I guess when Concepts goes live you could express that easily in your own code; I'm not sure exactly what you can do before then – M.M Aug 09 '17 at 03:22
  • @M.M Why does it use compiler magic instead of regular user metaprogramming + sfinae? – Nir Friedman Aug 09 '17 at 03:27
  • Change one of the arguments so that it is a different type ... for example, by appending a `U` to the literal for the `size_type`, or by explicitly adding a cast. – o11c Aug 09 '17 at 03:31
  • @NirFriedman I don't know the rationale – M.M Aug 09 '17 at 03:38
  • g++ 7.1 apparently implements the current specification of concepts, but I can't find any predefined concept for checking InputIterator – M.M Aug 09 '17 at 03:41
  • 1
    @M.M. Where does the standard specify that this must rely on compiler magic? Concepts are not required in order to determine if a type meets complies with `InputIterator` requirements. – Benjamin Lindley Aug 09 '17 at 04:01
  • @BenjaminLindley the standard says "If [these functions] are called with a type InputIterator that does not qualify as an input iterator, then these functions shall not participate in overload resolution." – M.M Aug 09 '17 at 05:51
  • 1
    @M.M. Okay, but how does that imply that there is compiler magic involved, since that qualification can be checked with template metaprogramming, and the functions can be consequently disabled with SFINAE? – Benjamin Lindley Aug 09 '17 at 06:15
  • @M.M OP looks to have copied members out of `std::vector` from the standard or some similar reference, and not realised there are extra conditions that make `std::vector` nice to use. When I look at my platform's implementation of `std::vector`, I see a bunch of SFINAE around the InputIterator overload. Not very "compiler magic" – Caleth Aug 09 '17 at 08:17

2 Answers2

7

Unfortunately, doing this fully correctly is a problem. However, you can do something that is reasonable (and will work in this case). Basically, you need to conditionally enable the second overload dependent on whether the deduced type InputIt actually meets the requirements for input iterator. There's a whole list of input iterator requirements: http://en.cppreference.com/w/cpp/concept/InputIterator. However, we'll just focus on one that will solve this situation and most common cases for us. Namely, we'll verify that the type InputIt actually has a correct operator*. We use the void_t trick to build a trait for this:

template <class ... T> using void_t = void;

template <class T, class = void>
struct has_iterator_deref : std::false_type {};

template <class T>
struct has_iterator_deref<T, std::enable_if_t<
    std::is_same<typename std::iterator_traits<T>::reference,
                 decltype(*std::declval<T>())>::value>> : std::true_type {};

The long and short of it is, that this struct will ensure that an instance of T can be dereferenced with * and yields the same type as iterator_traits<T>::reference. Having done that, we now use this to sfinae the second overload:

template <typename T, typename Allocator>
template <typename InputIt, class = enable_if_t<has_iterator_deref<T>::value>>
typename Vector<T, Allocator>::iterator
Vector<T, Allocator>::insert(const_iterator pos, InputIt first, InputIt last)
...

If you are feeling frisky, you can in fact go through the entire list of Input Iterator requirements, and as far as I can see, build a trait that detects if each one is present, and then finally take the conjunction to do exactly correct detection to ensure that InputIt meets the Input Iterator concept. It's rather a pain though.

Vadim Kotov
  • 8,084
  • 8
  • 48
  • 62
Nir Friedman
  • 17,108
  • 2
  • 44
  • 72
  • @1201ProgramAlarm The first line in the question is: "I'm implementing a simple std::vector" – Nir Friedman Aug 09 '17 at 03:45
  • I just got a error says : "no type named ‘type’ in ‘struct std::enable_if", there's something wrong in `is_same<...>` I think. – Sam Aug 09 '17 at 04:52
  • This works! But when I use `iter = vi.insert(vi.cend(), tmp.begin(), tmp.end())`, the compiler invoke `insert(const_iterator pos, size_type count, const T& value)`, guess I have more works to do. :) – Sam Aug 09 '17 at 06:45
1

A fairly simple way to do this is to rely on iterator_traits::iterator_category from <iterator>

It could look something like this:

#include <iterator>

template<typename It>
using iterator_category_t = typename std::iterator_traits<It>::iterator_category;

template<typename T>
class container
{
public:
    template <typename InputIt, typename ItCat = iterator_category_t<InputIt>>
    iterator insert(const_iterator pos, InputIt first, InputIt last);
    // the rest

This will SFINAE away things like int, since they don't fulfill any iterator category.

This has the added benefit that you can dispatch on ItCat{} inside the implementation, if you have different algorithms for different iterator categorys.

sp2danny
  • 7,488
  • 3
  • 31
  • 53