[libcxx-commits] [PATCH] D154116: [libc++] Implement LWG3938 (Cannot use std::expected monadic ops with move-only error_type)

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Thanks for working on this! A few comments.



================
Comment at: libcxx/docs/ReleaseNotes.rst:50
 - P2572R1 - ``std::format`` fill character allowances
+- LWG3938 - Cannot use ``std::expected`` monadic ops with move-only error_type
 
----------------
We typically don't mention LWG issues in our release notes, unless they have a huge impact.


================
Comment at: libcxx/include/__expected/expected.h:694
   template <class _Func>
     requires is_constructible_v<_Tp, _Tp&>
   _LIBCPP_HIDE_FROM_ABI constexpr auto or_else(_Func&& __f) & {
----------------
Can you update this to the Standard wording too? the same for the other places.


================
Comment at: libcxx/include/__expected/expected.h:750
   _LIBCPP_HIDE_FROM_ABI constexpr auto transform(_Func&& __f) & {
     using _Up = remove_cv_t<invoke_result_t<_Func, _Tp&>>;
     if (!has_value()) {
----------------
Please update this one too.


================
Comment at: libcxx/test/std/utilities/expected/expected.expected/monadic/and_then.pass.cpp:7
 //
 //===----------------------------------------------------------------------===//
 
----------------
Please update the `or_else` and `transform` test too.


================
Comment at: libcxx/test/std/utilities/expected/expected.expected/monadic/and_then.pass.cpp:142
+  NonCopyable(const NonCopyable&&) {}
 };
 
----------------
Can you make a separate MoveOnly class instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154116