[libcxx-commits] [PATCH] D99873: [libcxx] adds `std::ranges::iter_move` and `std::iter_rvalue_reference_t`

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added inline comments.


================
Comment at: libcxx/include/__iterator/iter_move.h:59
+  constexpr decltype(auto) operator()(_Ip&& __i) const
+    noexcept(noexcept(*_VSTD::forward<_Ip>(__i)))
+  {
----------------
ldionne wrote:
> cjdb wrote:
> > Quuxplusone wrote:
> > > cjdb wrote:
> > > > This noexcept specifier doesn't account for a throwing move.
> > > "`move` doesn't move," and this function returns `decltype(auto)`, so I think we're okay here. This function either returns an rvalue reference in the `if`, or URVOs a prvalue in the `else`.
> > @rsmith is the one who pointed it out to me, so I'm not so sure. Granted, the tests cover seem to be passing, but a looking over them again makes me realise I only tested noexcept specifiers in one direction. We'll need to plug those holes at the very least.
> > This function either returns an rvalue reference in the if, or URVOs a prvalue in the else.
> 
> That was my reasoning when writing that.
> 
> > but a looking over them again makes me realise I only tested noexcept specifiers in one direction. We'll need to plug those holes at the very least.
> 
> Sorry, I'm not quite following, what do you mean by testing `noexcept` specifiers only in one direction? I'll add the test coverage, but I'd like to understand what's missing.
> 
The tests in this commit exclusively assume `noexcept(std::ranges::iter_move(lvalue)) == false` and `noexcept(std::ranges::iter_move(rvalue)) == true`. That was an oversight in my design testing (and what I get for not knowing how to do TDD properly in 2019).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99873