[libcxx-commits] [libcxx] [libcxx] adds a size-based representation for `vector`'s unstable ABI (PR #155330)

[Christopher Di Bella via libcxx-commits]

================
@@ -974,22 +928,15 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 inline _LIBCPP_HIDE_FROM_ABI vector<_Tp, _Allocato
 #else
     _NOEXCEPT_(is_nothrow_move_constructible<allocator_type>::value)
 #endif
-    : __alloc_(std::move(__x.__alloc_)) {
-  this->__begin_ = __x.__begin_;
-  this->__end_   = __x.__end_;
-  this->__cap_   = __x.__cap_;
-  __x.__begin_ = __x.__end_ = __x.__cap_ = nullptr;
+    : __layout_(std::move(__x.__layout_)) {
 }
 
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 inline _LIBCPP_HIDE_FROM_ABI
 vector<_Tp, _Allocator>::vector(vector&& __x, const __type_identity_t<allocator_type>& __a)
-    : __alloc_(__a) {
-  if (__a == __x.__alloc_) {
-    this->__begin_ = __x.__begin_;
-    this->__end_   = __x.__end_;
-    this->__cap_   = __x.__cap_;
-    __x.__begin_ = __x.__end_ = __x.__cap_ = nullptr;
+    : __layout_(__a) {
+  if (__a == __x.__layout_.__alloc()) {
+    __layout_.__move_without_allocator(__x.__layout_);
----------------
cjdb wrote:

What's your goal in removing `__move_without_allocator`?

Here's the current constructor side-by-side with the proposed one:

**Current**
```cpp
vector(vector&& __x, const __type_identity_t<allocator_type>& __a)
    : __layout_(__a) {
  if (__a == __x.__layout_.__alloc()) {
    __layout_.__move_without_allocator(__x.__layout_);
  } else {
    typedef move_iterator<iterator> _Ip;
    __init_with_size(_Ip(__x.begin()), _Ip(__x.end()), __x.size());
  }
}
```

**Proposed**
```cpp
vector(vector&& __x, const __type_identity_t<allocator_type>& __a)
    : __layout_([&] {
        if (__a == __x.__layout_.__alloc()) {
          return __vector_layout(std::move(__x.__layout_), __a);
        } else {
          return __vector_layout(__a);
        }
      }())
{
  if (__a != __x.__layout_.__alloc()) {
    typedef move_iterator<iterator> _Ip;
    __init_with_size(_Ip(__x.begin()), _Ip(__x.end()), __x.size());
  }
}
```

I feel as if the proposed change has more complexity due to the lambda within the initialiser-list, and because we need to repeat the if-statement.

That said, I took a look at what `__init_with_size` does, and I'm not convinced it needs to be in `std::vector` at all. We could pull that and any dependencies out into the global namespace, or we could push them into `__vector_layout`. That would free us up to move the existing constructor's code into `__vector_layout` almost-verbatim (and potentially anything else that depends on `__init_with_size`?). As far as simplicity goes, making vector's constructor a pass-through to `__layout_` is probably the best we can get. What do you think about this as an alternative?

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