Details
- Reviewers
ldionne - Group Reviewers
Restricted Project - Commits
- rGea82a822d990: [libc++] Adds string_view constructor overload
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libcxx/include/bitset | ||
---|---|---|
835 | Given that we want to move away from defining member functions out-of-line, maybe we want to jut define the new functions inline directly? | |
844–858 | I think a helper to deduplicate this code between the overloads above and below wold be a good idea. There is also some pretty obvious optimization opportunity missed by clang for this code, so maybe we can improve on it later. |
libcxx/include/bitset | ||
---|---|---|
835 | I don't think we ever discussed this explicitly, but that's how we write every new class, so I don't see why that would be different for pre-existing classes. So I guess I'm assuming consensus on this based on how people write code. |
libcxx/include/bitset | ||
---|---|---|
835 | I see. I don't object so I inlined the new constructor too. |
Thanks! LGTM w/ test comments applied.
libcxx/include/bitset | ||
---|---|---|
42–53 | Maybe this is more explicit? template <class charT> constexpr explicit bitset(const charT* str, typename basic_string<charT>::size_type n = basic_string<charT>::npos, charT zero = charT('0'), charT one = charT('1')); // until C++26, constexpr since C++23 template <class charT> constexpr explicit bitset(const charT* str, typename basic_string_view<charT>::size_type n = basic_string_view<charT>::npos, charT zero = charT('0'), charT one = charT('1')); // since C++26 | |
libcxx/test/std/utilities/template.bitset/bitset.cons/string_view_ctor.pass.cpp | ||
2 | We need to add stricter tests for the default arguments. Specifically, we should be calling as bitset(s) bitset(s, pos) bitset(s, pos, n) bitset(s, pos, n, zero) bitset(s, pos, n, zero, one) Also, it would be great to backport these additional tests to the string and c-string constructors, since it'll be trivial to do that. I'd do it as a fly-by change in this patch. | |
4 | We are also not testing for the explicit-ness of the constructor. We can using the !is_convertible && is_constructible check. |
Maybe this is more explicit?