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

[Louis Dionne via libcxx-commits]

================
@@ -1129,18 +1087,18 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 inline
     void
 #endif
     vector<_Tp, _Allocator>::emplace_back(_Args&&... __args) {
-  pointer __end = this->__end_;
+  auto __current_boundary = __layout_.__boundary_representation();
   std::__if_likely_else(
-      __end < this->__cap_,
+      __current_boundary < __layout_.__capacity_representation(),
       [&] {
         __emplace_back_assume_capacity(std::forward<_Args>(__args)...);
-        ++__end;
+        ++__current_boundary;
       },
-      [&] { __end = __emplace_back_slow_path(std::forward<_Args>(__args)...); });
+      [&] { __current_boundary = __emplace_back_slow_path(std::forward<_Args>(__args)...); });
 
-  this->__end_ = __end;
+  __layout_.__set_boundary(__current_boundary);
----------------
ldionne wrote:

I think this is the last remaining function that is meaningfully different depending on the layout (and that's why it's the last remaining function that uses the `__set_boundary` overload).

In the interest of unblocking this patch, I'd actually prefer doing this than the status quo:

```c++
// TODO: Refactor into the layout type
#if SIZE_BASED_VECTOR
    std::__if_likely_else(
        size() < capacity(),
        [&] {
            __emplace_back_assume_capacity(std::forward<_Args>(__args)...);
            __layout.__set_size(size() + 1);
        },
        [&] { __layout_.__set_end(__emplace_back_slow_path(std::forward<_Args>(__args)...)); }
    );
#else
    // current code
    pointer __end = this->__end_;
    std::__if_likely_else(
      __end < this->__cap_,
      [&] {
        __emplace_back_assume_capacity(std::forward<_Args>(__args)...);
        ++__end;
      },
      [&] { __end = __emplace_back_slow_path(std::forward<_Args>(__args)...); }
    );
    this->__end_ = __end;
#endif
```

This gets rid of what I think is my biggest concern in the patch: the usage of boundary to mean either size of pointer. I've said it many times, but I think that's too subtle and at this point I'd rather temporarily give up on not having any `#ifdef`s in the main vector code (until we find a way to refactor this) than keep all the `boundary` related stuff just for this one usage.

Important note: In `__emplace_back_slow_path`, I'd return a pointer all the time. This does mean the size-based vector is going to be less efficient by one pointer arithmetic, but that's in the slow path where this is going to be completely unnoticeable.

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