Skip to content

Conversation

frederick-vs-ja
Copy link
Contributor

[template.bitset.general] indicates that bitset shouldn't have member typedef-names iterator and const_iterator. Currently libc++'s typedef-names are causing ambiguity in name lookup, which isn't conforming.

As these iterator types are themselves useful, I think we should just use __uglified member typedef-names for them.

Fixes #111125.

[template.bitset.general] indicates that `bitset` shouldn't have member
typedef-names `iterator` and `const_iterator`. Currently libc++'s
typedef-names are causing ambiguity in name lookup, which isn't
conforming.

As these iterator types are themselves useful, I think we should just
use __uglified member typedef-names for them.
@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner October 4, 2024 10:06
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Oct 4, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 4, 2024

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

[template.bitset.general] indicates that bitset shouldn't have member typedef-names iterator and const_iterator. Currently libc++'s typedef-names are causing ambiguity in name lookup, which isn't conforming.

As these iterator types are themselves useful, I think we should just use __uglified member typedef-names for them.

Fixes #111125.


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

2 Files Affected:

  • (modified) libcxx/include/bitset (+22-22)
  • (added) libcxx/test/std/utilities/template.bitset/bitset.members/nonstdmem.uglified.compile.pass.cpp (+46)
diff --git a/libcxx/include/bitset b/libcxx/include/bitset
index ce23d522168c4c..f90ceaab816cca 100644
--- a/libcxx/include/bitset
+++ b/libcxx/include/bitset
@@ -187,8 +187,8 @@ protected:
 
   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;
+  typedef __bit_iterator<__bitset, false> __iterator;
+  typedef __bit_iterator<__bitset, true> __const_iterator;
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __bitset() _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI explicit _LIBCPP_CONSTEXPR __bitset(unsigned long long __v) _NOEXCEPT;
@@ -199,11 +199,11 @@ protected:
   _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);
