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

[Joe Loser via Phabricator via libcxx-commits]

jloser marked 2 inline comments as done.
jloser added inline comments.


================
Comment at: libcxx/include/utility:187
+    T exchange(T& obj, U&& new_value)
+      noexcept(is_nothrow_move_constructible<T>::value && is_nothrow_assignable<T&, U>::value); // constexpr in C++17
 
----------------
Mordante wrote:
> Please add the `constexpr` in the signature.
> 
> Please add `noexcept in C++23`. This only is required in C++23. Since it's allowed to strengthen `noexcept` I don't mind it available in earlier versions.
Added both `constexpr` and `noexcept`.


================
Comment at: libcxx/test/std/utilities/utility/exchange/exchange.pass.cpp:110
 
+    test_noexcept();
+    static_assert(test_noexcept());
----------------
Mordante wrote:
> jloser wrote:
> > Mordante wrote:
> > > Maybe move this to line 105, so the `static_assert` test can be guarded by the `TEST_STD_VER` at line 106.
> > The above `TEST_STD_VER` is `> 17` but since this `noexcept` goes back to C++14 (when `std::exchange` was introduced), I think it's best to leave it here.
> I see the test executes no code, otherwise it couldn't execute the test in `constexpr` until C++20. `ASSERT_NOEXCEPT` is just a `static_assert`. So there's no real need to test this function twice.
Valid point -- just removed the runtime function call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111481