[libcxx-commits] [PATCH] D140911: [libc++] Implement P2505R5(Monadic operations for std::expected).

[Yurong via Phabricator via libcxx-commits]

yronglin marked 2 inline comments as done.
yronglin added a comment.

Thanks for your review @ldionne !



================
Comment at: libcxx/include/__expected/expected.h:641
+  template <class _Func>
+    requires is_copy_constructible_v<_Err>
+  _LIBCPP_HIDE_FROM_ABI constexpr auto and_then(_Func&& __f) & {
----------------
ldionne wrote:
> I don't quite understand why https://eel.is/c++draft/expected#object.monadic-2 says `is_constructible_v<E, decltype(error())>` but we're using `is_copy_constructible_v<_Err>`. I don't think those are equivalent, right? For example `error()` will return a non-const reference for this `&`-qualified function, but we're checking that `is_constructible_v<E, E const&>` instead of `is_constructible_v<E, E&>`? This is subtle, but if I'm right, we should add a test for this and then correct all the overloads below.
Wow, good catch! Seems this change maked by https://cplusplus.github.io/LWG/issue3877 , I'will apply this change.
https://github.com/cplusplus/draft/commit/1e278301ff6a1fe940aa72f6691f9dc79e06fb19


================
Comment at: libcxx/include/__expected/expected.h:644-645
+    using _Up = remove_cvref_t<invoke_result_t<_Func, _Tp&>>;
+    static_assert(
+        std::__is_std_expected<_Up>::value, "The result of f(value()) must be a specialization of std::expected");
+    static_assert(is_same_v<typename _Up::error_type, _Err>,
----------------
ldionne wrote:
> Here and below, qualification for this isn't required. We qualify function calls to avoid ADL, but this is not a function call.
Thanks, I'll remove that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140911