+  _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);
   }
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 const_iterator __make_iter(size_t __pos) const _NOEXCEPT {
-    return const_iterator(__first_ + __pos / __bits_per_word, __pos % __bits_per_word);
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __const_iterator __make_iter(size_t __pos) const _NOEXCEPT {
+    return __const_iterator(__first_ + __pos / __bits_per_word, __pos % __bits_per_word);
   }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void operator&=(const __bitset& __v) _NOEXCEPT;
@@ -335,8 +335,8 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void __bitset<_N_words, _Siz
 template <size_t _N_words, size_t _Size>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unsigned long
 __bitset<_N_words, _Size>::to_ulong(false_type) const {
-  const_iterator __e = __make_iter(_Size);
-  const_iterator __i = std::find(__make_iter(sizeof(unsigned long) * CHAR_BIT), __e, true);
+  __const_iterator __e = __make_iter(_Size);
+  __const_iterator __i = std::find(__make_iter(sizeof(unsigned long) * CHAR_BIT), __e, true);
   if (__i != __e)
     __throw_overflow_error("bitset to_ulong overflow error");
 
@@ -352,8 +352,8 @@ __bitset<_N_words, _Size>::to_ulong(true_type) const {
 template <size_t _N_words, size_t _Size>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unsigned long long
 __bitset<_N_words, _Size>::to_ullong(false_type) const {
-  const_iterator __e = __make_iter(_Size);
-  const_iterator __i = std::find(__make_iter(sizeof(unsigned long long) * CHAR_BIT), __e, true);
+  __const_iterator __e = __make_iter(_Size);
+  __const_iterator __i = std::find(__make_iter(sizeof(unsigned long long) * CHAR_BIT), __e, true);
   if (__i != __e)
     __throw_overflow_error("bitset to_ullong overflow error");
 
@@ -449,8 +449,8 @@ protected:
 
   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;
+  typedef __bit_iterator<__bitset, false> __iterator;
+  typedef __bit_iterator<__bitset, true> __const_iterator;
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __bitset() _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI explicit _LIBCPP_CONSTEXPR __bitset(unsigned long long __v) _NOEXCEPT;
@@ -461,11 +461,11 @@ protected:
   _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);
+  _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);
   }
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 const_iterator __make_iter(size_t __pos) const _NOEXCEPT {
-    return const_iterator(&__first_ + __pos / __bits_per_word, __pos % __bits_per_word);
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __const_iterator __make_iter(size_t __pos) const _NOEXCEPT {
+    return __const_iterator(&__first_ + __pos / __bits_per_word, __pos % __bits_per_word);
   }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void operator&=(const __bitset& __v) _NOEXCEPT;
@@ -564,8 +564,8 @@ protected:
 
   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;
+  typedef __bit_iterator<__bitset, false> __iterator;
+  typedef __bit_iterator<__bitset, true> __const_iterator;
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __bitset() _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI explicit _LIBCPP_CONSTEXPR __bitset(unsigned long long) _NOEXCEPT;
@@ -576,11 +576,11 @@ protected:
   _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);
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __iterator __make_iter(size_t) _NOEXCEPT {
+    return __iterator(nullptr, 0);
   }
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 const_iterator __make_iter(size_t) const _NOEXCEPT {
-    return const_iterator(nullptr, 0);
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __const_iterator __make_iter(size_t) const _NOEXCEPT {
+    return __const_iterator(nullptr, 0);
   }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void operator&=(const __bitset&) _NOEXCEPT {}
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
new file mode 100644
index 00000000000000..c9dd923d7130f5
--- /dev/null
+++ b/libcxx/test/std/utilities/template.bitset/bitset.members/nonstdmem.uglified.compile.pass.cpp
@@ -0,0 +1,46 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <bitset>
+
+// This test ensures that we don't use a non-uglified name 'iterator' and
+// 'const_iterator' in the implementation of bitset.
+//
+// See https://github.com/llvm/llvm-project/issues/111125.
+
+#include <cstddef>
+#include <bitset>
+#include <type_traits>
+
+struct my_base {
+  typedef int* iterator;
+  typedef const int* const_iterator;
+};
+
+template <std::size_t N>
+struct my_derived : my_base, std::bitset<N> {};
+
+static_assert(std::is_same<my_derived<0>::iterator, int*>::value, "");
+static_assert(std::is_same<my_derived<1>::iterator, int*>::value, "");
+static_assert(std::is_same<my_derived<8>::iterator, int*>::value, "");
+static_assert(std::is_same<my_derived<12>::iterator, int*>::value, "");
+static_assert(std::is_same<my_derived<16>::iterator, int*>::value, "");
+static_assert(std::is_same<my_derived<32>::iterator, int*>::value, "");
+static_assert(std::is_same<my_derived<48>::iterator, int*>::value, "");
+static_assert(std::is_same<my_derived<64>::iterator, int*>::value, "");
+static_assert(std::is_same<my_derived<96>::iterator, int*>::value, "");
+
+static_assert(std::is_same<my_derived<0>::const_iterator, const int*>::value, "");
+static_assert(std::is_same<my_derived<1>::const_iterator, const int*>::value, "");
+static_assert(std::is_same<my_derived<8>::const_iterator, const int*>::value, "");
+static_assert(std::is_same<my_derived<12>::const_iterator, const int*>::value, "");
+static_assert(std::is_same<my_derived<16>::const_iterator, const int*>::value, "");
+static_assert(std::is_same<my_derived<32>::const_iterator, const int*>::value, "");
+static_assert(std::is_same<my_derived<48>::const_iterator, const int*>::value, "");
+static_assert(std::is_same<my_derived<64>::const_iterator, const int*>::value, "");
+static_assert(std::is_same<my_derived<96>::const_iterator, const int*>::value, "");

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a release note.

@ldionne ldionne merged commit 159d694 into llvm:main Oct 10, 2024
64 checks passed
@frederick-vs-ja frederick-vs-ja deleted the bitset-__uglified-iter branch October 10, 2024 13:40
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
…m#111127)

[template.bitset.general] indicates that `bitset` shouldn't have member
typedef-names `iterator` and `const_iterator`. Currently libc++'s
typedef-names are causing ambiguity in name lookup, which isn't
conforming.

As these iterator types are themselves useful, I think we should just
use __uglified member typedef-names for them.

Fixes llvm#111125
frederick-vs-ja added a commit that referenced this pull request Oct 18, 2024
Currently, libc++'s `bitset`, `forward_list`, and `list` have
non-conforming member typedef name `base`. The typedef is private, but
can cause ambiguity in name lookup.

Some other classes in libc++ that are either implementation details or
not precisely specified by the standard also have member typdef `base`.
I think this can still be conforming.

Follows up #80706 and #111127.
frederick-vs-ja pushed a commit that referenced this pull request Jan 9, 2025
…#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,
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.

[libc++] bitset has non-conforming member typedef-names iterator and const_iterator

3 participants