Skip to content

[libc++] Fix {std, ranges}::equal for vector<bool> with small storage types #130394

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
merged 1 commit into from
Mar 19, 2025
Merged

[libc++] Fix {std, ranges}::equal for vector<bool> with small storage types #130394

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
36 changes: 20 additions & 16 deletions libcxx/include/__algorithm/equal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,24 +54,27 @@ __equal_unaligned(__bit_iterator<_Cp, _IsConst1> __first1,
unsigned __clz_f = __bits_per_word - __first1.__ctz_;
difference_type __dn = std::min(static_cast<difference_type>(__clz_f), __n);
__n -= __dn;
__storage_type __m = (~__storage_type(0) << __first1.__ctz_) & (~__storage_type(0) >> (__clz_f - __dn));
__storage_type __m = std::__middle_mask<__storage_type>(__clz_f - __dn, __first1.__ctz_);
__storage_type __b = *__first1.__seg_ & __m;
unsigned __clz_r = __bits_per_word - __first2.__ctz_;
__storage_type __ddn = std::min<__storage_type>(__dn, __clz_r);
__m = (~__storage_type(0) << __first2.__ctz_) & (~__storage_type(0) >> (__clz_r - __ddn));
__m = std::__middle_mask<__storage_type>(__clz_r - __ddn, __first2.__ctz_);
if (__first2.__ctz_ > __first1.__ctz_) {
if ((*__first2.__seg_ & __m) != (__b << (__first2.__ctz_ - __first1.__ctz_)))
if (static_cast<__storage_type>(*__first2.__seg_ & __m) !=
static_cast<__storage_type>(__b << (__first2.__ctz_ - __first1.__ctz_)))
return false;
} else {
if ((*__first2.__seg_ & __m) != (__b >> (__first1.__ctz_ - __first2.__ctz_)))
if (static_cast<__storage_type>(*__first2.__seg_ & __m) !=
static_cast<__storage_type>(__b >> (__first1.__ctz_ - __first2.__ctz_)))
return false;
}
__first2.__seg_ += (__ddn + __first2.__ctz_) / __bits_per_word;
__first2.__ctz_ = static_cast<unsigned>((__ddn + __first2.__ctz_) % __bits_per_word);
__dn -= __ddn;
if (__dn > 0) {
__m = ~__storage_type(0) >> (__bits_per_word - __dn);
if ((*__first2.__seg_ & __m) != (__b >> (__first1.__ctz_ + __ddn)))
__m = std::__trailing_mask<__storage_type>(__bits_per_word - __n);
if (static_cast<__storage_type>(*__first2.__seg_ & __m) !=
static_cast<__storage_type>(__b >> (__first1.__ctz_ + __ddn)))
return false;
__first2.__ctz_ = static_cast<unsigned>(__dn);
}
Expand All @@ -81,29 +84,30 @@ __equal_unaligned(__bit_iterator<_Cp, _IsConst1> __first1,
// __first1.__ctz_ == 0;
// do middle words
unsigned __clz_r = __bits_per_word - __first2.__ctz_;
__storage_type __m = ~__storage_type(0) << __first2.__ctz_;
__storage_type __m = std::__leading_mask<__storage_type>(__first2.__ctz_);
for (; __n >= __bits_per_word; __n -= __bits_per_word, ++__first1.__seg_) {
__storage_type __b = *__first1.__seg_;
if ((*__first2.__seg_ & __m) != (__b << __first2.__ctz_))
if (static_cast<__storage_type>(*__first2.__seg_ & __m) != static_cast<__storage_type>(__b << __first2.__ctz_))
return false;
++__first2.__seg_;
if ((*__first2.__seg_ & ~__m) != (__b >> __clz_r))
if (static_cast<__storage_type>(*__first2.__seg_ & static_cast<__storage_type>(~__m)) !=
static_cast<__storage_type>(__b >> __clz_r))
return false;
}
// do last word
if (__n > 0) {
__m = ~__storage_type(0) >> (__bits_per_word - __n);
__m = std::__trailing_mask<__storage_type>(__bits_per_word - __n);
__storage_type __b = *__first1.__seg_ & __m;
__storage_type __dn = std::min(__n, static_cast<difference_type>(__clz_r));
__m = (~__storage_type(0) << __first2.__ctz_) & (~__storage_type(0) >> (__clz_r - __dn));
if ((*__first2.__seg_ & __m) != (__b << __first2.__ctz_))
__m = std::__middle_mask<__storage_type>(__clz_r - __dn, __first2.__ctz_);
if (static_cast<__storage_type>(*__first2.__seg_ & __m) != static_cast<__storage_type>(__b << __first2.__ctz_))
return false;
__first2.__seg_ += (__dn + __first2.__ctz_) / __bits_per_word;
__first2.__ctz_ = static_cast<unsigned>((__dn + __first2.__ctz_) % __bits_per_word);
__n -= __dn;
if (__n > 0) {
__m = ~__storage_type(0) >> (__bits_per_word - __n);
if ((*__first2.__seg_ & __m) != (__b >> __dn))
__m = std::__trailing_mask<__storage_type>(__bits_per_word - __n);
if (static_cast<__storage_type>(*__first2.__seg_ & __m) != static_cast<__storage_type>(__b >> __dn))
return false;
}
}
Expand All @@ -128,7 +132,7 @@ __equal_aligned(__bit_iterator<_Cp, _IsConst1> __first1,
unsigned __clz = __bits_per_word - __first1.__ctz_;
difference_type __dn = std::min(static_cast<difference_type>(__clz), __n);
__n -= __dn;
__storage_type __m = (~__storage_type(0) << __first1.__ctz_) & (~__storage_type(0) >> (__clz - __dn));
__storage_type __m = std::__middle_mask<__storage_type>(__clz - __dn, __first1.__ctz_);
if ((*__first2.__seg_ & __m) != (*__first1.__seg_ & __m))
return false;
++__first2.__seg_;
Expand All @@ -144,7 +148,7 @@ __equal_aligned(__bit_iterator<_Cp, _IsConst1> __first1,
return false;
// do last word
if (__n > 0) {
__storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
__storage_type __m = std::__trailing_mask<__storage_type>(__bits_per_word - __n);
if ((*__first2.__seg_ & __m) != (*__first1.__seg_ & __m))
return false;
}
Expand Down
11 changes: 9 additions & 2 deletions libcxx/include/__bit_reference
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,20 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __trailing_mask
return static_cast<_StorageType>(~static_cast<_StorageType>(0)) >> __clz;
}

// Creates a mask of type `_StorageType` with a specified number of trailing zeros (__ctz) and sets all remaining
// bits to one.
template <class _StorageType>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __leading_mask(unsigned __ctz) {
static_assert(is_unsigned<_StorageType>::value, "__leading_mask only works with unsigned types");
return static_cast<_StorageType>(~static_cast<_StorageType>(0)) << __ctz;
}

// Creates a mask of type `_StorageType` with a specified number of leading zeros (__clz), a specified number of
// trailing zeros (__ctz), and sets all bits in between to one.
template <class _StorageType>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __middle_mask(unsigned __clz, unsigned __ctz) {
static_assert(is_unsigned<_StorageType>::value, "__middle_mask only works with unsigned types");
return (static_cast<_StorageType>(~static_cast<_StorageType>(0)) << __ctz) &
std::__trailing_mask<_StorageType>(__clz);
return std::__leading_mask<_StorageType>(__ctz) & std::__trailing_mask<_StorageType>(__clz);
}

// This function is designed to operate correctly even for smaller integral types like `uint8_t`, `uint16_t`,
Expand Down
3 changes: 3 additions & 0 deletions libcxx/include/__fwd/bit_reference.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ __fill_masked_range(_StoragePointer __word, unsigned __ctz, unsigned __clz, bool
template <class _StorageType>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __trailing_mask(unsigned __clz);

template <class _StorageType>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __leading_mask(unsigned __ctz);

template <class _StorageType>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __middle_mask(unsigned __clz, unsigned __ctz);

Expand Down
14 changes: 0 additions & 14 deletions libcxx/include/__vector/comparison.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,6 @@ operator==(const vector<_Tp, _Allocator>& __x, const vector<_Tp, _Allocator>& __
return __sz == __y.size() && std::equal(__x.begin(), __x.end(), __y.begin());
}

// FIXME: Remove this `vector<bool>` overload once #126369 is resolved, reverting to the generic `operator==`
// with `std::equal` for better performance.
template <class _Allocator>
_LIBCPP_CONSTEXPR_SINCE_CXX20 inline _LIBCPP_HIDE_FROM_ABI bool
operator==(const vector<bool, _Allocator>& __x, const vector<bool, _Allocator>& __y) {
const typename vector<bool, _Allocator>::size_type __sz = __x.size();
if (__sz != __y.size())
return false;
for (typename vector<bool, _Allocator>::size_type __i = 0; __i < __sz; ++__i)
if (__x[__i] != __y[__i])
return false;
return true;
}

#if _LIBCPP_STD_VER <= 17

template <class _Tp, class _Allocator>
Expand Down
86 changes: 86 additions & 0 deletions libcxx/test/std/algorithms/alg.nonmodifying/alg.equal/equal.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@
// MSVC warning C4244: 'argument': conversion from 'wchar_t' to 'const _Ty', possible loss of data
// MSVC warning C4389: '==': signed/unsigned mismatch
// ADDITIONAL_COMPILE_FLAGS(cl-style-warnings): /wd4242 /wd4244 /wd4389
// XFAIL: FROZEN-CXX03-HEADERS-FIXME

#include <algorithm>
#include <cassert>
#include <functional>
#include <vector>

#include "sized_allocator.h"
#include "test_iterators.h"
#include "test_macros.h"
#include "type_algorithms.h"
Expand Down Expand Up @@ -173,6 +175,90 @@ TEST_CONSTEXPR_CXX20 bool test() {
test_vector_bool<256>();
}

// Make sure std::equal behaves properly with std::vector<bool> iterators with custom size types.
// See issue: https://github.com/llvm/llvm-project/issues/126369.
{
//// Tests for std::equal with aligned bits

{ // Test the first (partial) word for uint8_t
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> in(6, true, Alloc(1));
std::vector<bool, Alloc> expected(8, true, Alloc(1));
assert(std::equal(in.begin() + 4, in.end(), expected.begin() + 4));
}
{ // Test the last word for uint8_t
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> in(12, true, Alloc(1));
std::vector<bool, Alloc> expected(16, true, Alloc(1));
assert(std::equal(in.begin(), in.end(), expected.begin()));
}
{ // Test middle words for uint8_t
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> in(24, true, Alloc(1));
std::vector<bool, Alloc> expected(29, true, Alloc(1));
assert(std::equal(in.begin(), in.end(), expected.begin()));
}

{ // Test the first (partial) word for uint16_t
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> in(12, true, Alloc(1));
std::vector<bool, Alloc> expected(16, true, Alloc(1));
assert(std::equal(in.begin() + 4, in.end(), expected.begin() + 4));
}
{ // Test the last word for uint16_t
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> in(24, true, Alloc(1));
std::vector<bool, Alloc> expected(32, true, Alloc(1));
assert(std::equal(in.begin(), in.end(), expected.begin()));
}
{ // Test middle words for uint16_t
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> in(48, true, Alloc(1));
std::vector<bool, Alloc> expected(55, true, Alloc(1));
assert(std::equal(in.begin(), in.end(), expected.begin()));
}

//// Tests for std::equal with unaligned bits

{ // Test the first (partial) word for uint8_t
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> in(6, true, Alloc(1));
std::vector<bool, Alloc> expected(8, true, Alloc(1));
assert(std::equal(in.begin() + 4, in.end(), expected.begin()));
}
{ // Test the last word for uint8_t
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> in(4, true, Alloc(1));
std::vector<bool, Alloc> expected(8, true, Alloc(1));
assert(std::equal(in.begin(), in.end(), expected.begin() + 3));
}
{ // Test middle words for uint8_t
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> in(16, true, Alloc(1));
std::vector<bool, Alloc> expected(24, true, Alloc(1));
assert(std::equal(in.begin(), in.end(), expected.begin() + 4));
}

{ // Test the first (partial) word for uint16_t
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> in(12, true, Alloc(1));
std::vector<bool, Alloc> expected(16, true, Alloc(1));
assert(std::equal(in.begin() + 4, in.end(), expected.begin()));
}
{ // Test the last word for uint16_t
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> in(12, true, Alloc(1));
std::vector<bool, Alloc> expected(16, true, Alloc(1));
assert(std::equal(in.begin(), in.end(), expected.begin() + 3));
}
{ // Test the middle words for uint16_t
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> in(32, true, Alloc(1));
std::vector<bool, Alloc> expected(64, true, Alloc(1));
assert(std::equal(in.begin(), in.end(), expected.begin() + 4));
}
}

return true;
}

Expand Down
125 changes: 117 additions & 8 deletions libcxx/test/std/algorithms/alg.nonmodifying/alg.equal/ranges.equal.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include <vector>

#include "almost_satisfies_types.h"
#include "sized_allocator.h"
#include "test_iterators.h"
#include "test_macros.h"

Expand Down Expand Up @@ -432,15 +433,123 @@ constexpr bool test() {
assert(projCount == 6);
}
}
}

{ // Test vector<bool>::iterator optimization
test_vector_bool<8>();
test_vector_bool<19>();
test_vector_bool<32>();
test_vector_bool<49>();
test_vector_bool<64>();
test_vector_bool<199>();
test_vector_bool<256>();
}

// Make sure std::equal behaves properly with std::vector<bool> iterators with custom size types.
// See issue: https://github.com/llvm/llvm-project/issues/126369.
{
//// Tests for std::equal with aligned bits

{ // Test the first (partial) word for uint8_t
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> in(6, true, Alloc(1));
std::vector<bool, Alloc> expected(8, true, Alloc(1));
auto a = std::ranges::subrange(in.begin() + 4, in.end());
auto b = std::ranges::subrange(expected.begin() + 4, expected.begin() + 4 + a.size());
assert(std::ranges::equal(a, b));
}
{ // Test the last word for uint8_t
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> in(12, true, Alloc(1));
std::vector<bool, Alloc> expected(16, true, Alloc(1));
auto a = std::ranges::subrange(in.begin(), in.end());
auto b = std::ranges::subrange(expected.begin(), expected.begin() + a.size());
assert(std::ranges::equal(a, b));
}
{ // Test middle words for uint8_t
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> in(24, true, Alloc(1));
std::vector<bool, Alloc> expected(29, true, Alloc(1));
auto a = std::ranges::subrange(in.begin(), in.end());
auto b = std::ranges::subrange(expected.begin(), expected.begin() + a.size());
assert(std::ranges::equal(a, b));
}

{ // Test vector<bool>::iterator optimization
test_vector_bool<8>();
test_vector_bool<19>();
test_vector_bool<32>();
test_vector_bool<49>();
test_vector_bool<64>();
test_vector_bool<199>();
test_vector_bool<256>();
{ // Test the first (partial) word for uint16_t
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> in(12, true, Alloc(1));
std::vector<bool, Alloc> expected(16, true, Alloc(1));
auto a = std::ranges::subrange(in.begin() + 4, in.end());
auto b = std::ranges::subrange(expected.begin() + 4, expected.begin() + 4 + a.size());
assert(std::ranges::equal(a, b));
}
{ // Test the last word for uint16_t
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> in(24, true, Alloc(1));
std::vector<bool, Alloc> expected(32, true, Alloc(1));
auto a = std::ranges::subrange(in.begin(), in.end());
auto b = std::ranges::subrange(expected.begin(), expected.begin() + a.size());
assert(std::ranges::equal(a, b));
}
{ // Test middle words for uint16_t
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> in(48, true, Alloc(1));
std::vector<bool, Alloc> expected(55, true, Alloc(1));
auto a = std::ranges::subrange(in.begin(), in.end());
auto b = std::ranges::subrange(expected.begin(), expected.begin() + a.size());
assert(std::ranges::equal(a, b));
}

//// Tests for std::equal with unaligned bits

{ // Test the first (partial) word for uint8_t
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> in(6, true, Alloc(1));
std::vector<bool, Alloc> expected(8, true, Alloc(1));
auto a = std::ranges::subrange(in.begin() + 4, in.end());
auto b = std::ranges::subrange(expected.begin(), expected.begin() + a.size());
assert(std::ranges::equal(a, b));
}
{ // Test the last word for uint8_t
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> in(4, true, Alloc(1));
std::vector<bool, Alloc> expected(8, true, Alloc(1));
auto a = std::ranges::subrange(in.begin(), in.end());
auto b = std::ranges::subrange(expected.begin() + 3, expected.begin() + 3 + a.size());
assert(std::ranges::equal(a, b));
}
{ // Test middle words for uint8_t
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> in(16, true, Alloc(1));
std::vector<bool, Alloc> expected(24, true, Alloc(1));
auto a = std::ranges::subrange(in.begin(), in.end());
auto b = std::ranges::subrange(expected.begin() + 4, expected.begin() + 4 + a.size());
assert(std::ranges::equal(a, b));
}

{ // Test the first (partial) word for uint16_t
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> in(12, true, Alloc(1));
std::vector<bool, Alloc> expected(16, true, Alloc(1));
auto a = std::ranges::subrange(in.begin() + 4, in.end());
auto b = std::ranges::subrange(expected.begin(), expected.begin() + a.size());
assert(std::ranges::equal(a, b));
}
{ // Test the last word for uint16_t
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> in(12, true, Alloc(1));
std::vector<bool, Alloc> expected(16, true, Alloc(1));
auto a = std::ranges::subrange(in.begin(), in.end());
auto b = std::ranges::subrange(expected.begin() + 3, expected.begin() + 3 + a.size());
assert(std::ranges::equal(a, b));
}
{ // Test the middle words for uint16_t
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> in(32, true, Alloc(1));
std::vector<bool, Alloc> expected(64, true, Alloc(1));
auto a = std::ranges::subrange(in.begin(), in.end());
auto b = std::ranges::subrange(expected.begin() + 4, expected.begin() + 4 + a.size());
assert(std::ranges::equal(a, b));
}
}

Expand Down