Skip to content

[libc++] Remove some unnecessary functions from __vector_layout #207152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
philnik777 merged 1 commit into from
Jul 9, 2026
+9 −55
Merged

[libc++] Remove some unnecessary functions from __vector_layout #207152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 0 additions & 46 deletions libcxx/include/__vector/layout.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,6 @@ class __vector_layout {
[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI size_type
__remaining_capacity() const _NOEXCEPT;

/// Determines if a reallocation is necessary.
[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI bool __is_full() const _NOEXCEPT;

/// Sets the member pointing to the first element in the vector to `__new_begin`, the member used
/// to obtain the vector's bound to the equivalent of `__new_size`, and the member that represents
/// the vector's capacity to the equivalent to `__new_capacity`.
Expand All @@ -202,9 +199,6 @@ class __vector_layout {

[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI size_type __size() const _NOEXCEPT;
[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI size_type __capacity() const _NOEXCEPT;
[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI bool __empty() const _NOEXCEPT;
[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI _Tp& __back() _NOEXCEPT;
[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI _Tp const& __back() const _NOEXCEPT;
[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI pointer __end_ptr() _NOEXCEPT;
[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI const_pointer __end_ptr() const _NOEXCEPT;
[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI pointer __capacity_ptr() _NOEXCEPT;
Expand Down Expand Up @@ -252,11 +246,6 @@ __vector_layout<_Tp, _Alloc>::__remaining_capacity() const _NOEXCEPT {
return __capacity_ - __size_;
}

template <class _Tp, class _Alloc>
_LIBCPP_CONSTEXPR_SINCE_CXX20 bool __vector_layout<_Tp, _Alloc>::__is_full() const _NOEXCEPT {
return __size_ == __capacity_;
}

template <class _Tp, class _Alloc>
_LIBCPP_CONSTEXPR_SINCE_CXX20 void __vector_layout<_Tp, _Alloc>::__set_layout(
pointer __new_begin, size_type __new_size, size_type __new_capacity) _NOEXCEPT {
Expand Down Expand Up @@ -324,21 +313,6 @@ __vector_layout<_Tp, _Alloc>::__capacity() const _NOEXCEPT {
return __capacity_;
}

template <class _Tp, class _Alloc>
_LIBCPP_CONSTEXPR_SINCE_CXX20 bool __vector_layout<_Tp, _Alloc>::__empty() const _NOEXCEPT {
return __size_ == 0;
}

template <class _Tp, class _Alloc>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _Tp& __vector_layout<_Tp, _Alloc>::__back() _NOEXCEPT {
return __begin_[__size_ - 1];
}

template <class _Tp, class _Alloc>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _Tp const& __vector_layout<_Tp, _Alloc>::__back() const _NOEXCEPT {
return __begin_[__size_ - 1];
}

template <class _Tp, class _Alloc>
_LIBCPP_CONSTEXPR_SINCE_CXX20 typename __vector_layout<_Tp, _Alloc>::pointer
__vector_layout<_Tp, _Alloc>::__end_ptr() _NOEXCEPT {
Expand Down Expand Up @@ -384,11 +358,6 @@ __vector_layout<_Tp, _Alloc>::__remaining_capacity() const _NOEXCEPT {
return __capacity_ - __end_;
}

template <class _Tp, class _Alloc>
[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 bool __vector_layout<_Tp, _Alloc>::__is_full() const _NOEXCEPT {
return __end_ == __capacity_;
}

template <class _Tp, class _Alloc>
_LIBCPP_CONSTEXPR_SINCE_CXX20 void __vector_layout<_Tp, _Alloc>::__set_layout(
pointer __new_begin, size_type __new_size, size_type __new_capacity) _NOEXCEPT {
Expand Down Expand Up @@ -456,21 +425,6 @@ __vector_layout<_Tp, _Alloc>::__capacity() const _NOEXCEPT {
return static_cast<size_type>(__capacity_ - __begin_);
}

template <class _Tp, class _Alloc>
_LIBCPP_CONSTEXPR_SINCE_CXX20 bool __vector_layout<_Tp, _Alloc>::__empty() const _NOEXCEPT {
return __begin_ == __end_;
}

template <class _Tp, class _Alloc>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _Tp& __vector_layout<_Tp, _Alloc>::__back() _NOEXCEPT {
return __end_[-1];
}

template <class _Tp, class _Alloc>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _Tp const& __vector_layout<_Tp, _Alloc>::__back() const _NOEXCEPT {
return __end_[-1];
}

template <class _Tp, class _Alloc>
_LIBCPP_CONSTEXPR_SINCE_CXX20 typename __vector_layout<_Tp, _Alloc>::pointer
__vector_layout<_Tp, _Alloc>::__end_ptr() _NOEXCEPT {
Expand Down
18 changes: 9 additions & 9 deletions libcxx/include/__vector/vector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ class vector {
return __layout_.__capacity();
}
[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI bool empty() const _NOEXCEPT {
return __layout_.__empty();
return size() == 0;

@nico nico Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

size() has to do arithmetic on the pointers, which requires complete types. The LHS, and vector before _LIBCPP_ABI_VECTOR_LAYOUT_SIZE_BASED, didn't require complete types to call empty().

Compete types are only needed if _LIBCPP_ABI_VECTOR_LAYOUT_SIZE_BASED is not defined, which means it is now ABI-dependent if a type has to be complete.

This also means not enabling _LIBCPP_ABI_VECTOR_LAYOUT_SIZE_BASED stops building for us for that reason. (And building with it causes a big size regression that we're currently investigating.)

As @ldionne says in #155330 (review), _LIBCPP_ABI_VECTOR_LAYOUT_SIZE_BASED seems like a risky change. Maybe we shouldn't make it harder to turn it off.

Could we maybe undo this change here to make it easier to compare behavior with the new layout on and off?

@ldionne ldionne Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't aware of (and didn't expect) this breakage before I read this. It seems like the original _LIBCPP_ABI_VECTOR_LAYOUT_SIZE_BASED patch indeed changed whether vector requires complete types to compute empty(). I can confirm that here: https://godbolt.org/z/EMff4KPjP. This was unintended, and we should fix that at least until we decide we want to change that guarantee. I filed #210732 for that.

I think this should also resolve your concern about different requirements depending on the ABI macro.

@ldionne ldionne Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I explained in #210732 (comment), it turns out that this is not "a guarantee" we were providing, merely something that happened to work. The Standard actually requires the type to be complete before instantiating any member function of vector<T>.

This doesn't change what I think we should do here: we should return to making it work at least for the LLVM 23 release cause this was unintended and it touches an important type -- but I suspect you should also get started fixing the code that relies on this.

@ldionne ldionne Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#210754

@nico nico Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

It seems like the original _LIBCPP_ABI_VECTOR_LAYOUT_SIZE_BASED patch indeed changed whether vector requires complete types to compute empty()

Just to be overly clear (I think you understood it already), but the problem for us was that this PR made the old vector require complete types, while the new one didn't. This made it impossible for us to switch back to the old impl to debug a size increase.

Thanks again for unblocking us :)

}

[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI size_type max_size() const _NOEXCEPT {
Expand Down Expand Up @@ -442,11 +442,11 @@ class vector {
}
[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI reference back() _NOEXCEPT {
_LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!empty(), "back() called on an empty vector");
return __layout_.__back();
return end()[-1];
}
[[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI const_reference back() const _NOEXCEPT {
_LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!empty(), "back() called on an empty vector");
return __layout_.__back();
return end()[-1];
}

//
Expand Down Expand Up @@ -595,7 +595,7 @@ class vector {
"vector::__vallocate can only be called on a vector that hasn't allocated memory. This vector either already "
"owns a buffer, or a deallocation function didn't reset the layout's begin pointer.");
_LIBCPP_ASSERT_INTERNAL(
__layout_.__empty(),
size() == 0,
"vector::__vallocate can only be called on a vector that hasn't allocated memory. This vector either already "
"owns a buffer, or a deallocation function didn't reset the layout's size.");
_LIBCPP_ASSERT_INTERNAL(
Expand Down Expand Up @@ -1080,7 +1080,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI typename vector<_Tp, _Alloc>
vector<_Tp, _Alloc>::emplace_back(_Args&&... __args) {
pointer __end = __layout_.__end_ptr();
std::__if_likely_else(
!__layout_.__is_full(),
size() != capacity(),
[&] {
__emplace_back_assume_capacity(std::forward<_Args>(__args)...);
++__end;
Expand Down Expand Up @@ -1134,7 +1134,7 @@ template <class _Tp, class _Allocator>
_LIBCPP_CONSTEXPR_SINCE_CXX20 typename vector<_Tp, _Allocator>::iterator
vector<_Tp, _Allocator>::insert(const_iterator __position, const_reference __x) {
pointer __p = this->__layout_.__begin_ptr() + (__position - begin());
if (!__layout_.__is_full()) {
if (size() != capacity()) {
pointer __end = __layout_.__end_ptr();
if (__p == __end) {
__emplace_back_assume_capacity(__x);
Expand All @@ -1157,7 +1157,7 @@ template <class _Tp, class _Allocator>
_LIBCPP_CONSTEXPR_SINCE_CXX20 typename vector<_Tp, _Allocator>::iterator
vector<_Tp, _Allocator>::insert(const_iterator __position, value_type&& __x) {
pointer __p = this->__layout_.__begin_ptr() + (__position - begin());
if (!__layout_.__is_full()) {
if (size() != capacity()) {
pointer __end = __layout_.__end_ptr();
if (__p == __end) {
__emplace_back_assume_capacity(std::move(__x));
Expand All @@ -1178,7 +1178,7 @@ template <class... _Args>
_LIBCPP_CONSTEXPR_SINCE_CXX20 typename vector<_Tp, _Allocator>::iterator
vector<_Tp, _Allocator>::emplace(const_iterator __position, _Args&&... __args) {
pointer __p = this->__layout_.__begin_ptr() + (__position - begin());
if (!__layout_.__is_full()) {
if (size() != capacity()) {
pointer __end = __layout_.__end_ptr();
if (__p == __end) {
__emplace_back_assume_capacity(std::forward<_Args>(__args)...);
Expand Down Expand Up @@ -1232,7 +1232,7 @@ vector<_Tp, _Allocator>::__insert_with_sentinel(const_iterator __position, _Inpu
difference_type __off = __position - begin();
pointer __p = this->__layout_.__begin_ptr() + __off;
pointer __old_last = __layout_.__end_ptr();
for (; !__layout_.__is_full() && __first != __last; ++__first)
for (; size() != capacity() && __first != __last; ++__first)
__emplace_back_assume_capacity(*__first);

if (__first == __last)
Expand Down
Loading