[libcxx-commits] [libcxx] [libc++] Fix UB in <expected> related to "has value" flag (#68552) (PR #68733)

[Jan Kokemüller via libcxx-commits]

================
@@ -136,14 +134,7 @@ class expected {
     noexcept(is_nothrow_copy_constructible_v<_Tp> && is_nothrow_copy_constructible_v<_Err>) // strengthened
     requires(is_copy_constructible_v<_Tp> && is_copy_constructible_v<_Err> &&
              !(is_trivially_copy_constructible_v<_Tp> && is_trivially_copy_constructible_v<_Err>))
-      : __has_val_(__other.__has_val_) {
-    if (__has_val_) {
-      std::construct_at(std::addressof(__union_.__val_), __other.__union_.__val_);
-    } else {
-      std::construct_at(std::addressof(__union_.__unex_), __other.__union_.__unex_);
-    }
-  }
-
+      : __union_(__union_from_expected(__other)), __has_val_(__other.__has_val_) { }
----------------
jiixyj wrote:

It turns out that GCC 13 didn't like this approach:

```txt
_bk;t=1697372498369# | In file included from /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-5c70ceab5f4e-1/llvm-project/libcxx-ci/build/generic-gcc/include/c++/v1/expected:44,
_bk;t=1697372498369# |                  from /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-5c70ceab5f4e-1/llvm-project/libcxx-ci/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.copy.pass.cpp:28:
_bk;t=1697372498369# | /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-5c70ceab5f4e-1/llvm-project/libcxx-ci/build/generic-gcc/include/c++/v1/__expected/expected.h: In instantiation of ‘constexpr std::__1::expected<_Tp, _Err>::expected(const std::__1::expected<_Tp, _Err>&) requires (is_copy_constructible_v<_Tp>) && (is_copy_constructible_v<_Err>) && !((is_trivially_copy_constructible_v<_Tp>) && (is_trivially_copy_constructible_v<_Err>)) [with _Tp = testException()::Throwing; _Err = int]’:
_bk;t=1697372498369# | /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-5c70ceab5f4e-1/llvm-project/libcxx-ci/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.copy.pass.cpp:112:34:   required from here
_bk;t=1697372498369# | /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-5c70ceab5f4e-1/llvm-project/libcxx-ci/build/generic-gcc/include/c++/v1/__expected/expected.h:137:9: error: use of deleted function ‘std::__1::expected<testException()::Throwing, int>::__union_t<testException()::Throwing, int>::__union_t(const std::__1::expected<testException()::Throwing, int>::__union_t<testException()::Throwing, int>&)’
_bk;t=1697372498369# |   137 |       : __union_(__union_from_expected(__other)), __has_val_(__other.__has_val_) { }
_bk;t=1697372498369# |       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
_bk;t=1697372498369# | /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-5c70ceab5f4e-1/llvm-project/libcxx-ci/build/generic-gcc/include/c++/v1/__expected/expected.h:860:9: note: ‘std::__1::expected<testException()::Throwing, int>::__union_t<testException()::Throwing, int>::__union_t(const std::__1::expected<testException()::Throwing, int>::__union_t<testException()::Throwing, int>&)’ is implicitly deleted because the default definition would be ill-formed:
_bk;t=1697372498369# |   860 |   union __union_t {
_bk;t=1697372498369# |       |         ^~~~~~~~~
_bk;t=1697372498369# | /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-5c70ceab5f4e-1/llvm-project/libcxx-ci/build/generic-gcc/include/c++/v1/__expected/expected.h:888:16: error: union member ‘std::__1::expected<testException()::Throwing, int>::__union_t<testException()::Throwing, int>::__val_’ with non-trivial ‘testException()::Throwing::Throwing(const testException()::Throwing&)’
_bk;t=1697372498369# |   888 |     _ValueType __val_;
_bk;t=1697372498369# |       |                ^~~~~~
```

I now added constructors to the `__union_t` as @huixie90 suggested. Let's see if the CI succeeds...

https://github.com/llvm/llvm-project/pull/68733