std::set_union accesses values after move.

[dyaroshev]

Summary

It's not ok to compare after assignment, it should be comare first and then assignment.

bug: https://bugs.llvm.org/show_bug.cgi?id=35119

[dyaroshev]

I forgot the synopsis.

[mclow.lists]

If you start playing with move_iterators, I suspect you'll find several of the algorithms don't work quite right.

This looks good to me.

[mclow.lists]

In general, you should add cfe-commits as a subscriber to any libc++ patch you propose.

[mclow.lists]

Committed as revision 316914. Thanks!

[EricWF]

We should backport this, right @mclow.lists?

include/algorithm

for (; __first1 != __last1; ++__result)
             return _VSTD::copy(__first1, __last1, __result);
         if (__comp(*__first2, *__first1))
         {
             *__result = *__first2;
             ++__first2;
         }
         else
         {
-            *__result = *__first1;
             if (!__comp(*__first1, *__first2))
                 ++__first2;
+            *__result = *__first1;
             ++__first1;
         }
     }
     return _VSTD::copy(__first2, __last2, __result);
 }
 
 template <class _InputIterator1, class _InputIterator2, class _OutputIterator, class _Compare>
 inline _LIBCPP_INLINE_VISIBILITY

test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// <algorithm>
+
+// template<InputIterator InIter1, InputIterator InIter2, typename OutIter,
+//          CopyConstructible Compare>
+//   requires OutputIterator<OutIter, InIter1::reference>
+//         && OutputIterator<OutIter, InIter2::reference>
+//         && Predicate<Compare, InIter1::value_type, InIter2::value_type>
+//         && Predicate<Compare, InIter2::value_type, InIter1::value_type>
+//   OutIter
+//   set_union(InIter1 first1, InIter1 last1, InIter2 first2, InIter2 last2,
+//             OutIter result, Compare comp);
+
+#include <algorithm>
+#include <cassert>
+#include <iterator>
+#include <vector>
+
+#include "MoveOnly.h"
+
+
+int main()
+{
+    std::vector<MoveOnly> lhs, rhs;
+    lhs.push_back(MoveOnly(2));
+    rhs.push_back(MoveOnly(2));
+
+    std::vector<MoveOnly> res;
+    std::set_union(std::make_move_iterator(lhs.begin()),
+                   std::make_move_iterator(lhs.end()),
+                   std::make_move_iterator(rhs.begin()),
+                   std::make_move_iterator(rhs.end()), std::back_inserter(res));
+
+    assert(res.size() == 1);
+    assert(res[0].get() == 2);
+}