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

Zoe Carver via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jun 15 06:19:30 PDT 2021


zoecarver added inline comments.


================
Comment at: libcxx/include/__iterator/iter_swap.h:42
+    indirectly_readable<_T1> && indirectly_readable<_T2> &&
+    swappable_with<decltype(*declval<_T1>()), decltype(*declval<_T2>())>;
+
----------------
ldionne wrote:
> I think it would be more in-line with what we usually do to say `swappable_with<iter_reference_t<_T1>, iter_reference_t<_T2>>` instead. WDYT?
> 
> As written, I think this is slightly incorrect (but I'm not sure it's possible to notice the difference). The Standard talks about the "reference type of `E1` and `E2`", which is `*declval<_T1&&>()`, not `*declval<_T1>()`. But regardless, I think we can just switch to `iter_reference_t<_T1>` unless there's a problem with that I'm not seeing.
Hmm, trying to come up with an example where this doesn't work. I think it's actually 100% valid because `indirectly_readable` enforces that `*in == iter_reference_t`:
```
 { *in } -> std::same_as<std::iter_reference_t<In>>;
```
So, I'll do that.


================
Comment at: libcxx/include/__iterator/iter_swap.h:68-70
+      noexcept(noexcept(iter_value_t<_T2>(ranges::iter_move(__y))) &&
+               noexcept(*__y = ranges::iter_move(__x)) &&
+               noexcept(*_VSTD::forward<_T1>(__x) = declval<iter_value_t<_T2>>()))
----------------
ldionne wrote:
> I don't see how to do better, but it's awful to have to duplicate the body like that.
I agree. We should really have a `noexcept(auto)`. This wouldn't be too hard to add to clang, and then we could just let GCC users have worse codegen...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102809



More information about the libcxx-commits mailing list