[libcxx-commits] [PATCH] D111481: [libc++] P2401: conditional noexcept for std::exchange

[Mark de Wever via Phabricator via libcxx-commits]

Mordante requested changes to this revision.
Mordante added a comment.
This revision now requires changes to proceed.

Thanks for working on this! It seems you use C++17 only code in C++14 mode.



================
Comment at: libcxx/include/__utility/exchange.h:26
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
-_T1 exchange(_T1& __obj, _T2 && __new_value)
+_T1 exchange(_T1& __obj, _T2 && __new_value) noexcept(is_nothrow_move_constructible_v<_T1> && is_nothrow_assignable_v<_T1&, _T2>)
 {
----------------
`is_nothrow_move_constructible_v` requires C++17.


================
Comment at: libcxx/test/std/utilities/utility/exchange/exchange.pass.cpp:16
 //    constexpr T            // constexpr after C++17
 //    exchange(T& obj, U&& new_value);
 
----------------
Please update the synopsis. Here and in the `<utility>` header.


================
Comment at: libcxx/test/std/utilities/utility/exchange/exchange.pass.cpp:54
+
+  TestExchangeNoexcept<true, false> nothrow_move_constructible{};
+  static_assert(std::is_nothrow_move_constructible_v<decltype(nothrow_move_constructible)>);
----------------
Can you make a typedef for this type so the `decltype`s below aren't required.


================
Comment at: libcxx/test/std/utilities/utility/exchange/exchange.pass.cpp:110
 
+    test_noexcept();
+    static_assert(test_noexcept());
----------------
Maybe move this to line 105, so the `static_assert` test can be guarded by the `TEST_STD_VER` at line 106.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111481