[libcxx-commits] [libcxx] [libc++] Make std::pair trivially copyable if its members are (PR #89652)

[Nico Weber via libcxx-commits]

nico wrote:

> @nico
> 
> I believe the new behavior is expected and correct. Indeed, based on https://godbolt.org/z/9TxKevxzn, I see the following behavior (please confirm we're on the same page):

If I use `-std=c++20` instead` of `-std=c++23`, I'm seeing the following (https://godbolt.org/z/35K7q4G6x)

```
// BEFORE THE PATCH
std::is_trivially_copyable<std::pair<int, OpIndex>>::value == false
std::is_trivially_copyable<std::pair<int, const OpIndex>>::value == true

// AFTER THE PATCH
std::is_trivially_copyable<std::pair<int, OpIndex>>::value == true
std::is_trivially_copyable<std::pair<int, const OpIndex>>::value == false
```

> Going from `false` to `true` for the non-const `OpIndex` case is the purpose of this patch, so that's an intended change. 

Yep.

> Staying `false` for the `const OpIndex` case isn't a change. I don't understand why it's not `true` after the patch and I'm not certain what's the best behavior within the Standard's constraints, but it doesn't seem like an obvious bug.

With `-std=c++20`, it _is_ a change, from `true` to `false` when the second element in the pari is a `const OpIndex`.

We currently have a `custom_container<T>` ([here](https://source.chromium.org/chromium/chromium/src/+/main:v8/src/compiler/turboshaft/loop-unrolling-reducer.h;l=566?q=loop-unrolling-reducer.h)) that asserts `is_trivially_copyable<T>`, and T is a `pair<int, const OpIndex>`. That currently compiles. After this patch, we have to change that to `pair<int, OpIndex>` without const – but we can't land that change since it breaks building with libc++ before this change.

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