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

[Yurong via Phabricator via libcxx-commits]

yronglin added a comment.

Thanks you for review @philnik @huixie90 , It is an great honor to contribute to libcxx, and I'm a newbie in libcxx. I don't know how to use `direct-non-list-initialized` to initialize named union member in `expected` and [[no_unique_address]] attribute cannot be used on anonymous union, so, I change to use anonymous in `expected`. You are all experts in libcxx, if you have any good ideas, please help me, many thanks!



================
Comment at: libcxx/include/__expected/expected.h:75
   (void)__arg;
-  std::abort();
+  _VSTD::abort();
 #  endif
----------------
huixie90 wrote:
> why all `std`s are changed to `_VSTD`? I thought we are always using `std` now
I have refer to the comments in https://reviews.llvm.org/D113408 , 
```
Note that this qualification of _VSTD::move is good because it protects against ADL. My comment above about qualifications do not apply to cases that prevent ADL from being triggered.
```
So, I understand it is better to use _VSTD? I don't know if I got it wrong, if so I would undo these changes.


================
Comment at: libcxx/include/__expected/expected.h:867-871
+   union {
+     __empty_t __empty_;
+     _Tp __val_;
+     _Err __unex_;
+  };
----------------
huixie90 wrote:
> I am not sure I understand this change. is this due to a wrong rebase or something? the purpose of `__empty_t` and the "named" union was to be able to apply `_LIBCPP_NO_UNIQUE_ADDRESS`
I removed [[no_unique_address]] attribute because I ran into a trouble.
in P2505R5, for the && and const&& overload of transform, the full wording of the paper is:
```
template<class F> constexpr auto transform(F&& f) &&;
template<class F> constexpr auto transform(F&& f) const &&;

Let U be remove_­cv_­t<invoke_­result_­t<F, decltype(std​::​move(value()))>>.

Constraints: is_­move_­constructible_­v<E> is true.

Mandates: U is a valid value type for expected.
If is_­void_­v<U> is false, the declaration U u(invoke(std::forward<F>(f), std::move(value())));
is well-formed for some invented variable u.

Effects:

    (24.1)
    If has_­value() is false, returns expected<U, E>(unexpect, std​::​move(error())).
    (24.2)
    Otherwise, if is_­void_­v<U> is false, returns an expected<U, E> object whose has_­val member is true and val member is `direct-non-list-initialized` with invoke(std​::​forward<F>(f), std​::​move(value())).
    (24.3)
    Otherwise, evaluates invoke(std​::​forward<F>(f), std​::​move(value())) and then returns expected<U, E>().

```
If we use named union, I have no idea to deal with 24.2:
```
 Otherwise, if is_­void_­v<U> is false, returns an expected<U, E> object whose has_­val member is true and val member is `direct-non-list-initialized` with invoke(std​::​forward<F>(f), std​::​move(value())).
```
In https://reviews.llvm.org/D124516 I learned that [[no_unique_address]] cannot be used on anonymous unions, so I removed this attribute and changed named unions to anonymous unions.

If use anonymous union, I can implement the `direct-non-list initialized` sematic:
```
  template <class _Func, class... _Args>
  constexpr explicit expected(__expected::__expected_construct_in_place_from_invoke_tag, _Func&& __f, _Args&&... __args)
      : __val_(_VSTD::invoke(_VSTD::forward<_Func>(__f), _VSTD::forward<_Args>(__args)...)), __has_val_(true) {}
```

The test case:
```
constexpr void test_xval() {
  struct NonCopyable {
    constexpr NonCopyable(int i) : i(i) {}
    NonCopyable(const NonCopyable&) = delete;
    int i;
  };

  auto xform = [](int i) { return NonCopyable(i); };
  std::expected<int, int> e2(2);
  std::expected<NonCopyable, int> n2 = e2.transform(xform);
  assert(n2.value().i == 2);
}

```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140911