[libcxx-commits] [PATCH] D102809: [libcxx][ranges] Add `ranges::iter_swap`.

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/include/__iterator/iter_swap.h:75-76
+                !__readable_swappable<_T1, _T2>) &&
+               indirectly_movable_storable<_T1, _T2> &&
+               indirectly_movable_storable<_T2, _T1>
+    constexpr void operator()(_T1&& __x, _T2&& __y) const
----------------
Quuxplusone wrote:
> zoecarver wrote:
> > Quuxplusone wrote:
> > > tcanens wrote:
> > > > zoecarver wrote:
> > > > > Quuxplusone wrote:
> > > > > > This should say `indirectly_movable_storable<_T1&&, _T2&&>`, right? because the type of `E1` is not `_T1` but rather `_T1&&`?
> > > > > > Can you find a test case that demonstrates the difference, and add it as a regression test?
> > > > > > This comment //probably// applies equally to all your uses of exposition-only helpers such as `__readable_swappable`, but I'm even less confident that that difference will be detectable by a test.
> > > > > This is my bad for naming these `_T1` and `_T2` so when I read the standard wording, my brain thought it meant to just use the template params (which don't exist in the wording). I think you're right. 
> > > > The type of an expression is never a reference type.
> > > > The type of an expression is never a reference type.
> > > 
> > > So all of these trait checks should be using e.g. `indirectly_movable_storable<remove_reference_t<_T1>, remove_reference_t<_T2>>`? Blech. OK.
> > > So all of these trait checks should be using e.g. indirectly_movable_storable<remove_reference_t<_T1>, remove_reference_t<_T2>>? Blech. OK.
> > 
> > I think they can actually just be `_T1` and `_T2` (no `remove_reference_t`), right? 
> @tcanens said "the type of an expression is never a reference type," so no, you'd have to remove the reference first (e.g. if `_T1` is `int&`, then we should apply the trait to `int`, not to `int&`).
I'm not sure I have the correct words to describe what I'm thinking here but won't the `&&` in `_T1&&` "eat" the reference part of the reference type? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102809