[libcxx] r243530 - Fix a self-move bug in inplace_merge. Thanks to Ted and Dexon for the report and the suggested fix.
Sean Silva
chisophugis at gmail.com
Thu Jul 30 10:10:49 PDT 2015
On Wed, Jul 29, 2015 at 12:50 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
>
> > On 2015-Jul-29, at 09:25, Marshall Clow <mclow.lists at gmail.com> wrote:
> >
> > Author: marshall
> > Date: Wed Jul 29 11:25:45 2015
> > New Revision: 243530
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=243530&view=rev
> > Log:
> > Fix a self-move bug in inplace_merge. Thanks to Ted and Dexon for the
> report and the suggested fix.
> >
> > Modified:
> > libcxx/trunk/include/algorithm
> >
> libcxx/trunk/test/std/algorithms/alg.sorting/alg.merge/inplace_merge.pass.cpp
> >
> libcxx/trunk/test/std/algorithms/alg.sorting/alg.merge/inplace_merge_comp.pass.cpp
> >
> > Modified: libcxx/trunk/include/algorithm
> > URL:
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/algorithm?rev=243530&r1=243529&r2=243530&view=diff
> >
> ==============================================================================
> > --- libcxx/trunk/include/algorithm (original)
> > +++ libcxx/trunk/include/algorithm Wed Jul 29 11:25:45 2015
> > @@ -4361,6 +4361,34 @@ merge(_InputIterator1 __first1, _InputIt
> >
> > // inplace_merge
> >
> > +template <class _Compare, class _InputIterator1, class _InputIterator2,
> > + class _OutputIterator>
> > +void __half_inplace_merge(_InputIterator1 __first1, _InputIterator1
> __last1,
> > + _InputIterator2 __first2, _InputIterator2
> __last2,
> > + _OutputIterator __result, _Compare __comp)
> > +{
> > + for (; __first1 != __last1; ++__result)
> > + {
> > + if (__first2 == __last2)
> > + {
> > + _VSTD::move(__first1, __last1, __result);
> > + return;
> > + }
> > +
> > + if (__comp(*__first2, *__first1))
> > + {
> > + *__result = _VSTD::move(*__first2);
> > + ++__first2;
> > + }
> > + else
> > + {
> > + *__result = _VSTD::move(*__first1);
> > + ++__first1;
> > + }
> > + }
> > + // __first2 through __last2 are already in the right spot.
> > +}
> > +
> > template <class _Compare, class _BidirectionalIterator>
> > void
> > __buffered_inplace_merge(_BidirectionalIterator __first,
> _BidirectionalIterator __middle, _BidirectionalIterator __last,
> > @@ -4376,11 +4404,7 @@ __buffered_inplace_merge(_BidirectionalI
> > value_type* __p = __buff;
> > for (_BidirectionalIterator __i = __first; __i != __middle;
> __d.__incr((value_type*)0), (void) ++__i, ++__p)
> > ::new(__p) value_type(_VSTD::move(*__i));
> > - __merge<_Compare>(move_iterator<value_type*>(__buff),
> > - move_iterator<value_type*>(__p),
> > -
> move_iterator<_BidirectionalIterator>(__middle),
> > - move_iterator<_BidirectionalIterator>(__last),
> > - __first, __comp);
> > + __half_inplace_merge(__buff, __p, __middle, __last, __first,
> __comp);
> > }
> > else
> > {
> > @@ -4389,9 +4413,9 @@ __buffered_inplace_merge(_BidirectionalI
> > ::new(__p) value_type(_VSTD::move(*__i));
> > typedef reverse_iterator<_BidirectionalIterator> _RBi;
> > typedef reverse_iterator<value_type*> _Rv;
> > - __merge(move_iterator<_RBi>(_RBi(__middle)),
> move_iterator<_RBi>(_RBi(__first)),
> > - move_iterator<_Rv>(_Rv(__p)),
> move_iterator<_Rv>(_Rv(__buff)),
> > - _RBi(__last), __negate<_Compare>(__comp));
> > + __half_inplace_merge(_Rv(__p), _Rv(__buff),
> > + _RBi(__middle), _RBi(__first),
> > + _RBi(__last), __negate<_Compare>(__comp));
> > }
> > }
> >
> >
> > Modified:
> libcxx/trunk/test/std/algorithms/alg.sorting/alg.merge/inplace_merge.pass.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/algorithms/alg.sorting/alg.merge/inplace_merge.pass.cpp?rev=243530&r1=243529&r2=243530&view=diff
> >
> ==============================================================================
> > ---
> libcxx/trunk/test/std/algorithms/alg.sorting/alg.merge/inplace_merge.pass.cpp
> (original)
> > +++
> libcxx/trunk/test/std/algorithms/alg.sorting/alg.merge/inplace_merge.pass.cpp
> Wed Jul 29 11:25:45 2015
> > @@ -20,12 +20,35 @@
> >
> > #include "test_iterators.h"
> >
> > +#ifndef TEST_STD_VER >= 11
>
> Should this be `#if`?
>
Does clang have a warning for this?
-- Sean Silva
>
> > +struct S {
> > + S() : i_(0) {}
> > + S(int i) : i_(i) {}
> > +
> > + S(const S& rhs) : i_(rhs.i_) {}
> > + S( S&& rhs) : i_(rhs.i_) { rhs.i_ = -1; }
> > +
> > + S& operator =(const S& rhs) { i_ = rhs.i_; return
> *this; }
> > + S& operator =( S&& rhs) { i_ = rhs.i_; rhs.i_ = -2;
> assert(this != &rhs); return *this; }
> > + S& operator =(int i) { i_ = i; return
> *this; }
> > +
> > + bool operator <(const S& rhs) const { return i_ < rhs.i_; }
> > + bool operator ==(const S& rhs) const { return i_ == rhs.i_; }
> > + bool operator ==(int i) const { return i_ == i; }
> > +
> > + void set(int i) { i_ = i; }
> > +
> > + int i_;
> > + };
> > +#endif
> > +
> > template <class Iter>
> > void
> > test_one(unsigned N, unsigned M)
> > {
> > + typedef typename std::iterator_traits<Iter>::value_type value_type;
> > assert(M <= N);
> > - int* ia = new int[N];
> > + value_type* ia = new value_type[N];
> > for (unsigned i = 0; i < N; ++i)
> > ia[i] = i;
> > std::random_shuffle(ia, ia+N);
> > @@ -76,4 +99,10 @@ int main()
> > test<bidirectional_iterator<int*> >();
> > test<random_access_iterator<int*> >();
> > test<int*>();
> > +
> > +#ifndef TEST_STD_VER >= 11
>
> #if?
>
> > + test<bidirectional_iterator<S*> >();
> > + test<random_access_iterator<S*> >();
> > + test<S*>();
> > +#endif // TEST_STD_VER >= 11
> > }
> >
> > Modified:
> libcxx/trunk/test/std/algorithms/alg.sorting/alg.merge/inplace_merge_comp.pass.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/algorithms/alg.sorting/alg.merge/inplace_merge_comp.pass.cpp?rev=243530&r1=243529&r2=243530&view=diff
> >
> ==============================================================================
> > ---
> libcxx/trunk/test/std/algorithms/alg.sorting/alg.merge/inplace_merge_comp.pass.cpp
> (original)
> > +++
> libcxx/trunk/test/std/algorithms/alg.sorting/alg.merge/inplace_merge_comp.pass.cpp
> Wed Jul 29 11:25:45 2015
> > @@ -18,7 +18,10 @@
> > #include <algorithm>
> > #include <functional>
> > #include <cassert>
> > -#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
> > +
> > +#include "test_macros.h"
> > +
> > +#ifndef TEST_STD_VER >= 11
> > #include <memory>
> >
> > struct indirect_less
> > @@ -28,7 +31,29 @@ struct indirect_less
> > {return *x < *y;}
> > };
> >
> > -#endif // _LIBCPP_HAS_NO_RVALUE_REFERENCES
> > +struct S {
> > + S() : i_(0) {}
> > + S(int i) : i_(i) {}
> > +
> > + S(const S& rhs) : i_(rhs.i_) {}
> > + S( S&& rhs) : i_(rhs.i_) { rhs.i_ = -1; }
> > +
> > + S& operator =(const S& rhs) { i_ = rhs.i_; return
> *this; }
> > + S& operator =( S&& rhs) { i_ = rhs.i_; rhs.i_ = -2;
> assert(this != &rhs); return *this; }
> > + S& operator =(int i) { i_ = i; return
> *this; }
> > +
> > + bool operator <(const S& rhs) const { return i_ < rhs.i_; }
> > + bool operator >(const S& rhs) const { return i_ > rhs.i_; }
> > + bool operator ==(const S& rhs) const { return i_ == rhs.i_; }
> > + bool operator ==(int i) const { return i_ == i; }
> > +
> > + void set(int i) { i_ = i; }
> > +
> > + int i_;
> > + };
> > +
> > +
> > +#endif // TEST_STD_VER >= 11
> >
> > #include "test_iterators.h"
> > #include "counting_predicates.hpp"
> > @@ -38,19 +63,20 @@ void
> > test_one(unsigned N, unsigned M)
> > {
> > assert(M <= N);
> > - int* ia = new int[N];
> > + typedef typename std::iterator_traits<Iter>::value_type value_type;
> > + value_type* ia = new value_type[N];
> > for (unsigned i = 0; i < N; ++i)
> > ia[i] = i;
> > std::random_shuffle(ia, ia+N);
> > - std::sort(ia, ia+M, std::greater<int>());
> > - std::sort(ia+M, ia+N, std::greater<int>());
> > - binary_counting_predicate<std::greater<int>, int, int>
> pred((std::greater<int>()));
> > + std::sort(ia, ia+M, std::greater<value_type>());
> > + std::sort(ia+M, ia+N, std::greater<value_type>());
> > + binary_counting_predicate<std::greater<value_type>, value_type,
> value_type> pred((std::greater<value_type>()));
> > std::inplace_merge(Iter(ia), Iter(ia+M), Iter(ia+N), std::ref(pred));
> > if(N > 0)
> > {
> > assert(ia[0] == N-1);
> > assert(ia[N-1] == 0);
> > - assert(std::is_sorted(ia, ia+N, std::greater<int>()));
> > + assert(std::is_sorted(ia, ia+N, std::greater<value_type>()));
> > assert(pred.count() <= (N-1));
> > }
> > delete [] ia;
> > @@ -93,7 +119,11 @@ int main()
> > test<random_access_iterator<int*> >();
> > test<int*>();
> >
> > -#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
> > +#ifndef TEST_STD_VER >= 11
>
> #if?
>
> > + test<bidirectional_iterator<S*> >();
> > + test<random_access_iterator<S*> >();
> > + test<S*>();
> > +
> > {
> > unsigned N = 100;
> > unsigned M = 50;
> > @@ -112,5 +142,5 @@ int main()
> > }
> > delete [] ia;
> > }
> > -#endif // _LIBCPP_HAS_NO_RVALUE_REFERENCES
> > +#endif // TEST_STD_VER >= 11
> > }
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150730/bd9ec483/attachment.html>
More information about the cfe-commits
mailing list