[libcxx] r243530 - Fix a self-move bug in inplace_merge. Thanks to Ted and Dexon for the report and the suggested fix.

Duncan P. N. Exon Smith dexonsmith at apple.com
Thu Jul 30 11:54:36 PDT 2015


> On 2015-Jul-30, at 10:10, Sean Silva <chisophugis at gmail.com> wrote:
> 
> 
> 
> 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

Yes.  I spotted it while running the tests locally.





More information about the cfe-commits mailing list