[libcxx-commits] [PATCH] D130538: [libc++] Make `_IterOps::__iter_move` more similar to `std::ranges::iter_move`.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const marked 2 inline comments as done.
var-const added inline comments.


================
Comment at: libcxx/include/__algorithm/iterator_operations.h:85-86
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11 static
+  // Declaring the return type is necessary for C++03.
+  // If the result of dereferencing `_Iter` is a reference type, deduce the result of calling `std::move` on it.
+  __enable_if_t<
----------------
huixie90 wrote:
> nit: i think the comments no long apply anymore. The return type also has sfinae logic in it
Rephrased the comment to make it clearer that we can't use `decltype(auto)` or `auto` as the return type, regardless of the SFINAE.


================
Comment at: libcxx/include/__algorithm/iterator_operations.h:93
+
     return std::move(*std::forward<_Iter>(__i));
   }
----------------
huixie90 wrote:
> extra nit: perhaps we don't need to forward it, because the `__deref_t` and `_move_t` are defined in terms of lvalue of `_Iter`
Discussed offline -- this makes it closer to how `iter_reference_t` and `iter_move` are defined in C++20, and there doesn't seem to be an observable difference.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130538/new/

https://reviews.llvm.org/D130538