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

[Louis Dionne via libcxx-commits]

================
@@ -862,47 +857,52 @@ template <ranges::input_range _Range,
 vector(from_range_t, _Range&&, _Alloc = _Alloc()) -> vector<ranges::range_value_t<_Range>, _Alloc>;
 #endif
 
-// __swap_out_circular_buffer relocates the objects in [__begin_, __end_) into the front of __v and swaps the buffers of
-// *this and __v. It is assumed that __v provides space for exactly (__end_ - __begin_) objects in the front. This
+// __swap_out_circular_buffer relocates the objects in [__begin_ptr(), size()) into the front of __v and swaps the
+// buffers of *this and __v. It is assumed that __v provides space for exactly size() objects in the front. This
 // function has a strong exception guarantee.
 template <class _Tp, class _Allocator>
-_LIBCPP_CONSTEXPR_SINCE_CXX20 void
-vector<_Tp, _Allocator>::__swap_out_circular_buffer(__split_buffer<value_type, allocator_type>& __v) {
+_LIBCPP_CONSTEXPR_SINCE_CXX20 void vector<_Tp, _Allocator>::__swap_out_circular_buffer(_SplitBuffer& __v) {
   __annotate_delete();
   auto __new_begin = __v.begin() - size();
   std::__uninitialized_allocator_relocate(
-      this->__alloc_, std::__to_address(__begin_), std::__to_address(__end_), std::__to_address(__new_begin));
+      this->__alloc(),
+      std::__to_address(__begin_ptr()),
+      std::__to_address(__end_ptr()),
+      std::__to_address(__new_begin));
   __v.__set_valid_range(__new_begin, __v.end());
-  __end_ = __begin_; // All the objects have been destroyed by relocating them.
+  __set_boundary(static_cast<size_type>(0)); // All the objects have been destroyed by relocating them.
 
   __swap_layouts(__v);
   __v.__set_data(__v.begin());
   __annotate_new(size());
 }
 
-// __swap_out_circular_buffer relocates the objects in [__begin_, __p) into the front of __v, the objects in
-// [__p, __end_) into the back of __v and swaps the buffers of *this and __v. It is assumed that __v provides space for
-// exactly (__p - __begin_) objects in the front and space for at least (__end_ - __p) objects in the back. This
-// function has a strong exception guarantee if __begin_ == __p || __end_ == __p.
+// __swap_out_circular_buffer relocates the objects in [__begin_ptr(), __p) into the front of __v, the objects in
+// [__p, end()) into the back of __v and swaps the buffers of *this and __v. It is assumed that __v provides space for
+// exactly (__p - __begin_ptr()) objects in the front and space for at least (size() - __p) objects in the back. This
+// function has a strong exception guarantee if __begin_ptr() == __p || size() == __p.
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 typename vector<_Tp, _Allocator>::pointer
-vector<_Tp, _Allocator>::__swap_out_circular_buffer(__split_buffer<value_type, allocator_type>& __v, pointer __p) {
+vector<_Tp, _Allocator>::__swap_out_circular_buffer(_SplitBuffer& __v, pointer __p) {
   __annotate_delete();
   pointer __ret = __v.begin();
 
-  // Relocate [__p, __end_) first to avoid having a hole in [__begin_, __end_)
-  // in case something in [__begin_, __p) throws.
+  pointer __end = __end_ptr();
+  // Relocate [__p, __end) first to avoid having a hole in [__begin_ptr(), __end)
+  // in case something in [__begin_ptr(), __p) throws.
   std::__uninitialized_allocator_relocate(
-      this->__alloc_, std::__to_address(__p), std::__to_address(__end_), std::__to_address(__v.end()));
-  auto __relocated_so_far = __end_ - __p;
+      this->__alloc(), std::__to_address(__p), std::__to_address(__end), std::__to_address(__v.end()));
+  auto __relocated_so_far = __end - __p;
   __v.__set_sentinel(__v.end() + __relocated_so_far);
-  __end_           = __p; // The objects in [__p, __end_) have been destroyed by relocating them.
-  auto __new_begin = __v.begin() - (__p - __begin_);
+  __set_boundary(__boundary_representation() -
+                 __relocated_so_far); // The objects in [__p, __end_) have been destroyed by relocating them.
+  auto __new_begin = __v.begin() - (__p - __begin_ptr());
 
   std::__uninitialized_allocator_relocate(
-      this->__alloc_, std::__to_address(__begin_), std::__to_address(__p), std::__to_address(__new_begin));
-  __v.__set_valid_range(__new_begin, __v.end());
-  __end_ = __begin_; // All the objects have been destroyed by relocating them.
+      this->__alloc(), std::__to_address(__begin_ptr()), std::__to_address(__p), std::__to_address(__new_begin));
+  __v.__set_valid_range(__new_begin, __v.size() + size());
+  __set_boundary(static_cast<size_type>(0)); // All the objects have been destroyed by relocating them.
----------------
ldionne wrote:

I am wondering whether it would be clearer to instead provide the following two functions in the layout class:

```
void __set_boundary(pointer p);
void __set_size(size_type s);
```

Then, each layout implements both, and we call whatever function makes the most sense from the vector implementation. This would make it easier to follow than having two overloads of `__set_boundary`.

And while we're at it, perhaps we can go for this naming instead:

```
void __set_end(pointer p);
void __set_size(size_type s);
```

Since we're not trying to use the same term to denote two things (either the size of the end pointer), perhaps we don't need to tip-toe around it anymore and we can just use the more usual term in each case?

Edit: After writing the above comment about the changing type of expressions like `__boundary_representation() - __relocated_so_far`, I realize that both are tied. You need to have a single overload of `__set_boundary` that takes both possible expression types, however I think that's a bug, not a feature. So I think that making a change like `__set_end` and `__set_size` would *force* us to spot places where expressions have a "undecided" type, and figure out a proper way to make those work.

Note that I'd be willing to go as far as refactoring `__swap_out_circular_buffer` into something different, perhaps even part of the layout classes, if that's what we need to eliminate this kind of macro-like-just-not-with-the-preprocessor code.

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