Skip to content

Conversation

@winner245
Copy link
Contributor

@winner245 winner245 commented Jan 4, 2025

According to [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_iterators (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,

@winner245 winner245 force-pushed the fix-nonconforming-typedef-bitset branch from e4f1d20 to f6a9d40 Compare January 4, 2025 04:13
@github-actions
Copy link

github-actions bot commented Jan 4, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@winner245 winner245 force-pushed the fix-nonconforming-typedef-bitset branch from f6a9d40 to b97c017 Compare January 4, 2025 04:18
@winner245 winner245 changed the title [libc++] Fix non-conforming member typedef const_reference [libc++] Uglify non-standard member typedef const_reference in bitset Jan 4, 2025
@winner245 winner245 marked this pull request as ready for review January 4, 2025 20:58
@winner245 winner245 requested a review from a team as a code owner January 4, 2025 20:58
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 4, 2025

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

According to [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_iterators (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,


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

4 Files Affected:

  • (modified) libcxx/docs/ReleaseNotes/20.rst (+4-3)
  • (modified) libcxx/include/bitset (+11-11)
  • (modified) libcxx/test/std/utilities/template.bitset/bitset.members/index_const.pass.cpp (+13-11)
  • (modified) libcxx/test/std/utilities/template.bitset/bitset.members/nonstdmem.uglified.compile.pass.cpp (+14-2)
diff --git a/libcxx/docs/ReleaseNotes/20.rst b/libcxx/docs/ReleaseNotes/20.rst
index c8a07fb8b73348..913c2d8ff6380e 100644
--- a/libcxx/docs/ReleaseNotes/20.rst
+++ b/libcxx/docs/ReleaseNotes/20.rst
@@ -92,9 +92,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,
diff --git a/libcxx/include/bitset b/libcxx/include/bitset
index 919d2a0f07e096..c16635dc8092cd 100644
--- a/libcxx/include/bitset
+++ b/libcxx/include/bitset
@@ -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;
 
@@ -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);
@@ -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;
 
@@ -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);
@@ -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;
 
@@ -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);
@@ -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 {}
@@ -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);
   }
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 77eb9056bc6d99..bb7e10afc62ea9 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
@@ -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>
@@ -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
 }
 
@@ -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
diff --git a/libcxx/test/std/utilities/template.bitset/bitset.members/nonstdmem.uglified.compile.pass.cpp b/libcxx/test/std/utilities/template.bitset/bitset.members/nonstdmem.uglified.compile.pass.cpp
index ae3ac819b1f9c6..ee5c64f9df5c74 100644
--- a/libcxx/test/std/utilities/template.bitset/bitset.members/nonstdmem.uglified.compile.pass.cpp
+++ b/libcxx/test/std/utilities/template.bitset/bitset.members/nonstdmem.uglified.compile.pass.cpp
@@ -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
 
@@ -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>
@@ -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, "");

@frederick-vs-ja frederick-vs-ja merged commit 06673a9 into llvm:main Jan 9, 2025
66 checks passed
@winner245 winner245 deleted the fix-nonconforming-typedef-bitset branch January 9, 2025 12:48
ldionne pushed a commit that referenced this pull request Jan 14, 2025
This PR fixes the ambiguities in name lookup caused by non-standard
member typedefs `size_type` and `difference_type` in `std::bitset`.

Follows up #121620.
Closes #121618.
philnik777 added a commit to philnik777/llvm-project that referenced this pull request Aug 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants