[PATCH] D48616: Implement LWG 2946, 3075 and 3076
mclow.lists created this revision. mclow.lists added reviewers: EricWF, ldionne, STL_MSFT. A massive amount of doinking around in . https://cplusplus.github.io/LWG/issue2946 https://cplusplus.github.io/LWG/issue3075 https://cplusplus.github.io/LWG/issue3076 This is not quite right yet, but I wanted to get it up here for people to look at. I may have stepped on a bug fix for old versions of gcc in C++03 mode - Eric? [ was introduced in r292830 ] Stephan - I updated your deduction guide tests now that we implement all of them. https://reviews.llvm.org/D48616 Files: include/memory include/string test/std/strings/basic.string/string.cons/default_noexcept.pass.cpp test/std/strings/basic.string/string.cons/dtor_noexcept.pass.cpp test/std/strings/basic.string/string.cons/implicit_deduction_guides.pass.cpp test/std/strings/basic.string/string.cons/move_assign_noexcept.pass.cpp test/std/strings/basic.string/string.cons/move_noexcept.pass.cpp test/std/strings/basic.string/string.cons/string_view_deduction.fail.cpp test/std/strings/basic.string/string.cons/string_view_deduction.pass.cpp test/std/strings/basic.string/string.cons/string_view_size_size_deduction.fail.cpp test/std/strings/basic.string/string.cons/string_view_size_size_deduction.pass.cpp test/std/strings/basic.string/string.nonmembers/string.special/swap_noexcept.pass.cpp Index: test/std/strings/basic.string/string.nonmembers/string.special/swap_noexcept.pass.cpp === --- test/std/strings/basic.string/string.nonmembers/string.special/swap_noexcept.pass.cpp +++ test/std/strings/basic.string/string.nonmembers/string.special/swap_noexcept.pass.cpp @@ -35,8 +35,8 @@ some_alloc() {} some_alloc(const some_alloc&); + T *allocate(size_t); void deallocate(void*, unsigned) {} - typedef std::true_type propagate_on_container_swap; }; @@ -47,6 +47,7 @@ some_alloc2() {} some_alloc2(const some_alloc2&); + T *allocate(size_t); void deallocate(void*, unsigned) {} typedef std::false_type propagate_on_container_swap; Index: test/std/strings/basic.string/string.cons/string_view_size_size_deduction.pass.cpp === --- test/std/strings/basic.string/string.cons/string_view_size_size_deduction.pass.cpp +++ test/std/strings/basic.string/string.cons/string_view_size_size_deduction.pass.cpp @@ -0,0 +1,98 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// +// UNSUPPORTED: c++98, c++03, c++11, c++14 +// XFAIL: libcpp-no-deduction-guides + +// template +// basic_string(InputIterator begin, InputIterator end, +// const Allocator& a = Allocator()); + +// template +// > +// basic_string(basic_string_view, +//typename see below::size_type, +//typename see below::size_type, +//const Allocator& = Allocator()) +// -> basic_string; +// +// A size_type parameter type in a basic_string deduction guide refers to the size_type +// member type of the type deduced by the deduction guide. +// +// The deduction guide shall not participate in overload resolution if Allocator is +// is a type that does not qualify as an allocator. + + +#include +#include +#include +#include + +#include "test_macros.h" +#include "test_allocator.h" +#include "../input_iterator.h" +#include "min_allocator.h" + +int main() +{ +{ +std::string_view sv = "12345678901234"; +std::basic_string s1{sv, 0, 4}; +using S = decltype(s1); // what type did we get? +static_assert(std::is_same_v, ""); +static_assert(std::is_same_v>, ""); +static_assert(std::is_same_v>, ""); +assert(s1.size() == 4); +assert(s1.compare(0, s1.size(), sv.data(), s1.size()) == 0); +} + +{ +std::string_view sv = "12345678901234"; +std::basic_string s1{sv, 0, 4, std::allocator{}}; +using S = decltype(s1); // what type did we get? +static_assert(std::is_same_v, ""); +static_assert(std::is_same_v>, ""); +static_assert(std::is_same_v>, ""); +assert(s1.size() == 4); +assert(s1.compare(0, s1.size(), sv.data(), s1.size()) == 0); +} +{ +std::wstring_view sv = L"12345678901234"; +std::basic_string s1{sv, 0, 4, test_allocator{}}; +using S = decltype(s1); // what type did we get? +static_assert(std::is_same_v, ""); +static_assert(std::is_same_v>, ""); +static_assert(std::is_same_v>, ""); +assert(s1.size() == 4); +assert(s1.compare(0, s1.size(), sv.data(), s1.size()) == 0); +} +{ +std::u16string_view sv = u"12345678901234"; +std::basic_string
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
mclow.lists added a comment. Ok, for some reason, the four tests that I *added* didn't get into the diff. https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
STL_MSFT added inline comments. Comment at: test/std/strings/basic.string/string.cons/string_view_deduction.fail.cpp:26 +// The deduction guide shall not participate in overload resolution if Allocator is +// is a type that does not qualify as an allocator. + Repeated "is", occurs repeatedly in other files. Comment at: test/std/strings/basic.string/string.cons/string_view_deduction.fail.cpp:37 +{ +std::string_view sv = "12345678901234"; +std::basic_string s1{sv, 23}; // 23 is not an allocator! You need to include ``, occurs in other files. Comment at: test/std/strings/basic.string/string.cons/string_view_deduction.pass.cpp:32 +#include +#include + You technically need `` for std::allocator, `` for std::is_same_v. Comment at: test/std/strings/basic.string/string.nonmembers/string.special/swap_noexcept.pass.cpp:38 some_alloc(const some_alloc&); + T *allocate(size_t); void deallocate(void*, unsigned) {} Weird indentation here - is there a tab instead of spaces? Occurs below. https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
mclow.lists updated this revision to Diff 152995. mclow.lists added a comment. Remove a bunch of `noexcept`s that I missed the first time. Put in an error message to be checked in one of the failing deduction cases that was lacking it. Address STL's comments. https://reviews.llvm.org/D48616 Files: include/memory include/string test/std/strings/basic.string/string.cons/default_noexcept.pass.cpp test/std/strings/basic.string/string.cons/dtor_noexcept.pass.cpp test/std/strings/basic.string/string.cons/implicit_deduction_guides.pass.cpp test/std/strings/basic.string/string.cons/move_assign_noexcept.pass.cpp test/std/strings/basic.string/string.cons/move_noexcept.pass.cpp test/std/strings/basic.string/string.cons/string_view_deduction.fail.cpp test/std/strings/basic.string/string.cons/string_view_deduction.pass.cpp test/std/strings/basic.string/string.cons/string_view_size_size_deduction.fail.cpp test/std/strings/basic.string/string.cons/string_view_size_size_deduction.pass.cpp test/std/strings/basic.string/string.nonmembers/string.special/swap_noexcept.pass.cpp Index: test/std/strings/basic.string/string.nonmembers/string.special/swap_noexcept.pass.cpp === --- test/std/strings/basic.string/string.nonmembers/string.special/swap_noexcept.pass.cpp +++ test/std/strings/basic.string/string.nonmembers/string.special/swap_noexcept.pass.cpp @@ -35,8 +35,8 @@ some_alloc() {} some_alloc(const some_alloc&); + T *allocate(size_t); void deallocate(void*, unsigned) {} - typedef std::true_type propagate_on_container_swap; }; @@ -47,6 +47,7 @@ some_alloc2() {} some_alloc2(const some_alloc2&); + T *allocate(size_t); void deallocate(void*, unsigned) {} typedef std::false_type propagate_on_container_swap; Index: test/std/strings/basic.string/string.cons/string_view_size_size_deduction.pass.cpp === --- test/std/strings/basic.string/string.cons/string_view_size_size_deduction.pass.cpp +++ test/std/strings/basic.string/string.cons/string_view_size_size_deduction.pass.cpp @@ -0,0 +1,99 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// +// UNSUPPORTED: c++98, c++03, c++11, c++14 +// XFAIL: libcpp-no-deduction-guides + +// template +// basic_string(InputIterator begin, InputIterator end, +// const Allocator& a = Allocator()); + +// template +// > +// basic_string(basic_string_view, +//typename see below::size_type, +//typename see below::size_type, +//const Allocator& = Allocator()) +// -> basic_string; +// +// A size_type parameter type in a basic_string deduction guide refers to the size_type +// member type of the type deduced by the deduction guide. +// +// The deduction guide shall not participate in overload resolution if Allocator +// is a type that does not qualify as an allocator. + + +#include +#include +#include +#include +#include + +#include "test_macros.h" +#include "test_allocator.h" +#include "../input_iterator.h" +#include "min_allocator.h" + +int main() +{ +{ +std::string_view sv = "12345678901234"; +std::basic_string s1{sv, 0, 4}; +using S = decltype(s1); // what type did we get? +static_assert(std::is_same_v, ""); +static_assert(std::is_same_v>, ""); +static_assert(std::is_same_v>, ""); +assert(s1.size() == 4); +assert(s1.compare(0, s1.size(), sv.data(), s1.size()) == 0); +} + +{ +std::string_view sv = "12345678901234"; +std::basic_string s1{sv, 0, 4, std::allocator{}}; +using S = decltype(s1); // what type did we get? +static_assert(std::is_same_v, ""); +static_assert(std::is_same_v>, ""); +static_assert(std::is_same_v>, ""); +assert(s1.size() == 4); +assert(s1.compare(0, s1.size(), sv.data(), s1.size()) == 0); +} +{ +std::wstring_view sv = L"12345678901234"; +std::basic_string s1{sv, 0, 4, test_allocator{}}; +using S = decltype(s1); // what type did we get? +static_assert(std::is_same_v, ""); +static_assert(std::is_same_v>, ""); +static_assert(std::is_same_v>, ""); +assert(s1.size() == 4); +assert(s1.compare(0, s1.size(), sv.data(), s1.size()) == 0); +} +{ +std::u16string_view sv = u"12345678901234"; +std::basic_string s1{sv, 0, 4, min_allocator{}}; +using S = decltype(s1); // what type did we get? +static_assert(std::is_same_v, ""); +static_assert(std::is_same_v>, ""); +static_assert(std::is_same_v>, ""); +assert(s1.size() == 4); +assert(s1.compare(0, s1.size
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
mclow.lists updated this revision to Diff 153001. mclow.lists added a comment. Updated the `__is_allocator` type trait to work all the way back to C++03 Added a bunch of tests from issue2946. I think this has all the pieces that it needs now. https://reviews.llvm.org/D48616 Files: include/memory include/string test/std/strings/basic.string/string.cons/copy_assignment.pass.cpp test/std/strings/basic.string/string.cons/default_noexcept.pass.cpp test/std/strings/basic.string/string.cons/dtor_noexcept.pass.cpp test/std/strings/basic.string/string.cons/implicit_deduction_guides.pass.cpp test/std/strings/basic.string/string.cons/move_assign_noexcept.pass.cpp test/std/strings/basic.string/string.cons/move_noexcept.pass.cpp test/std/strings/basic.string/string.cons/pointer_size_alloc.pass.cpp test/std/strings/basic.string/string.cons/string_view_deduction.fail.cpp test/std/strings/basic.string/string.cons/string_view_deduction.pass.cpp test/std/strings/basic.string/string.cons/string_view_size_size_deduction.fail.cpp test/std/strings/basic.string/string.cons/string_view_size_size_deduction.pass.cpp test/std/strings/basic.string/string.modifiers/string_append/string.pass.cpp test/std/strings/basic.string/string.modifiers/string_insert/size_string.pass.cpp test/std/strings/basic.string/string.modifiers/string_op_plus_equal/string.pass.cpp test/std/strings/basic.string/string.modifiers/string_replace/iter_iter_string.pass.cpp test/std/strings/basic.string/string.modifiers/string_replace/size_size_string.pass.cpp test/std/strings/basic.string/string.nonmembers/string.special/swap_noexcept.pass.cpp test/std/strings/basic.string/string.ops/string_compare/size_size_string.pass.cpp test/std/strings/basic.string/string.ops/string_compare/string.pass.cpp test/std/strings/basic.string/string.ops/string_find.first.not.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp Index: test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp @@ -154,4 +154,10 @@ test1(); } #endif + + +{ // LWG 2946 +std::string s = " !"; +assert(s.rfind({"abc", 1}) == std::string::npos); +} } Index: test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp @@ -154,4 +154,9 @@ test1(); } #endif + +{ // LWG 2946 +std::string s = " !"; +assert(s.find_last_of({"abc", 1}) == std::string::npos); +} } Index: test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp @@ -154,4 +154,9 @@ test1(); } #endif + +{ // LWG 2946 +std::string s = " !"; +assert(s.find_last_not_of({"abc", 1}) == s.size() - 1); +} } Index: test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp @@ -154,4 +154,10 @@ test1(); } #endif + + +{ // LWG 2946 +std::string s = " !"; +assert(s.find_first_of({"abc", 1}) == std::string::npos); +} } Index: test/std/strings/basic.string/string.ops/string_find.first.not.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_find.first.not.of/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_find.first.not.of/string_size.pass.cpp @@ -154,4 +154,9 @@ test1(); } #endif + +{ // LWG 2946 +std::string s = " !"; +assert(s.find_first_not_of({"abc", 1}) == 0); +} } Index: test/std/strings/basic.string/string.ops/string_find/string_size.pass.cpp === --- test/std/strings/basic.
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
mclow.lists added inline comments. Comment at: test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp:161 +std::string s = " !"; +assert(s.rfind({"abc", 1}) == std::string::npos); +} These tests don't work in C++03; they'll need to be ifdef-ed. https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
mclow.lists updated this revision to Diff 153097. mclow.lists added a comment. Update the tests from 2946 - they are removed for C++03 https://reviews.llvm.org/D48616 Files: include/memory include/string test/std/strings/basic.string/string.cons/copy_assignment.pass.cpp test/std/strings/basic.string/string.cons/default_noexcept.pass.cpp test/std/strings/basic.string/string.cons/dtor_noexcept.pass.cpp test/std/strings/basic.string/string.cons/implicit_deduction_guides.pass.cpp test/std/strings/basic.string/string.cons/move_assign_noexcept.pass.cpp test/std/strings/basic.string/string.cons/move_noexcept.pass.cpp test/std/strings/basic.string/string.cons/pointer_size_alloc.pass.cpp test/std/strings/basic.string/string.cons/string_view_deduction.fail.cpp test/std/strings/basic.string/string.cons/string_view_deduction.pass.cpp test/std/strings/basic.string/string.cons/string_view_size_size_deduction.fail.cpp test/std/strings/basic.string/string.cons/string_view_size_size_deduction.pass.cpp test/std/strings/basic.string/string.modifiers/string_append/string.pass.cpp test/std/strings/basic.string/string.modifiers/string_insert/size_string.pass.cpp test/std/strings/basic.string/string.modifiers/string_op_plus_equal/string.pass.cpp test/std/strings/basic.string/string.modifiers/string_replace/iter_iter_string.pass.cpp test/std/strings/basic.string/string.modifiers/string_replace/size_size_string.pass.cpp test/std/strings/basic.string/string.nonmembers/string.special/swap_noexcept.pass.cpp test/std/strings/basic.string/string.ops/string_compare/size_size_string.pass.cpp test/std/strings/basic.string/string.ops/string_compare/string.pass.cpp test/std/strings/basic.string/string.ops/string_find.first.not.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp Index: test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp @@ -14,6 +14,7 @@ #include #include +#include "test_macros.h" #include "min_allocator.h" template @@ -154,4 +155,11 @@ test1(); } #endif + +#if TEST_STD_VER > 3 +{ // LWG 2946 +std::string s = " !"; +assert(s.rfind({"abc", 1}) == std::string::npos); +} +#endif } Index: test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp @@ -14,6 +14,7 @@ #include #include +#include "test_macros.h" #include "min_allocator.h" template @@ -154,4 +155,11 @@ test1(); } #endif + +#if TEST_STD_VER > 3 +{ // LWG 2946 +std::string s = " !"; +assert(s.find_last_of({"abc", 1}) == std::string::npos); +} +#endif } Index: test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp @@ -14,6 +14,7 @@ #include #include +#include "test_macros.h" #include "min_allocator.h" template @@ -154,4 +155,11 @@ test1(); } #endif + +#if TEST_STD_VER > 3 +{ // LWG 2946 +std::string s = " !"; +assert(s.find_last_not_of({"abc", 1}) == s.size() - 1); +} +#endif } Index: test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp @@ -14,6 +14,7 @@ #include #include +#include "test_macros.h" #include "min_allocator.h" template @@ -154,4 +155,11 @@ test1(); } #endif + +#if TEST_STD_VER > 3 +{ // LWG 2946 +std::string s = " !"; +assert(s.find_first_of({"abc", 1}) == std::string::npos); +} +#endif } Index: test/std/strings/basic.string/string.ops/string_find.first.not.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_find.first.not.of/str
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
ldionne added a comment. LGTM. All comments/questions are just for my education. Other things I did: double-check that you changed all the functions changed by https://cplusplus.github.io/LWG/lwg-defects.html#2946. Comment at: include/memory:5647 + typename __void_t::type, + typename __void_t().allocate(size_t(0)))>::type + > Sorry -- still not very fluent with how things are done in libc++, but don't we need to guard this based on C++11 at the very least because it's using `decltype`? Comment at: include/string:842 +explicit basic_string(const _Tp& __t, + typename enable_if<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value, void>::type* = 0); + Is there a reason why you use a different `enable_if` pattern here (as a default argument) and above (as a default template argument)? Comment at: include/string:1646 + class _Allocator = allocator<_CharT>, + class = typename enable_if<__is_allocator<_Allocator>::value, void>::type + > You don't need to specify the `void` in `enable_if<__is_allocator, void>::type`. There's no harm in specifying it, but I'm curious to know if there's a reason for it? Comment at: include/string:1779 template -inline _LIBCPP_INLINE_VISIBILITY +template basic_string<_CharT, _Traits, _Allocator>::basic_string(const _CharT* __s) Wow, it's terrible that we need to write this. https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
ldionne added a comment. More missed `noexcept`s. Comment at: include/string:279 +template +size_type find_first_of(const T& t, size_type pos = 0) const noexcept; // C++17 size_type find_first_of(const value_type* s, size_type pos, size_type n) const noexcept; I think you need to remove this `noexcept`. Comment at: include/string:286 +template +size_type find_last_of(const T& t, size_type pos = npos) const noexcept; // C++17 size_type find_last_of(const value_type* s, size_type pos, size_type n) const noexcept; Remove `noexcept`. Comment at: include/string:293 +template +size_type find_first_not_of(const T& t, size_type pos = 0) const noexcept; // C++17 size_type find_first_not_of(const value_type* s, size_type pos, size_type n) const noexcept; Remove `noexcept`. Comment at: include/string:307 +template +int compare(const T& t) const noexcept; // C++17 int compare(size_type pos1, size_type n1, const basic_string& str) const; Remove `noexcept`. https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
mclow.lists added inline comments. Comment at: include/string:842 +explicit basic_string(const _Tp& __t, + typename enable_if<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value, void>::type* = 0); + ldionne wrote: > Is there a reason why you use a different `enable_if` pattern here (as a > default argument) and above (as a default template argument)? One for constructors, one for non-constructors. https://stackoverflow.com/questions/17842478/select-class-constructor-using-enable-if Comment at: include/string:1646 + class _Allocator = allocator<_CharT>, + class = typename enable_if<__is_allocator<_Allocator>::value, void>::type + > ldionne wrote: > You don't need to specify the `void` in `enable_if<__is_allocator, > void>::type`. There's no harm in specifying it, but I'm curious to know if > there's a reason for it? Habit, I guess - I always put the type into `enable_if`, even if I don't use it. Sometimes I use `nullptr_t` instead of `void`. https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
ldionne added inline comments. Comment at: include/string:842 +explicit basic_string(const _Tp& __t, + typename enable_if<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value, void>::type* = 0); + mclow.lists wrote: > ldionne wrote: > > Is there a reason why you use a different `enable_if` pattern here (as a > > default argument) and above (as a default template argument)? > One for constructors, one for non-constructors. > https://stackoverflow.com/questions/17842478/select-class-constructor-using-enable-if Even after reading the SO question, I think the following would work as well? ``` template::value, void>::type> _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS explicit basic_string(const _Tp& __t); ``` Again, I'm not suggesting any change, just trying to understand, Comment at: include/string:1646 + class _Allocator = allocator<_CharT>, + class = typename enable_if<__is_allocator<_Allocator>::value, void>::type + > mclow.lists wrote: > ldionne wrote: > > You don't need to specify the `void` in `enable_if<__is_allocator, > > void>::type`. There's no harm in specifying it, but I'm curious to know if > > there's a reason for it? > Habit, I guess - I always put the type into `enable_if`, even if I don't use > it. Sometimes I use `nullptr_t` instead of `void`. Understood. https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
mclow.lists marked 5 inline comments as done. mclow.lists added inline comments. Comment at: include/memory:5647 + typename __void_t::type, + typename __void_t().allocate(size_t(0)))>::type + > ldionne wrote: > Sorry -- still not very fluent with how things are done in libc++, but don't > we need to guard this based on C++11 at the very least because it's using > `decltype`? libc++ has an emulation of `decltype` for C++03, based on `typeof`. It's not perfect, but it works in a lot of cases. Comment at: include/string:842 +explicit basic_string(const _Tp& __t, + typename enable_if<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value, void>::type* = 0); + ldionne wrote: > mclow.lists wrote: > > ldionne wrote: > > > Is there a reason why you use a different `enable_if` pattern here (as a > > > default argument) and above (as a default template argument)? > > One for constructors, one for non-constructors. > > https://stackoverflow.com/questions/17842478/select-class-constructor-using-enable-if > Even after reading the SO question, I think the following would work as well? > > ``` > template enable_if<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value, > void>::type> > _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS > explicit basic_string(const _Tp& __t); > ``` > > Again, I'm not suggesting any change, just trying to understand, I tried this locally, and it seems to work. Updated patch incoming. https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
mclow.lists updated this revision to Diff 153178. mclow.lists marked 2 inline comments as done. mclow.lists added a comment. Update diff per Louis' suggestion. Remove noexcepts from the synopsis. https://reviews.llvm.org/D48616 Files: include/memory include/string test/std/strings/basic.string/string.cons/copy_assignment.pass.cpp test/std/strings/basic.string/string.cons/default_noexcept.pass.cpp test/std/strings/basic.string/string.cons/dtor_noexcept.pass.cpp test/std/strings/basic.string/string.cons/implicit_deduction_guides.pass.cpp test/std/strings/basic.string/string.cons/move_assign_noexcept.pass.cpp test/std/strings/basic.string/string.cons/move_noexcept.pass.cpp test/std/strings/basic.string/string.cons/pointer_size_alloc.pass.cpp test/std/strings/basic.string/string.cons/string_view_deduction.fail.cpp test/std/strings/basic.string/string.cons/string_view_deduction.pass.cpp test/std/strings/basic.string/string.cons/string_view_size_size_deduction.fail.cpp test/std/strings/basic.string/string.cons/string_view_size_size_deduction.pass.cpp test/std/strings/basic.string/string.modifiers/string_append/string.pass.cpp test/std/strings/basic.string/string.modifiers/string_insert/size_string.pass.cpp test/std/strings/basic.string/string.modifiers/string_op_plus_equal/string.pass.cpp test/std/strings/basic.string/string.modifiers/string_replace/iter_iter_string.pass.cpp test/std/strings/basic.string/string.modifiers/string_replace/size_size_string.pass.cpp test/std/strings/basic.string/string.nonmembers/string.special/swap_noexcept.pass.cpp test/std/strings/basic.string/string.ops/string_compare/size_size_string.pass.cpp test/std/strings/basic.string/string.ops/string_compare/string.pass.cpp test/std/strings/basic.string/string.ops/string_find.first.not.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp Index: test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp @@ -14,6 +14,7 @@ #include #include +#include "test_macros.h" #include "min_allocator.h" template @@ -154,4 +155,11 @@ test1(); } #endif + +#if TEST_STD_VER > 3 +{ // LWG 2946 +std::string s = " !"; +assert(s.rfind({"abc", 1}) == std::string::npos); +} +#endif } Index: test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp @@ -14,6 +14,7 @@ #include #include +#include "test_macros.h" #include "min_allocator.h" template @@ -154,4 +155,11 @@ test1(); } #endif + +#if TEST_STD_VER > 3 +{ // LWG 2946 +std::string s = " !"; +assert(s.find_last_of({"abc", 1}) == std::string::npos); +} +#endif } Index: test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp @@ -14,6 +14,7 @@ #include #include +#include "test_macros.h" #include "min_allocator.h" template @@ -154,4 +155,11 @@ test1(); } #endif + +#if TEST_STD_VER > 3 +{ // LWG 2946 +std::string s = " !"; +assert(s.find_last_not_of({"abc", 1}) == s.size() - 1); +} +#endif } Index: test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp @@ -14,6 +14,7 @@ #include #include +#include "test_macros.h" #include "min_allocator.h" template @@ -154,4 +155,11 @@ test1(); } #endif + +#if TEST_STD_VER > 3 +{ // LWG 2946 +std::string s = " !"; +assert(s.find_first_of({"abc", 1}) == std::string::npos); +} +#endif } Index: test/std/strings/basic.string/string.ops/string_find.first.not.of/string_size.pass.cpp === --- test/std
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
ldionne added inline comments. Comment at: include/string:836 _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS -basic_string(const _Tp& __t, size_type __pos, size_type __n, - const allocator_type& __a = allocator_type(), - typename enable_if<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value, void>::type* = 0); -_LIBCPP_INLINE_VISIBILITY explicit -basic_string(__self_view __sv); -_LIBCPP_INLINE_VISIBILITY -basic_string(__self_view __sv, const _Allocator& __a); +explicit basic_string(const _Tp& __t, size_type __pos, size_type __n, + const allocator_type& __a = allocator_type()); I believe an unwelcome `explicit` snuck in here. Comment at: include/string:839 + +// template +// _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS Did you mean to leave those comments there? Comment at: include/string:856 +_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS +explicit basic_string(const _Tp& __t, const allocator_type& __a); + I think this `explicit` shouldn't be there, too. https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
tcanens added inline comments. Comment at: include/string:856 +_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS +explicit basic_string(const _Tp& __t, const allocator_type& __a); + ldionne wrote: > I think this `explicit` shouldn't be there, too. This one is `explicit` in the standard (because it had a default argument: `template explicit basic_string(const T& t, const Allocator& a = Allocator());`) Comment at: include/string:1987 { __self_view __sv = __self_view(__t).substr(__pos, __n); `__self_view(__t)` is wrong - the wording was intentionally crafted to require the conversion to `basic_string_view` to be done using copy-initialization. Using direct-initialization can potentially result in different overload resolution results. https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
ldionne added inline comments. Comment at: include/string:856 +_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS +explicit basic_string(const _Tp& __t, const allocator_type& __a); + tcanens wrote: > ldionne wrote: > > I think this `explicit` shouldn't be there, too. > This one is `explicit` in the standard (because it had a default argument: > `template explicit basic_string(const T& t, const Allocator& a = > Allocator());`) Ah, you're right. So basically ``` template explicit basic_string(const T& t, const Allocator& a = Allocator()); ``` is implemented as two overloads ``` template explicit basic_string(const T& t); ``` and ``` template explicit basic_string(const T& t, const Allocator& a); ``` https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
mclow.lists marked 5 inline comments as done. mclow.lists added inline comments. Comment at: include/string:836 _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS -basic_string(const _Tp& __t, size_type __pos, size_type __n, - const allocator_type& __a = allocator_type(), - typename enable_if<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value, void>::type* = 0); -_LIBCPP_INLINE_VISIBILITY explicit -basic_string(__self_view __sv); -_LIBCPP_INLINE_VISIBILITY -basic_string(__self_view __sv, const _Allocator& __a); +explicit basic_string(const _Tp& __t, size_type __pos, size_type __n, + const allocator_type& __a = allocator_type()); ldionne wrote: > I believe an unwelcome `explicit` snuck in here. Right. Comment at: include/string:839 + +// template +// _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS ldionne wrote: > Did you mean to leave those comments there? Nope. Thanks. Comment at: include/string:856 +_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS +explicit basic_string(const _Tp& __t, const allocator_type& __a); + ldionne wrote: > tcanens wrote: > > ldionne wrote: > > > I think this `explicit` shouldn't be there, too. > > This one is `explicit` in the standard (because it had a default argument: > > `template explicit basic_string(const T& t, const Allocator& a = > > Allocator());`) > Ah, you're right. So basically > > ``` > template > explicit basic_string(const T& t, > const Allocator& a = Allocator()); > ``` > > is implemented as two overloads > > ``` > template > explicit basic_string(const T& t); > ``` > > and > > ``` > template > explicit basic_string(const T& t, const Allocator& a); > ``` > Right. We frequently split defaulted allocator ctors. Comment at: include/string:1987 { __self_view __sv = __self_view(__t).substr(__pos, __n); tcanens wrote: > `__self_view(__t)` is wrong - the wording was intentionally crafted to > require the conversion to `basic_string_view` to be done using > copy-initialization. Using direct-initialization can potentially result in > different overload resolution results. Nice catch! https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
mclow.lists updated this revision to Diff 153432. mclow.lists marked 3 inline comments as done. mclow.lists added a comment. Update in response to comments. https://reviews.llvm.org/D48616 Files: include/memory include/string test/std/strings/basic.string/string.cons/copy_assignment.pass.cpp test/std/strings/basic.string/string.cons/default_noexcept.pass.cpp test/std/strings/basic.string/string.cons/dtor_noexcept.pass.cpp test/std/strings/basic.string/string.cons/implicit_deduction_guides.pass.cpp test/std/strings/basic.string/string.cons/move_assign_noexcept.pass.cpp test/std/strings/basic.string/string.cons/move_noexcept.pass.cpp test/std/strings/basic.string/string.cons/pointer_size_alloc.pass.cpp test/std/strings/basic.string/string.cons/string_view_deduction.fail.cpp test/std/strings/basic.string/string.cons/string_view_deduction.pass.cpp test/std/strings/basic.string/string.cons/string_view_size_size_deduction.fail.cpp test/std/strings/basic.string/string.cons/string_view_size_size_deduction.pass.cpp test/std/strings/basic.string/string.modifiers/string_append/string.pass.cpp test/std/strings/basic.string/string.modifiers/string_insert/size_string.pass.cpp test/std/strings/basic.string/string.modifiers/string_op_plus_equal/string.pass.cpp test/std/strings/basic.string/string.modifiers/string_replace/iter_iter_string.pass.cpp test/std/strings/basic.string/string.modifiers/string_replace/size_size_string.pass.cpp test/std/strings/basic.string/string.nonmembers/string.special/swap_noexcept.pass.cpp test/std/strings/basic.string/string.ops/string_compare/size_size_string.pass.cpp test/std/strings/basic.string/string.ops/string_compare/string.pass.cpp test/std/strings/basic.string/string.ops/string_find.first.not.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_find/string_size.pass.cpp test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp Index: test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_rfind/string_size.pass.cpp @@ -14,6 +14,7 @@ #include #include +#include "test_macros.h" #include "min_allocator.h" template @@ -154,4 +155,11 @@ test1(); } #endif + +#if TEST_STD_VER > 3 +{ // LWG 2946 +std::string s = " !"; +assert(s.rfind({"abc", 1}) == std::string::npos); +} +#endif } Index: test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_find.last.of/string_size.pass.cpp @@ -14,6 +14,7 @@ #include #include +#include "test_macros.h" #include "min_allocator.h" template @@ -154,4 +155,11 @@ test1(); } #endif + +#if TEST_STD_VER > 3 +{ // LWG 2946 +std::string s = " !"; +assert(s.find_last_of({"abc", 1}) == std::string::npos); +} +#endif } Index: test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_find.last.not.of/string_size.pass.cpp @@ -14,6 +14,7 @@ #include #include +#include "test_macros.h" #include "min_allocator.h" template @@ -154,4 +155,11 @@ test1(); } #endif + +#if TEST_STD_VER > 3 +{ // LWG 2946 +std::string s = " !"; +assert(s.find_last_not_of({"abc", 1}) == s.size() - 1); +} +#endif } Index: test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp +++ test/std/strings/basic.string/string.ops/string_find.first.of/string_size.pass.cpp @@ -14,6 +14,7 @@ #include #include +#include "test_macros.h" #include "min_allocator.h" template @@ -154,4 +155,11 @@ test1(); } #endif + +#if TEST_STD_VER > 3 +{ // LWG 2946 +std::string s = " !"; +assert(s.find_first_of({"abc", 1}) == std::string::npos); +} +#endif } Index: test/std/strings/basic.string/string.ops/string_find.first.not.of/string_size.pass.cpp === --- test/std/strings/basic.string/string.ops/string
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
mclow.lists closed this revision. mclow.lists added a comment. Committed as revision 336132. https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits