Commit 1d452319 authored by Tom Stellard's avatar Tom Stellard
Browse files

Merging r315994: 

------------------------------------------------------------------------
r315994 | ericwf | 2017-10-17 06:03:17 -0700 (Tue, 17 Oct 2017) | 18 lines

[libc++] Fix PR34898 - vector iterator constructors and assign method perform push_back instead of emplace_back.

Summary:
The constructors `vector(Iter, Iter, Alloc = Alloc{})` and `assign(Iter, Iter)` don't correctly perform EmplaceConstruction from the result of dereferencing the iterator. This results in them performing an additional and unneeded copy.

This patch addresses the issue by correctly using `emplace_back` in C++11 and newer.

There are also some bugs in our `insert` implementation, but those will be handled separately.

@mclow.lists We should probably merge this into 5.1, agreed?

Reviewers: mclow.lists, dlj, EricWF

Reviewed By: mclow.lists, EricWF

Subscribers: cfe-commits, mclow.lists

Differential Revision: https://reviews.llvm.org/D38757
------------------------------------------------------------------------

llvm-svn: 318837
parent 8c2d95c8

libcxx/include/deque

+4 −1
Original line number Diff line number Diff line
@@ -1356,7 +1356,6 @@ public: 
    iterator insert(const_iterator __p, initializer_list<value_type> __il)

        {return insert(__p, __il.begin(), __il.end());}

#endif  // _LIBCPP_CXX03_LANG



    iterator insert(const_iterator __p, const value_type& __v);

    iterator insert(const_iterator __p, size_type __n, const value_type& __v);

    template <class _InputIter>
@@ -2224,7 +2223,11 @@ deque<_Tp, _Allocator>::__append(_InpIter __f, _InpIter __l, 
                                                   !__is_forward_iterator<_InpIter>::value>::type*)

{

    for (; __f != __l; ++__f)

#ifdef _LIBCPP_CXX03_LANG

        push_back(*__f);

#else

        emplace_back(*__f);

#endif

}



template <class _Tp, class _Allocator>

libcxx/include/list

+11 −2
Original line number Diff line number Diff line
@@ -992,6 +992,15 @@ public: 
    void push_front(const value_type& __x);

    void push_back(const value_type& __x);



#ifndef _LIBCPP_CXX03_LANG

    template <class _Arg>

    _LIBCPP_INLINE_VISIBILITY

    void __emplace_back(_Arg&& __arg) { emplace_back(_VSTD::forward<_Arg>(__arg)); }

#else

    _LIBCPP_INLINE_VISIBILITY

    void __emplace_back(value_type const& __arg) { push_back(__arg); }

#endif



    iterator insert(const_iterator __p, const value_type& __x);

    iterator insert(const_iterator __p, size_type __n, const value_type& __x);

    template <class _InpIter>
@@ -1189,7 +1198,7 @@ list<_Tp, _Alloc>::list(_InpIter __f, _InpIter __l, 
    __get_db()->__insert_c(this);

#endif

    for (; __f != __l; ++__f)

        push_back(*__f);

        __emplace_back(*__f);

}



template <class _Tp, class _Alloc>
@@ -1202,7 +1211,7 @@ list<_Tp, _Alloc>::list(_InpIter __f, _InpIter __l, const allocator_type& __a, 
    __get_db()->__insert_c(this);

#endif

    for (; __f != __l; ++__f)

        push_back(*__f);

        __emplace_back(*__f);

}



template <class _Tp, class _Alloc>

libcxx/include/vector

+14 −3
Original line number Diff line number Diff line
@@ -674,6 +674,17 @@ public: 
    const value_type* data() const _NOEXCEPT

        {return _VSTD::__to_raw_pointer(this->__begin_);}



#ifdef _LIBCPP_CXX03_LANG

    _LIBCPP_INLINE_VISIBILITY

    void __emplace_back(const value_type& __x) { push_back(__x); }

#else

    template <class _Arg>

    _LIBCPP_INLINE_VISIBILITY

    void __emplace_back(_Arg&& __arg) {

      emplace_back(_VSTD::forward<_Arg>(__arg));

    }

#endif



    _LIBCPP_INLINE_VISIBILITY void push_back(const_reference __x);



#ifndef _LIBCPP_CXX03_LANG
@@ -1128,7 +1139,7 @@ vector<_Tp, _Allocator>::vector(_InputIterator __first, 
    __get_db()->__insert_c(this);

#endif

    for (; __first != __last; ++__first)

        push_back(*__first);

        __emplace_back(*__first);

}



template <class _Tp, class _Allocator>
@@ -1145,7 +1156,7 @@ vector<_Tp, _Allocator>::vector(_InputIterator __first, _InputIterator __last, c 
    __get_db()->__insert_c(this);

#endif

    for (; __first != __last; ++__first)

        push_back(*__first);

        __emplace_back(*__first);

}



