Skip to content

[libc++] Uglify non-standard member typedef const_reference in bitset #121620

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.

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

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
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.
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