[libcxx-commits] [libcxx] [libc++][ranges][abi-break] Fix `movable_box` overwriting memory of data that lives in the tail padding (PR #71314)

[Louis Dionne via libcxx-commits]

================
@@ -134,6 +134,13 @@ concept __doesnt_need_empty_state =
          // 2. Otherwise, movable-box<T> should store only a T if either T models movable or
          //    is_nothrow_move_constructible_v<T> is true.
          : movable<_Tp> || is_nothrow_move_constructible_v<_Tp>);
----------------
ldionne wrote:

@huixie90 We actually discussed this in the libc++ monthly meeting today and @philnik777 made a really good observation. This change is only an ABI break for types that *contain* a `__movable_box` object, since it can change the layout of the outer type. But the layout of `__movable_box` itself never changes (when it's not a member of something else). This is not so relevant for `__movable_box`, but it's extremely relevant for `std::expected`. This basically means that adding an ABI tag to `__movable_box` itself isn't useful, instead we should add an ABI tag to the types whose layout may change, which are types that use `__movable_box` in their implementation.

If we added an ABI tag, we'd essentially create a lot of linker failures (due to the mangling changes) for cases where there is no actual ABI concern, such as passing such an object as an argument or else. And the cases where this is *actually* an ABI break (when it appears as a member of something else) are exactly the cases where an ABI tag is not going to help, since ABI tags don't propagate.

CC @jiixyj since this will apply to `std::expected` as well. But for `std::expected`, I don't think we have any type that holds an `expected` as an implementation detail, which would mean that we don't need to ABI tag anything.

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