[libc++] __uglify internal member names of iterators in bitset
#111127
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
[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.
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: A. Jiang (frederick-vs-ja) Changes[template.bitset.general] indicates that 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:
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, "");
|
ldionne
approved these changes
Oct 9, 2024
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,
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
[template.bitset.general] indicates that
bitsetshouldn't have member typedef-namesiteratorandconst_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.