Skip to content

Commit

Permalink
[libc++] Uglify non-standard member typedef const_reference in bitset (
Browse files Browse the repository at this point in the history
…#121620)

According to
[[template.bitset.general]](https://eel.is/c++draft/template.bitset.general),
`std::bitset` is supposed to have only
one (public) member typedef, `reference`. However, libc++'s
implementation of `std::bitset` offers more that that. Specifically, it
offers a public typedef `const_reference` and two private typedefs
`size_type` and `difference_type`. These non-standard member typedefs,
despite being private, can cause potential ambiguities in name lookup in
user-defined classes, as demonstrated in issue #121618.

Fixing the public member typedef `const_reference` is straightforward:
we can simply replace it with an `__ugly_name` such as
`__const_reference`. However, fixing the private member typedefs
`size_type` and `difference_type` is not so straightforward as they are
required by the `__bit_iterator` class and the corresponding algorithms
optimized for `__bit_iterator`s (e.g., `ranges::fill`).
 
This PR fixes the member typedef `const_reference` by using uglified
name for it. Further work will be undertaken to address `size_type` and
`difference_type`.

Follows up #80706, #111127, and #112843,
  • Loading branch information
winner245 authored Jan 9, 2025
1 parent f88ef1b commit 06673a9
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 27 deletions.
7 changes: 4 additions & 3 deletions libcxx/docs/ReleaseNotes/20.rst
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,10 @@ Deprecations and Removals
supported as an extension anymore, please migrate any code that uses e.g. ``std::vector<const T>`` to be
standards conforming.

- Non-conforming member typedefs ``base``, ``iterator`` and ``const_iterator`` of ``std::bitset``, and member typedef
``base`` of ``std::forward_list`` and ``std::list`` are removed. Previously, they were private but could cause
ambiguity in name lookup. Code that expects such ambiguity will possibly not compile in LLVM 20.
- Non-conforming member typedefs ``base``, ``iterator``, ``const_iterator``, and ``const_reference`` of ``std::bitset``,
and member typedef ``base`` of ``std::forward_list`` and ``std::list`` are removed. Previously, these member typedefs
(except ``const_reference``) were private but could cause ambiguity in name lookup. Code that expects such ambiguity
will possibly not compile in LLVM 20.

- The function ``__libcpp_verbose_abort()`` is now ``noexcept``, to match ``std::terminate()``. (The combination of
``noexcept`` and ``[[noreturn]]`` has special significance for function effects analysis.) For backwards compatibility,
Expand Down
22 changes: 11 additions & 11 deletions libcxx/include/bitset
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ protected:
__storage_type __first_[_N_words];

typedef __bit_reference<__bitset> reference;
typedef __bit_const_reference<__bitset> const_reference;
typedef __bit_const_reference<__bitset> __const_reference;
typedef __bit_iterator<__bitset, false> __iterator;
typedef __bit_iterator<__bitset, true> __const_iterator;

Expand All @@ -199,8 +199,8 @@ 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 __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);
Expand Down Expand Up @@ -451,7 +451,7 @@ protected:
__storage_type __first_;

typedef __bit_reference<__bitset> reference;
typedef __bit_const_reference<__bitset> const_reference;
typedef __bit_const_reference<__bitset> __const_reference;
typedef __bit_iterator<__bitset, false> __iterator;
typedef __bit_iterator<__bitset, true> __const_iterator;

Expand All @@ -461,8 +461,8 @@ 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 __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 {
return __iterator(&__first_ + __pos / __bits_per_word, __pos % __bits_per_word);
Expand Down Expand Up @@ -566,7 +566,7 @@ protected:
friend struct __bit_array<__bitset>;

typedef __bit_reference<__bitset> reference;
typedef __bit_const_reference<__bitset> const_reference;
typedef __bit_const_reference<__bitset> __const_reference;
typedef __bit_iterator<__bitset, false> __iterator;
typedef __bit_iterator<__bitset, true> __const_iterator;

Expand All @@ -576,8 +576,8 @@ 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 __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);
Expand Down Expand Up @@ -619,7 +619,7 @@ public:

public:
typedef typename __base::reference reference;
typedef typename __base::const_reference const_reference;
typedef typename __base::__const_reference __const_reference;

// 23.3.5.1 constructors:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bitset() _NOEXCEPT {}
Expand Down Expand Up @@ -689,7 +689,7 @@ public:
return __base::__make_ref(__p);
}
# else
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR const_reference operator[](size_t __p) const {
_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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

// constexpr bool operator[](size_t pos) const; // constexpr since C++23

// XFAIL: FROZEN-CXX03-HEADERS-FIXME

#include <bitset>
#include <cassert>
#include <cstddef>
Expand All @@ -18,17 +20,17 @@

template <std::size_t N>
TEST_CONSTEXPR_CXX23 void test_index_const() {
std::vector<std::bitset<N> > const cases = get_test_cases<N>();
for (std::size_t c = 0; c != cases.size(); ++c) {
std::bitset<N> const v = cases[c];
if (v.size() > 0) {
assert(v[N/2] == v.test(N/2));
}
std::vector<std::bitset<N> > const cases = get_test_cases<N>();
for (std::size_t c = 0; c != cases.size(); ++c) {
std::bitset<N> const v = cases[c];
if (v.size() > 0) {
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);
ASSERT_SAME_TYPE(decltype(cases[0][0]), bool);
#else
ASSERT_SAME_TYPE(decltype(cases[0][0]), typename std::bitset<N>::const_reference);
ASSERT_SAME_TYPE(decltype(cases[0][0]), typename std::bitset<N>::__const_reference);
#endif
}

Expand All @@ -43,10 +45,10 @@ TEST_CONSTEXPR_CXX23 bool test() {
test_index_const<65>();

std::bitset<1> set_;
set_[0] = false;
set_[0] = false;
const auto& set = set_;
auto b = set[0];
set_[0] = true;
auto b = set[0];
set_[0] = true;
#if !defined(_LIBCPP_VERSION) || defined(_LIBCPP_ABI_BITSET_VECTOR_BOOL_CONST_SUBSCRIPT_RETURN_BOOL)
assert(!b);
#else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@

// <bitset>

// This test ensures that we don't use a non-uglified name 'iterator',
// 'const_iterator', and 'base' in the implementation of bitset.
// This test ensures that we don't use a non-uglified name 'base', 'iterator',
// 'const_iterator', and `const_reference` in the implementation of bitset.
//
// See https://github.com/llvm/llvm-project/issues/111125.
// See https://github.com/llvm/llvm-project/issues/121618.

// XFAIL: FROZEN-CXX03-HEADERS-FIXME

Expand All @@ -23,6 +24,7 @@ struct my_base {
typedef int* iterator;
typedef const int* const_iterator;
typedef my_base base;
typedef const int& const_reference;
};

template <std::size_t N>
Expand Down Expand Up @@ -57,3 +59,13 @@ static_assert(std::is_same<my_derived<32>::base, my_base>::value, "");
static_assert(std::is_same<my_derived<48>::base, my_base>::value, "");
static_assert(std::is_same<my_derived<64>::base, my_base>::value, "");
static_assert(std::is_same<my_derived<96>::base, my_base>::value, "");

static_assert(std::is_same<my_derived<0>::const_reference, const int&>::value, "");
static_assert(std::is_same<my_derived<1>::const_reference, const int&>::value, "");
static_assert(std::is_same<my_derived<8>::const_reference, const int&>::value, "");
static_assert(std::is_same<my_derived<12>::const_reference, const int&>::value, "");
static_assert(std::is_same<my_derived<16>::const_reference, const int&>::value, "");
static_assert(std::is_same<my_derived<32>::const_reference, const int&>::value, "");
static_assert(std::is_same<my_derived<48>::const_reference, const int&>::value, "");
static_assert(std::is_same<my_derived<64>::const_reference, const int&>::value, "");
static_assert(std::is_same<my_derived<96>::const_reference, const int&>::value, "");

0 comments on commit 06673a9

Please sign in to comment.