[libcxx-commits] [PATCH] D130197: [libc++] Fix `_IterOps::__iter_move` to support proxy iterators.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const created this revision.
Herald added a subscriber: mgrang.
Herald added a project: All.
var-const requested review of this revision.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130197

Files:
  libcxx/include/__algorithm/iterator_operations.h
  libcxx/test/std/algorithms/alg.sorting/alg.sort/sort/sort_proxy.pass.cpp


Index: libcxx/test/std/algorithms/alg.sorting/alg.sort/sort/sort_proxy.pass.cpp
===================================================================
--- /dev/null
+++ libcxx/test/std/algorithms/alg.sorting/alg.sort/sort/sort_proxy.pass.cpp
@@ -0,0 +1,26 @@
+//===----------------------------------------------------------------------===//
+//
+// 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;
+}
Index: libcxx/include/__algorithm/iterator_operations.h
===================================================================
--- libcxx/include/__algorithm/iterator_operations.h
+++ libcxx/include/__algorithm/iterator_operations.h
@@ -67,7 +67,11 @@
   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) {
+  // Note that for proxy iterators, `reference` might be different from `value_type&` -- returning `value_type&&` might
+  // result in a dangling temporary in that case.
+  static typename remove_reference<
+      typename iterator_traits<__uncvref_t<_Iter> >::reference
+  >::type&& __iter_move(_Iter&& __i) {
     return std::move(*std::forward<_Iter>(__i));
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130197.446241.patch
Type: text/x-patch
Size: 1927 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20220720/f92a2d6a/attachment.bin>