[libcxx] Qualify make_move_iterator in vector::insert for input iterators

[logan-5]

Summary

Unqualified calls to make_move_iterator in the vector::insert overload for input iterators lead to ADL issues: https://gcc.godbolt.org/z/bmcNbh

As a bookkeeping question, are these ADL fixes more useful as tiny separate patches like this, or would it be less annoying if they were bundled together in a larger patch?

[• Quuxplusone]

I don't understand the actual algorithm being used in insert-with-input-iterators, but the ADL change certainly looks good to me. :)

[logan-5]

In D74290#1865949, @Quuxplusone wrote:

I don't understand the actual algorithm being used in insert-with-input-iterators

Seems reasonable enough to me:

1. insert up to capacity at the end of the vector
2. construct any additional elements that would overflow capacity in a scratch buffer, then see how big it is (because input iterators), and reserve new capacity (if exception is thrown, destroy items from step 1--easy since they're still sitting at the end)
3. rotate items from step 1 into place, and then insert() the scratch buffer after those

I'm not sure why you'd insert() the scratch buffer instead of just moving its elements onto the vector's end and then rotating all the elements into place at once, but maybe the insert() overload called for the scratch buffer does things more efficiently somehow.

[ldionne]

Thanks for the patch!

libcxx/include/vector

#ifndef _LIBCPP_NO_EXCEPTIONS
         catch (...)
         {
             erase(__make_iter(__old_last), end());
             throw;
         }
 #endif  // _LIBCPP_NO_EXCEPTIONS
     }
     __p = _VSTD::rotate(__p, __old_last, this->__end_);
-    insert(__make_iter(__p), make_move_iterator(__v.begin()),
-                                    make_move_iterator(__v.end()));
+    insert(__make_iter(__p), _VSTD::make_move_iterator(__v.begin()),
+                             _VSTD::make_move_iterator(__v.end()));
     return begin() + __off;
 }
 
 template <class _Tp, class _Allocator>
 template <class _ForwardIterator>
 typename enable_if
 <
     __is_cpp17_forward_iterator<_ForwardIterator>::value &&

libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_iter_iter.pass.cpp

 #include <cstddef>
 
 #include "test_macros.h"
 #include "test_allocator.h"
 #include "test_iterators.h"
 #include "min_allocator.h"
 #include "asan_testing.h"
 
+namespace adl {
+struct S {};
+void make_move_iterator(S*) {}
+}
+
 int main(int, char**)
 {
     {
         typedef std::vector<int> V;
         V v(100);
         int a[] = {1, 2, 3, 4, 5};
         const int N = sizeof(a)/sizeof(a[0]);
         V::iterator i = v.insert(v.cbegin() + 10, input_iterator<const int*>(a),
#if TEST_STD_VER >= 11
             assert(v[j] == 0);
         for (std::size_t k = 0; k < N; ++j, ++k)
             assert(v[j] == a[k]);
         for (; j < 105; ++j)
             assert(v[j] == 0);
     }
 #endif
 
+    {
+        std::vector<adl::S> s;
+        s.insert(s.end(), input_iterator<adl::S*>(), input_iterator<adl::S*>());
+    }
+
   return 0;
 }