[libcxx-commits] [PATCH] D115658: [libc++] Fix the noexceptness of __decay_copy.

Mon Dec 13 12:22:36 PST 2021

ldionne added inline comments.

================
Comment at: libcxx/include/__utility/decay_copy.h:27
 #if _LIBCPP_STD_VER > 17
-    noexcept(is_nothrow_convertible_v<_Tp, remove_reference_t<_Tp>>)
+    noexcept(is_nothrow_convertible_v<_Tp, decay_t<_Tp>>)
 #endif
----------------
Quuxplusone wrote:
> Actually, it occurs to me that several things are probably wrong here and we should just fix them all at once.
> - We don't care about //convertible// but rather //constructible//.
> - We should be returning `decay_t<_Tp>(static_cast<_Tp&&>(__t))`, not simply `static_cast<_Tp&&>(__t)`.
> - We should be noexcept'ing on the same expression.
> - We should just go ahead and make this a full "write it three times" function, so that `__decay_copy(mymutex)` SFINAEs away instead of pretending it exists and then hard-erroring during instantiation.
> - In C++23, //DECAY_COPY// is changing over to `auto(x)`, which doesn't call move constructors when you pass it a prvalue, so we'll need something else anyway. How about we just make a macro like this?
> 
> ```
> #if PRESENT_DAY
>  #define _LIBCPP_DECAY_COPY(x) static_cast<typename decay<decltype((x))>::type>(x)
> #else
>  #define _LIBCPP_DECAY_COPY(x) auto(x)
> #endif
> ```
> and replace all our usages throughout the codebase right now?
> 
> I have a vested interest because D115607 adds a //ton// of `__decay_copy` calls. :)
http://eel.is/c++draft/expos.only.func#lib:decay-copy is pretty clear on what it's doing. The change from convertible-to to constructible-from seems like a functional change to me.

Is this function somehow not implementing the exposition-only function I linked? If not, you know what I'm going to say. It might be boring, but we're not here to change the design of the standard, we're here to implement it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115658