This is an archive of the discontinued LLVM Phabricator instance.

Differential D154860

[NFC][libc++][bitset] Refactors constructors.
ClosedPublic

Authored by Mordante on Jul 10 2023, 9:33 AM.

Details

Reviewers
philnik
ldionne
Group Reviewers
Restricted Project
Commits
rG56ae3568ead3: [NFC][libc++][bitset] Refactors constructors.
Summary

Based on the review comments in D153201 this combines the string and
c-string constructors. The common constructor is using a string_view:

  • it allows propagating the _Traits, which are required to be used for comparison.
  • it avoids allocating
  • libc++ supports it in C++03

Diff Detail

Event Timeline

Mordante created this revision.Jul 10 2023, 9:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 10 2023, 9:33 AM
Mordante requested review of this revision.Jul 10 2023, 9:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 10 2023, 9:33 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
ldionne accepted this revision.Jul 10 2023, 12:18 PM
ldionne added a subscriber: ldionne.

LGTM w/ green CI.

libcxx/include/bitset
117

Commit message typo: commont.

710

Any reason for this being in parens? I'd remove them if we can, while we're at it.

This revision is now accepted and ready to land.Jul 10 2023, 12:18 PM
philnik accepted this revision.Jul 10 2023, 12:49 PM

Thanks for the cleanup. LGTM with comments addressed.

libcxx/include/bitset
796

Could we name this something along the lines of __init_from_string? We have __init in a bunch of places throughout the code base and in many places the overload set is completely impenetrable.

811

We might as well do that now, since the lines are being touched.

Harbormaster completed remote builds in B244185: Diff 538698.Jul 10 2023, 12:56 PM
Mordante updated this revision to Diff 540668.Jul 15 2023, 3:21 AM

Rebased and addresses review comments.

Mordante edited the summary of this revision. (Show Details)Jul 15 2023, 3:21 AM
Mordante marked 4 inline comments as done.Jul 15 2023, 3:24 AM
Mordante added inline comments.
libcxx/include/bitset
710

No reason, I just copy-pasted it and didn't notice.

Mordante updated this revision to Diff 540669.Jul 15 2023, 3:25 AM
Mordante marked an inline comment as done.

Addresses the last review comment that I forgot to save.

Harbormaster completed remote builds in B245578: Diff 540669.Jul 15 2023, 6:16 AM
Closed by commit rG56ae3568ead3: [NFC][libc++][bitset] Refactors constructors. (authored by Mordante). · Explain WhyJul 15 2023, 6:21 AM
This revision was automatically updated to reflect the committed changes.
Mordante added a commit: rG56ae3568ead3: [NFC][libc++][bitset] Refactors constructors..
var-const mentioned this in rG91876eab93a9: [libc++] Increase the constexpr steps limit on some `bitset` tests..Jul 27 2023, 3:29 PM

Revision Contents

PathSize
libcxx/
include/
101 lines
test/
libcxx/
transitive_includes/
1 line
1 line
1 line
1 line
1 line
1 line
1 line
std/
utilities/
template.bitset/
bitset.members/
3 lines

Diff 540678

libcxx/include/bitset

Show First 20 Lines • Show All 108 Lines • ▼ Show 20 Lines
template <size_t N> struct hash<std::bitset<N>>; template <size_t N> struct hash<std::bitset<N>>;
} // std } // std
*/ */
#include <__algorithm/fill.h> #include <__algorithm/fill.h>
#include <__assert> // all public C++ headers provide the assertion handler #include <__assert> // all public C++ headers provide the assertion handler
#include <__bit_reference> #include <__bit_reference>
ldionneUnsubmitted
Done
ReplyInline Actions

Commit message typo: commont.