template <class _Tp, class _Allocator>
@@ -1365,7 +1376,7 @@ vector<_Tp, _Allocator>::assign(_InputIterator __first, _InputIterator __last) 
{

    clear();

    for (; __first != __last; ++__first)

        push_back(*__first);

        __emplace_back(*__first);

}



template <class _Tp, class _Allocator>

libcxx/test/std/containers/sequences/deque/deque.cons/assign_iter_iter.pass.cpp

+52 −1
Original line number Diff line number Diff line
@@ -19,6 +19,9 @@ 
#include "test_macros.h"

#include "test_iterators.h"

#include "min_allocator.h"

#if TEST_STD_VER >= 11

#include "emplace_constructible.h"

#endif



template <class C>

C
@@ -80,7 +83,7 @@ testNI(int start, int N, int M) 
    testI(c1, c2);

}



int main()

void basic_test()

{

    {

    int rng[] = {0, 1, 2, 3, 1023, 1024, 1025, 2047, 2048, 2049};
@@ -103,3 +106,51 @@ int main() 
    }

#endif

}



void test_emplacable_concept() {

#if TEST_STD_VER >= 11

  int arr1[] = {42};

  int arr2[] = {1, 101, 42};

  {

    using T = EmplaceConstructibleMoveableAndAssignable<int>;

    using It = random_access_iterator<int*>;

    {

      std::deque<T> v;

      v.assign(It(arr1), It(std::end(arr1)));

      assert(v[0].value == 42);

    }

    {

      std::deque<T> v;

      v.assign(It(arr2), It(std::end(arr2)));

      assert(v[0].value == 1);

      assert(v[1].value == 101);

      assert(v[2].value == 42);

    }

  }

  {

    using T = EmplaceConstructibleMoveableAndAssignable<int>;

    using It = input_iterator<int*>;

    {

      std::deque<T> v;

      v.assign(It(arr1), It(std::end(arr1)));

      assert(v[0].copied == 0);

      assert(v[0].value == 42);

    }

    {

      std::deque<T> v;

      v.assign(It(arr2), It(std::end(arr2)));

      //assert(v[0].copied == 0);

      assert(v[0].value == 1);

      //assert(v[1].copied == 0);

      assert(v[1].value == 101);

      assert(v[2].copied == 0);

      assert(v[2].value == 42);

    }

  }

#endif

}



int main() {

  basic_test();

  test_emplacable_concept();

}

libcxx/test/std/containers/sequences/deque/deque.cons/iter_iter.pass.cpp

+50 −1
Original line number Diff line number Diff line
@@ -15,9 +15,13 @@ 
#include <cassert>

#include <cstddef>



#include "test_macros.h"

#include "test_allocator.h"

#include "test_iterators.h"

#include "min_allocator.h"

#if TEST_STD_VER >= 11

#include "emplace_constructible.h"

#endif



template <class InputIterator>

void
@@ -48,7 +52,7 @@ test(InputIterator f, InputIterator l) 
        assert(*i == *f);

}



int main()

void basic_test()

{

    int ab[] = {3, 4, 2, 8, 0, 1, 44, 34, 45, 96, 80, 1, 13, 31, 45};

    int* an = ab + sizeof(ab)/sizeof(ab[0]);
@@ -61,3 +65,48 @@ int main() 
    test<min_allocator<int> >(ab, an);

#endif

}





void test_emplacable_concept() {

#if TEST_STD_VER >= 11

  int arr1[] = {42};

  int arr2[] = {1, 101, 42};

  {

    using T = EmplaceConstructibleAndMoveable<int>;

    using It = random_access_iterator<int*>;

    {

      std::deque<T> v(It(arr1), It(std::end(arr1)));

      assert(v[0].value == 42);

    }

    {

      std::deque<T> v(It(arr2), It(std::end(arr2)));

      assert(v[0].value == 1);

      assert(v[1].value == 101);

      assert(v[2].value == 42);

    }

  }

  {

    using T = EmplaceConstructibleAndMoveable<int>;

    using It = input_iterator<int*>;

    {

      std::deque<T> v(It(arr1), It(std::end(arr1)));

      assert(v[0].copied == 0);

      assert(v[0].value == 42);

    }

    {

      std::deque<T> v(It(arr2), It(std::end(arr2)));

      //assert(v[0].copied == 0);

      assert(v[0].value == 1);

      //assert(v[1].copied == 0);

      assert(v[1].value == 101);

      assert(v[2].copied == 0);

      assert(v[2].value == 42);

    }

  }

#endif

}



int main() {

  basic_test();

  test_emplacable_concept();

}