[libcxx-commits] [PATCH] D132268: [libc++][spaceship] Implement `operator<=>` for `vector`

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D132268#4324117 <https://reviews.llvm.org/D132268#4324117>, @H-G-Hristov wrote:

> In D132268#4322525 <https://reviews.llvm.org/D132268#4322525>, @philnik wrote:
>
>> IIUC the problem is that `__bit_iterator::operator*` returns an object instead of a reference, which can't bind to an lvalue reference. If we use perfect forwarding that should fix the problem?
>
> Could you please confirm if I understand correctly: I am supposed to perfectly forward the arguments to `std::__lexicographical_compare_three_way_fast_path` something like that:
>
>   return std::__lexicographical_compare_three_way_fast_path(
>       std::forward<_InputIterator1>(__first1),
>       std::forward<_InputIterator1>(__last1),
>       std::forward<_InputIterator2>(__first2),
>       std::forward<_InputIterator2>(__last2),
>       __wrapped_comp_ref);
>
> No, that still wasn't enough. The compilation fails without changes to `LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Tp& __x, _Up& __y)`'s signature. There was ways an issue with the 5th parameter (for the record I tried `forward`, `move`).
> In the current revision I changed all references to rvalue references and applied forward to the iterators . I admit that I am not quite sure if that's the correct approach.

I meant that you should perfect-forward the arguments in `__debug_three_way_comp::operator()`. i.e. `std::forward` the arguments to `__comp_`. The `std::forward`s you are currently doing in the calls are essentially just `std::move`s, since the arguments are prvalue types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132268