ldionne: Commit message typo: `commont`.
#include <__config> #include <__config>
#include <__functional/hash.h> #include <__functional/hash.h>
#include <__functional/unary_function.h> #include <__functional/unary_function.h>
#include <__type_traits/is_char_like_type.h> #include <__type_traits/is_char_like_type.h>
#include <climits> #include <climits>
#include <cstddef> #include <cstddef>
#include <stdexcept> #include <stdexcept>
#include <string_view>
#include <version> #include <version>
// standard-mandated includes // standard-mandated includes
// [bitset.syn] // [bitset.syn]
#include <iosfwd> #include <iosfwd>
#include <string> #include <string>
▲ Show 20 Lines • Show All 553 Lines • ▼ Show 20 Lines
public: public:
typedef typename base::reference reference; typedef typename base::reference reference;
typedef typename base::const_reference const_reference; typedef typename base::const_reference const_reference;
// 23.3.5.1 constructors: // 23.3.5.1 constructors:
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR bitset() _NOEXCEPT {} _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR bitset() _NOEXCEPT {}
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
bitset(unsigned long long __v) _NOEXCEPT : base(__v) {} bitset(unsigned long long __v) _NOEXCEPT : base(__v) {}
template<class _CharT, class = __enable_if_t<_IsCharLikeType<_CharT>::value> > template <class _CharT, class = __enable_if_t<_IsCharLikeType<_CharT>::value> >
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 explicit bitset(
explicit bitset(const _CharT* __str, const _CharT* __str,
typename basic_string<_CharT>::size_type __n = basic_string<_CharT>::npos, typename basic_string<_CharT>::size_type __n = basic_string<_CharT>::npos,
_CharT __zero = _CharT('0'), _CharT __one = _CharT('1')); _CharT __zero = _CharT('0'),
_CharT __one = _CharT('1')) {
size_t __rlen = std::min(__n, char_traits<_CharT>::length(__str));
__init_from_string_view(basic_string_view<_CharT>(__str, __rlen), __zero, __one);
}
template<class _CharT, class _Traits, class _Allocator> template <class _CharT, class _Traits, class _Allocator>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 explicit bitset(
explicit bitset(const basic_string<_CharT,_Traits,_Allocator>& __str, const basic_string<_CharT, _Traits, _Allocator>& __str,
typename basic_string<_CharT,_Traits,_Allocator>::size_type __pos = 0, typename basic_string<_CharT, _Traits, _Allocator>::size_type __pos = 0,
typename basic_string<_CharT,_Traits,_Allocator>::size_type __n = typename basic_string<_CharT, _Traits, _Allocator>::size_type __n =
(basic_string<_CharT,_Traits,_Allocator>::npos), basic_string<_CharT, _Traits, _Allocator>::npos,
ldionneUnsubmitted
Done
ReplyInline Actions

Any reason for this being in parens? I'd remove them if we can, while we're at it.

ldionne: Any reason for this being in parens? I'd remove them if we can, while we're at it.
MordanteAuthorUnsubmitted
Done
ReplyInline Actions

No reason, I just copy-pasted it and didn't notice.

Mordante: No reason, I just copy-pasted it and didn't notice.
_CharT __zero = _CharT('0'), _CharT __one = _CharT('1')); _CharT __zero = _CharT('0'),
_CharT __one = _CharT('1')) {
if (__pos > __str.size())
std::__throw_out_of_range("bitset string pos out of range");
size_t __rlen = std::min(__n, __str.size() - __pos);
__init_from_string_view(basic_string_view<_CharT, _Traits>(__str.data() + __pos, __rlen), __zero, __one);
}
// 23.3.5.2 bitset operations: // 23.3.5.2 bitset operations:
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23
bitset& operator&=(const bitset& __rhs) _NOEXCEPT; bitset& operator&=(const bitset& __rhs) _NOEXCEPT;
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23
bitset& operator|=(const bitset& __rhs) _NOEXCEPT; bitset& operator|=(const bitset& __rhs) _NOEXCEPT;
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23
bitset& operator^=(const bitset& __rhs) _NOEXCEPT; bitset& operator^=(const bitset& __rhs) _NOEXCEPT;
▲ Show 20 Lines • Show All 59 Lines • ▼ Show 20 Lines#endif
bool any() const _NOEXCEPT; bool any() const _NOEXCEPT;
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 bool none() const _NOEXCEPT {return !any();} _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 bool none() const _NOEXCEPT {return !any();}
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23
bitset operator<<(size_t __pos) const _NOEXCEPT; bitset operator<<(size_t __pos) const _NOEXCEPT;
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23
bitset operator>>(size_t __pos) const _NOEXCEPT; bitset operator>>(size_t __pos) const _NOEXCEPT;
private: private:
template <class _CharT, class _Traits>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void
__init_from_string_view(basic_string_view<_CharT, _Traits> __str, _CharT __zero, _CharT __one) {
philnikUnsubmitted
Done
ReplyInline Actions

Could we name this something along the lines of __init_from_string? We have __init in a bunch of places throughout the code base and in many places the overload set is completely impenetrable.

philnik: Could we name this something along the lines of `__init_from_string`? We have `__init` in a…
_LIBCPP_INLINE_VISIBILITY for (size_t __i = 0; __i < __str.size(); ++__i)
size_t __hash_code() const _NOEXCEPT {return base::__hash_code();} if (!_Traits::eq(__str[__i], __zero) && !_Traits::eq(__str[__i], __one))
std::__throw_invalid_argument("bitset string ctor has invalid argument");
friend struct hash<bitset>;
};
template <size_t _Size>
template<class _CharT, class>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23
bitset<_Size>::bitset(const _CharT* __str,
typename basic_string<_CharT>::size_type __n,
_CharT __zero, _CharT __one)
{
size_t __rlen = _VSTD::min(__n, char_traits<_CharT>::length(__str));
for (size_t __i = 0; __i < __rlen; ++__i)
if (__str[__i] != __zero && __str[__i] != __one)
__throw_invalid_argument("bitset string ctor has invalid argument");
size_t __mp = _VSTD::min(__rlen, _Size); size_t __mp = std::min(__str.size(), _Size);
size_t __i = 0; size_t __i = 0;
for (; __i < __mp; ++__i) for (; __i < __mp; ++__i) {
{
_CharT __c = __str[__mp - 1 - __i]; _CharT __c = __str[__mp - 1 - __i];
(*this)[__i] = (__c == __one); (*this)[__i] = _Traits::eq(__c, __one);
} }
_VSTD::fill(base::__make_iter(__i), base::__make_iter(_Size), false); std::fill(base::__make_iter(__i), base::__make_iter(_Size), false);
} }
template <size_t _Size> _LIBCPP_INLINE_VISIBILITY
philnikUnsubmitted
Done
ReplyInline Actions
std::fill(base::__make_iter(__i), base::__make_iter(_Size), false);
}
- _LIBCPP_INLINE_VISIBILITY
+ _LIBCPP_HIDE_FROM_ABI
size_t __hash_code() const _NOEXCEPT {return base::__hash_code();}

