[libc++] Always return bool from bitset::operator[](size_t) const #169894
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
philnik777
force-pushed
the
bitset_abi_break
branch
from
a31aedb to
b5fa141
Compare
llvmbot
added
the
libc++
Member
|
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesThis takes an ABI break unconditionally, since it's small enough that nobody should be affected. This both simplifies Full diff: https://github.com/llvm/llvm-project/pull/169894.diff 3 Files Affected:
diff --git a/libcxx/include/__cxx03/bitset b/libcxx/include/__cxx03/bitset
index 37ad674686ba4..1da18832e9ddf 100644
--- a/libcxx/include/__cxx03/bitset
+++ b/libcxx/include/__cxx03/bitset
@@ -612,11 +612,7 @@ public:
_LIBCPP_HIDE_FROM_ABI bitset& flip(size_t __pos);
// element access:
-#ifdef _LIBCPP_ABI_BITSET_VECTOR_BOOL_CONST_SUBSCRIPT_RETURN_BOOL
_LIBCPP_HIDE_FROM_ABI bool operator[](size_t __p) const { return __base::__make_ref(__p); }
-#else
- _LIBCPP_HIDE_FROM_ABI __const_reference operator[](size_t __p) const { return __base::__make_ref(__p); }
-#endif
_LIBCPP_HIDE_FROM_ABI reference operator[](size_t __p) { return __base::__make_ref(__p); }
_LIBCPP_HIDE_FROM_ABI unsigned long to_ulong() const;
_LIBCPP_HIDE_FROM_ABI unsigned long long to_ullong() const;
diff --git a/libcxx/include/bitset b/libcxx/include/bitset
index 3453c2fcde71e..fa71fc9cbeaab 100644
--- a/libcxx/include/bitset
+++ b/libcxx/include/bitset
@@ -151,6 +151,7 @@ template <size_t N> struct hash<std::bitset<N>>;
# include <__type_traits/integral_constant.h>
# include <__type_traits/is_char_like_type.h>
# include <__utility/integer_sequence.h>
+# include <__utility/unreachable.h>
# include <climits>
# include <stdexcept>
# include <string_view>
@@ -191,7 +192,6 @@ protected:
static const unsigned __bits_per_word = static_cast<unsigned>(sizeof(__storage_type) * CHAR_BIT);
friend class __bit_reference<__bitset>;
- friend class __bit_const_reference<__bitset>;
friend class __bit_iterator<__bitset, false>;
friend class __bit_iterator<__bitset, true>;
friend struct __bit_array<__bitset>;
@@ -199,7 +199,6 @@ protected:
__storage_type __first_[_N_words];
typedef __bit_reference<__bitset> reference;
- typedef __bit_const_reference<__bitset> __const_reference;
typedef __bit_iterator<__bitset, false> __iterator;
typedef __bit_iterator<__bitset, true> __const_iterator;
@@ -209,9 +208,6 @@ protected:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 reference __make_ref(size_t __pos) _NOEXCEPT {
return reference(__first_ + __pos / __bits_per_word, __storage_type(1) << __pos % __bits_per_word);
}
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __const_reference __make_ref(size_t __pos) const _NOEXCEPT {
- return __const_reference(__first_ + __pos / __bits_per_word, __storage_type(1) << __pos % __bits_per_word);
- }
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __iterator __make_iter(size_t __pos) _NOEXCEPT {
return __iterator(__first_ + __pos / __bits_per_word, __pos % __bits_per_word);
}
@@ -219,6 +215,10 @@ protected:
return __const_iterator(__first_ + __pos / __bits_per_word, __pos % __bits_per_word);
}
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool __is_set(size_t __pos) const _NOEXCEPT {
+ return __first_[__pos / __bits_per_word] & (__storage_type(1) << __pos % __bits_per_word);
+ }
+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void operator&=(const __bitset& __v) _NOEXCEPT;
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void operator|=(const __bitset& __v) _NOEXCEPT;
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void operator^=(const __bitset& __v) _NOEXCEPT;
@@ -416,7 +416,6 @@ protected:
static const unsigned __bits_per_word = static_cast<unsigned>(sizeof(__storage_type) * CHAR_BIT);
friend class __bit_reference<__bitset>;
- friend class __bit_const_reference<__bitset>;
friend class __bit_iterator<__bitset, false>;
friend class __bit_iterator<__bitset, true>;
friend struct __bit_array<__bitset>;
@@ -424,7 +423,6 @@ protected:
__storage_type __first_;
typedef __bit_reference<__bitset> reference;
- typedef __bit_const_reference<__bitset> __const_reference;
typedef __bit_iterator<__bitset, false> __iterator;
typedef __bit_iterator<__bitset, true> __const_iterator;
@@ -434,9 +432,6 @@ protected:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 reference __make_ref(size_t __pos) _NOEXCEPT {
return reference(&__first_, __storage_type(1) << __pos);
}
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __const_reference __make_ref(size_t __pos) const _NOEXCEPT {
- return __const_reference(&__first_, __storage_type(1) << __pos);
- }
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __iterator __make_iter(size_t __pos) _NOEXCEPT {
// Allow the == case to accommodate the past-the-end iterator.
_LIBCPP_ASSERT_INTERNAL(__pos <= __bits_per_word, "Out of bounds access in the single-word bitset implementation.");
@@ -448,6 +443,10 @@ protected:
return __pos != __bits_per_word ? __const_iterator(&__first_, __pos) : __const_iterator(&__first_ + 1, 0);
}
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool __is_set(size_t __pos) const _NOEXCEPT {
+ return __first_ & (__storage_type(1) << __pos);
+ }
+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void operator&=(const __bitset& __v) _NOEXCEPT;
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void operator|=(const __bitset& __v) _NOEXCEPT;
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void operator^=(const __bitset& __v) _NOEXCEPT;
@@ -556,13 +555,11 @@ protected:
static const unsigned __bits_per_word = static_cast<unsigned>(sizeof(__storage_type) * CHAR_BIT);
friend class __bit_reference<__bitset>;
- friend class __bit_const_reference<__bitset>;
friend class __bit_iterator<__bitset, false>;
friend class __bit_iterator<__bitset, true>;
friend struct __bit_array<__bitset>;
typedef __bit_reference<__bitset> reference;
- typedef __bit_const_reference<__bitset> __const_reference;
typedef __bit_iterator<__bitset, false> __iterator;
typedef __bit_iterator<__bitset, true> __const_iterator;
@@ -572,9 +569,6 @@ protected:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 reference __make_ref(size_t) _NOEXCEPT {
return reference(nullptr, 1);
}
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __const_reference __make_ref(size_t) const _NOEXCEPT {
- return __const_reference(nullptr, 1);
- }
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __iterator __make_iter(size_t) _NOEXCEPT {
return __iterator(nullptr, 0);
}
@@ -582,6 +576,10 @@ protected:
return __const_iterator(nullptr, 0);
}
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool __is_set(size_t) const _NOEXCEPT {
+ std::__libcpp_unreachable();
+ }
+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void operator&=(const __bitset&) _NOEXCEPT {}
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void operator|=(const __bitset&) _NOEXCEPT {}
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void operator^=(const __bitset&) _NOEXCEPT {}
@@ -618,7 +616,6 @@ public:
static const unsigned __n_words = _Size == 0 ? 0 : (_Size - 1) / (sizeof(size_t) * CHAR_BIT) + 1;
typedef __bitset<__n_words, _Size> __base;
typedef typename __base::reference reference;
- typedef typename __base::__const_reference __const_reference;
// 23.3.5.1 constructors:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bitset() _NOEXCEPT {}
@@ -680,17 +677,10 @@ public:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bitset& flip(size_t __pos);
// element access:
-# ifdef _LIBCPP_ABI_BITSET_VECTOR_BOOL_CONST_SUBSCRIPT_RETURN_BOOL
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bool operator[](size_t __p) const {
_LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__p < _Size, "bitset::operator[] index out of bounds");
- return __base::__make_ref(__p);
+ return __base::__is_set(__p);
}
-# else
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __const_reference operator[](size_t __p) const {
- _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__p < _Size, "bitset::operator[] index out of bounds");
- return __base::__make_ref(__p);
- }
-# endif
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 reference operator[](size_t __p) {
_LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__p < _Size, "bitset::operator[] index out of bounds");
return __base::__make_ref(__p);
diff --git a/libcxx/test/std/utilities/template.bitset/bitset.members/index_const.pass.cpp b/libcxx/test/std/utilities/template.bitset/bitset.members/index_const.pass.cpp
index d1bf5b2c5d992..b941a4b2c7074 100644
--- a/libcxx/test/std/utilities/template.bitset/bitset.members/index_const.pass.cpp
+++ b/libcxx/test/std/utilities/template.bitset/bitset.members/index_const.pass.cpp
@@ -25,11 +25,7 @@ TEST_CONSTEXPR_CXX23 void test_index_const() {
assert(v[N / 2] == v.test(N / 2));
}
}
-#if !defined(_LIBCPP_VERSION) || defined(_LIBCPP_ABI_BITSET_VECTOR_BOOL_CONST_SUBSCRIPT_RETURN_BOOL)
ASSERT_SAME_TYPE(decltype(cases[0][0]), bool);
-#else
- ASSERT_SAME_TYPE(decltype(cases[0][0]), typename std::bitset<N>::__const_reference);
-#endif
}
TEST_CONSTEXPR_CXX23 bool test() {
@@ -47,11 +43,7 @@ TEST_CONSTEXPR_CXX23 bool test() {
const auto& set = set_;
auto b = set[0];
set_[0] = true;
-#if !defined(_LIBCPP_VERSION) || defined(_LIBCPP_ABI_BITSET_VECTOR_BOOL_CONST_SUBSCRIPT_RETURN_BOOL)
assert(!b);
-#else
- assert(b);
-#endif
return true;
}
|
ldionne
requested changes
Dec 8, 2025
Member
ldionne
left a comment
ldionne
left a comment
There was a problem hiding this comment.
I think this is probably reasonable, however I would like to provide an opt-out for two releases so we give people an easy way to work around issues and provide feedback in case anybody runs into problems.
philnik777
force-pushed
the
bitset_abi_break
branch
2 times, most recently
from
0e0e4a5 to
feb0742
Compare
ldionne
approved these changes
Dec 10, 2025
philnik777
force-pushed
the
bitset_abi_break
branch
from
feb0742 to
4c789da
Compare
philnik777
force-pushed
the
bitset_abi_break
branch
from
4c789da to
b51f3c0
Compare
anonymouspc
pushed a commit
to anonymouspc/llvm
that referenced
this pull request
Dec 15, 2025
…vm#169894) This takes an ABI break unconditionally, since it's small enough that nobody should be affected. This both simplifies `bitset` a bit and makes us more conforming.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This takes an ABI break unconditionally, since it's small enough that nobody should be affected. This both simplifies
bitseta bit and makes us more conforming.