[libcxx-commits] [PATCH] D124516: [libc++] Implement `std::expected` P0323R12

[Hui via Phabricator via libcxx-commits]

huixie90 added inline comments.


================
Comment at: libcxx/include/__expected/expected.h:630-633
+  union {
+    _Tp __val_;
+    _Err __unex_;
+  };
----------------
@jwakely Hi Jon, I have noticed that in libstdc++, there is a place holder type as the first type in the union
https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/std/expected#L893

I am trying to understand the purpose of that type. At the first glance, I thought it is for the case where `T` is not default constructible and we need that type in case we cannot initialise `T` directly in the `expected`'s constructor's initializer lists. 
It turns out that I was wrong and it is possible to have the union uninitialised even if `T` is not default constructible.
https://godbolt.org/z/9xePvrE4x

So I believe libstdc++ has that dummy type for other reasons. Could you please shed some lights on this?


================
Comment at: libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.u.pass.cpp:115-125
+  // this is a confusing example, but the behaviour
+  // is exactly what is specified in the spec
+  {
+    struct BaseError {};
+    struct DerivedError : BaseError {};
+
+    std::expected<bool, BaseError> e1(false);
----------------
@jwakely Hi Jon, thank you very much for commenting on this patch and they are really helpful. While you are here, what do you think about this test case? If I switch `bool` to `int`, 
```
std::expected<int, BaseError> e1(5);
std::expected<int, DerivedError> e2(e1);
```

the other ctor overload

```
expected(const expected<U, G>&)
```
will be selected and the `int` value `5` will be copied. 

But for `bool`, as `T` (`bool`) is constructible from the `expected`, the other overload will be disabled and this overload will be selected
```
expected(U&&);
```
so instead of copying the `bool` (`false`), it uses `operator bool`, which is `true` this case.

Do you think this behaviour is confusing and not very consistent


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124516