We might as well do that now, since the lines are being touched.

philnik: We might as well do that now, since the lines are being touched.
template<class _CharT, class _Traits, class _Allocator> size_t __hash_code() const _NOEXCEPT {return base::__hash_code();}
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23
bitset<_Size>::bitset(const basic_string<_CharT,_Traits,_Allocator>& __str,
typename basic_string<_CharT,_Traits,_Allocator>::size_type __pos,
typename basic_string<_CharT,_Traits,_Allocator>::size_type __n,
_CharT __zero, _CharT __one)
{
if (__pos > __str.size())
__throw_out_of_range("bitset string pos out of range");
size_t __rlen = _VSTD::min(__n, __str.size() - __pos);
for (size_t __i = __pos; __i < __pos + __rlen; ++__i)
if (!_Traits::eq(__str[__i], __zero) && !_Traits::eq(__str[__i], __one))
__throw_invalid_argument("bitset string ctor has invalid argument");
size_t __mp = _VSTD::min(__rlen, _Size); friend struct hash<bitset>;
size_t __i = 0; };
for (; __i < __mp; ++__i)
{
_CharT __c = __str[__pos + __mp - 1 - __i];
(*this)[__i] = _Traits::eq(__c, __one);
}
_VSTD::fill(base::__make_iter(__i), base::__make_iter(_Size), false);
}
template <size_t _Size> template <size_t _Size>
inline inline
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23
bitset<_Size>& bitset<_Size>&
bitset<_Size>::operator&=(const bitset& __rhs) _NOEXCEPT bitset<_Size>::operator&=(const bitset& __rhs) _NOEXCEPT
{ {
base::operator&=(__rhs); base::operator&=(__rhs);
▲ Show 20 Lines • Show All 325 Lines • Show Last 20 Lines

libcxx/test/libcxx/transitive_includes/cxx03.csv

Show First 20 Lines • Show All 89 Lines • ▼ Show 20 Lines
bitset cstdlib bitset cstdlib
bitset cstring bitset cstring
bitset initializer_list bitset initializer_list
bitset iosfwd bitset iosfwd
bitset limits bitset limits
bitset new bitset new
bitset stdexcept bitset stdexcept
bitset string bitset string
bitset string_view
bitset type_traits bitset type_traits
bitset version bitset version
ccomplex complex ccomplex complex
charconv cerrno charconv cerrno
charconv cmath charconv cmath
charconv concepts charconv concepts
charconv cstddef charconv cstddef
charconv cstdint charconv cstdint
▲ Show 20 Lines • Show All 886 Lines • Show Last 20 Lines

libcxx/test/libcxx/transitive_includes/cxx11.csv

Show First 20 Lines • Show All 89 Lines • ▼ Show 20 Lines
bitset cstdlib bitset cstdlib
bitset cstring bitset cstring
bitset initializer_list bitset initializer_list
bitset iosfwd bitset iosfwd
bitset limits bitset limits
bitset new bitset new
bitset stdexcept bitset stdexcept
bitset string bitset string
bitset string_view
bitset type_traits bitset type_traits
bitset version bitset version
ccomplex complex ccomplex complex
charconv cerrno charconv cerrno
charconv cmath charconv cmath
charconv concepts charconv concepts
charconv cstddef charconv cstddef
charconv cstdint charconv cstdint
▲ Show 20 Lines • Show All 887 Lines • Show Last 20 Lines

libcxx/test/libcxx/transitive_includes/cxx14.csv

Show First 20 Lines • Show All 89 Lines • ▼ Show 20 Lines
bitset cstdlib bitset cstdlib
bitset cstring bitset cstring
bitset initializer_list bitset initializer_list
bitset iosfwd bitset iosfwd
bitset limits bitset limits
bitset new bitset new
bitset stdexcept bitset stdexcept
bitset string bitset string
bitset string_view
bitset type_traits bitset type_traits
bitset version bitset version
ccomplex complex ccomplex complex
charconv cerrno charconv cerrno
charconv cmath charconv cmath
charconv concepts charconv concepts
charconv cstddef charconv cstddef
charconv cstdint charconv cstdint
▲ Show 20 Lines • Show All 889 Lines • Show Last 20 Lines

libcxx/test/libcxx/transitive_includes/cxx17.csv

Show First 20 Lines • Show All 89 Lines • ▼ Show 20 Lines
bitset cstdlib bitset cstdlib
bitset cstring bitset cstring
bitset initializer_list bitset initializer_list
bitset iosfwd bitset iosfwd
bitset limits bitset limits
bitset new bitset new
bitset stdexcept bitset stdexcept
bitset string bitset string
bitset string_view
bitset type_traits bitset type_traits
bitset version bitset version
ccomplex complex ccomplex complex
charconv cerrno charconv cerrno
charconv cmath charconv cmath
charconv concepts charconv concepts
charconv cstddef charconv cstddef
charconv cstdint charconv cstdint
▲ Show 20 Lines • Show All 889 Lines • Show Last 20 Lines

libcxx/test/libcxx/transitive_includes/cxx20.csv

Show First 20 Lines • Show All 88 Lines • ▼ Show 20 Lines
bitset cstdlib bitset cstdlib
bitset cstring bitset cstring
bitset initializer_list bitset initializer_list
bitset iosfwd bitset iosfwd
bitset limits bitset limits
bitset new bitset new
bitset stdexcept bitset stdexcept
bitset string bitset string
bitset string_view
bitset type_traits bitset type_traits
bitset version bitset version
ccomplex complex ccomplex complex
charconv cerrno charconv cerrno
charconv cmath charconv cmath
charconv concepts charconv concepts
charconv cstddef charconv cstddef
charconv cstdint charconv cstdint
▲ Show 20 Lines • Show All 895 Lines • Show Last 20 Lines

libcxx/test/libcxx/transitive_includes/cxx23.csv

Show First 20 Lines • Show All 50 Lines • ▼ Show 20 Lines
bitset cstdint bitset cstdint
bitset cstring bitset cstring
bitset initializer_list bitset initializer_list
bitset iosfwd bitset iosfwd
bitset limits bitset limits
bitset new bitset new
bitset stdexcept bitset stdexcept
bitset string bitset string
bitset string_view
bitset version bitset version
ccomplex complex ccomplex complex
charconv cerrno charconv cerrno
charconv cmath charconv cmath
charconv cstddef charconv cstddef
charconv cstdint charconv cstdint
charconv initializer_list charconv initializer_list
charconv limits charconv limits
▲ Show 20 Lines • Show All 638 Lines • Show Last 20 Lines

libcxx/test/libcxx/transitive_includes/cxx26.csv

Show First 20 Lines • Show All 50 Lines • ▼ Show 20 Lines
bitset cstdint bitset cstdint
bitset cstring bitset cstring
bitset initializer_list bitset initializer_list
bitset iosfwd bitset iosfwd
bitset limits bitset limits
bitset new bitset new
bitset stdexcept bitset stdexcept
bitset string bitset string
bitset string_view
bitset version bitset version
ccomplex complex ccomplex complex
charconv cerrno charconv cerrno
charconv cmath charconv cmath
charconv cstddef charconv cstddef
charconv cstdint charconv cstdint
charconv initializer_list charconv initializer_list
charconv limits charconv limits
▲ Show 20 Lines • Show All 638 Lines • Show Last 20 Lines

libcxx/test/std/utilities/template.bitset/bitset.members/left_shift.pass.cpp

//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// //
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information. // See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// The CI "Apple back-deployment with assertions enabled" needs a higher value
// ADDITIONAL_COMPILE_FLAGS(has-fconstexpr-steps): -fconstexpr-steps=12712420
// bitset<N> operator<<(size_t pos) const; // constexpr since C++23 // bitset<N> operator<<(size_t pos) const; // constexpr since C++23
#include <bitset> #include <bitset>
#include <cassert> #include <cassert>
#include <cstddef> #include <cstddef>
#include <vector> #include <vector>
#include "../bitset_test_cases.h" #include "../bitset_test_cases.h"
Show All 36 Lines