[libc++] Fix improper static_cast in std::deque and __split_buffer

[winner245]

This PR addresses the improper use of static_cast to size_t where size_type is intended. Although the size_type member type of STL containers is usually a synonym of std::size_t, there is no guarantee that they are always equivalent. The C++ standard does not mandate this equivalence.

In libc++'s implementations of std::deque, std::vector, and __split_buffer, the size_type member type is defined as std::allocator_traits<allocator_type>::size_type, which is either allocator_type::size_type if available or std::make_unsigned<difference_type>::type. While it is true for std::allocator that the size_type member type is std::size_t, for user-defined allocator types, they may mismatch. This justifies the need to replace static_cast<size_t> with static_cast<size_type> in this PR.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

This PR addresses the improper use of static_cast to size_t where size_type is intended. Although the size_type member type of STL containers is usually a synonym of std::size_t, there is no guarantee that they are always equivalent. The C++ standard does not mandate this equivalence.

In libc++'s implementations of std::deque, std::vector, and __split_buffer, the size_type member type is defined as std::allocator_traits<allocator_type>::size_type, which is either allocator_type::size_type if available or std::make_unsigned<difference_type>::type. While it is true for std::allocator that the size_type member type is std::size_t, for user-defined allocator types, they may mismatch. This justifies the need to replace static_cast<size_t> with static_cast<size_type> in this PR.

---

Full diff: https://github.com/llvm/llvm-project/pull/119106.diff

2 Files Affected:

• (modified) libcxx/include/__split_buffer (+2-2)
• (modified) libcxx/include/deque (+2-2)

diff --git a/libcxx/include/__split_buffer b/libcxx/include/__split_buffer
index a637c83d17d107..5ee70220b55ab1 100644
--- a/libcxx/include/__split_buffer
+++ b/libcxx/include/__split_buffer
@@ -435,7 +435,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::emplace_fron
       __begin_            = std::move_backward(__begin_, __end_, __end_ + __d);
       __end_ += __d;
     } else {
-      size_type __c = std::max<size_type>(2 * static_cast<size_t>(__cap_ - __first_), 1);
+      size_type __c = std::max<size_type>(2 * static_cast<size_type>(__cap_ - __first_), 1);
       __split_buffer<value_type, __alloc_rr&> __t(__c, (__c + 3) / 4, __alloc_);
       __t.__construct_at_end(move_iterator<pointer>(__begin_), move_iterator<pointer>(__end_));
       std::swap(__first_, __t.__first_);
@@ -458,7 +458,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::emplace_back
       __end_              = std::move(__begin_, __end_, __begin_ - __d);
       __begin_ -= __d;
     } else {
-      size_type __c = std::max<size_type>(2 * static_cast<size_t>(__cap_ - __first_), 1);
+      size_type __c = std::max<size_type>(2 * static_cast<size_type>(__cap_ - __first_), 1);
       __split_buffer<value_type, __alloc_rr&> __t(__c, __c / 4, __alloc_);
       __t.__construct_at_end(move_iterator<pointer>(__begin_), move_iterator<pointer>(__end_));
       std::swap(__first_, __t.__first_);
diff --git a/libcxx/include/deque b/libcxx/include/deque
index ad667503489741..f44eeb3b2de81a 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -2431,7 +2431,7 @@ typename deque<_Tp, _Allocator>::iterator deque<_Tp, _Allocator>::erase(const_it
   difference_type __pos = __f - __b;
   iterator __p          = __b + __pos;
   allocator_type& __a   = __alloc();
-  if (static_cast<size_t>(__pos) <= (size() - 1) / 2) { // erase from front
+  if (static_cast<size_type>(__pos) <= (size() - 1) / 2) { // erase from front
     std::move_backward(__b, __p, std::next(__p));
     __alloc_traits::destroy(__a, std::addressof(*__b));
     --__size();
@@ -2459,7 +2459,7 @@ typename deque<_Tp, _Allocator>::iterator deque<_Tp, _Allocator>::erase(const_it
   iterator __p          = __b + __pos;
   if (__n > 0) {
     allocator_type& __a = __alloc();
-    if (static_cast<size_t>(__pos) <= (size() - __n) / 2) { // erase from front
+    if (static_cast<size_type>(__pos) <= (size() - __n) / 2) { // erase from front
       iterator __i = std::move_backward(__b, __p, __p + __n);
       for (; __b != __i; ++__b)
         __alloc_traits::destroy(__a, std::addressof(*__b));

[ldionne]

This makes sense to me, but would it make sense to try to exercise this with a test?

[winner245]

Thank you for the feedback. I am not sure how to add a suitable test for this purpose. I have tried to find examples that can trigger issues due to the improper static_cast, but I couldn't find a good one. The reason is that casting an integral value to std::size_t instead of size_type generally only enlarges the data range (since any practical size_type cannot exceed std::size_t), which does not cause data loss.

However, the improper static_cast I pointed out in this PR is indeed redundant. For example, in the following code, the explicit static_cast<size_t> first casts to size_t. But it then implicitly casts to size_type due to the explicit template argument specified as size_type:

size_type __c = std::max<size_type>(2 * static_cast<size_t>(__cap_ - __first_), 1);

I have also experimented with some custom allocators with different size_type definitions, e.g., uint_8, uint_16 (https://godbolt.org/z/cdse8Gh48). I found that std::vector strictly respects max_size() when calling reserve() and resize(), but std::deque seems to have completely ignored max_size() (perhaps because it is not a contiguous container). I am not sure if we should add a test similar to what I have done with the custom allocator.