[libc++] Fix `_IterOps::__iter_move` to support proxy iterators.

[var-const]

Summary

The return type was specified incorrectly for proxy iterators that
define reference to be a class that implicitly converts to
value_type. __iter_move would end up returning an object of type
reference which would then implicitly convert to value_type; thus,
the function will return a value_type&& rvalue reference to the local
temporary.

[ldionne]

LGTM, please ship this if it passes locally under --param std=X for all standards.

libcxx/include/__algorithm/iterator_operations.h

68–73

In particular, I think the comment about dangling references is not super useful here since we have a test for that.

[var-const]

Feedback.

[var-const]

Feedback.

[var-const]

Fix the test in C++03 mode.

[huixie90]

libcxx/include/__algorithm/iterator_operations.h

70–73

I am not sure if this solves the problem.

std::vector<bool>::iterator::operator* returns a prvalue Proxy. Even the return type is now Proxy&&, it is still a dangling reference to a local temporary Proxy created by operator*

I think we might have to return by value without std::move if the reference type is a prvalue. (similar to c++17 move_iterator https://timsong-cpp.github.io/cppwp/n4659/move.iterators#move.iterator-1)

libcxx/include/__algorithm/iterator_operations.h

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
struct _IterOps<_ClassicAlgPolicy> {
   template <class _Iter>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
   static typename iterator_traits<_Iter>::difference_type distance(_Iter __first, _Iter __last) {
     return std::distance(__first, __last);
   }
 
   // iter_move
   template <class _Iter>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
-  // Declaring the return type is necessary for the C++03 mode (which doesn't support placeholder return types).
-  static typename iterator_traits<__uncvref_t<_Iter> >::value_type&& __iter_move(_Iter&& __i) {
+  // Declaring the return type is necessary for C++03, so we basically mirror what `decltype(auto)` would deduce.
+  static typename remove_reference<
+      typename iterator_traits<__uncvref_t<_Iter> >::reference
+  >::type&& __iter_move(_Iter&& __i) {
     return std::move(*std::forward<_Iter>(__i));
   }
 
   // iter_swap
   template <class _Iter1, class _Iter2>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
   static void iter_swap(_Iter1&& __a, _Iter2&& __b) {
     std::iter_swap(std::forward<_Iter1>(__a), std::forward<_Iter2>(__b));
   }

libcxx/test/std/algorithms/alg.sorting/alg.sort/sort/sort_proxy.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <algorithm>
+
+#include <algorithm>
+#include <cassert>
+#include <vector>
+
+void test() {
+  // TODO: use a custom proxy iterator instead of (or in addition to) `vector<bool>`.
+  std::vector<bool> v = {true, false, true, false, true};
+  std::sort(v.begin(), v.end());
+  assert(std::is_sorted(v.begin(), v.end()));
+}
+
+int main(int, char**) {
+  test();
+
+  return 0;
+}