From 49d83e7bf4889c47317d098b5b5aa943eb7fdab2 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 25 Nov 2024 12:12:30 +0800 Subject: [PATCH 1/5] Implement LWG-4135 and improve test coverage for P1209R0 --- stl/inc/forward_list | 2 +- stl/inc/list | 2 +- tests/std/test.lst | 1 + .../test.cpp | 117 ------------ .../std/tests/P1209R0_erase_if_erase/env.lst | 4 + .../std/tests/P1209R0_erase_if_erase/test.cpp | 177 ++++++++++++++++++ 6 files changed, 184 insertions(+), 119 deletions(-) create mode 100644 tests/std/tests/P1209R0_erase_if_erase/env.lst create mode 100644 tests/std/tests/P1209R0_erase_if_erase/test.cpp diff --git a/stl/inc/forward_list b/stl/inc/forward_list index 993bb3ac575..0d556e5639a 100644 --- a/stl/inc/forward_list +++ b/stl/inc/forward_list @@ -1616,7 +1616,7 @@ _NODISCARD bool operator>=(const forward_list<_Ty, _Alloc>& _Left, const forward #if _HAS_CXX20 _EXPORT_STD template forward_list<_Ty, _Alloc>::size_type erase(forward_list<_Ty, _Alloc>& _Cont, const _Uty& _Val) { - return _Cont.remove_if([&](_Ty& _Elem) -> bool { return _Elem == _Val; }); + return _Cont.remove_if([&](const _Ty& _Elem) -> bool { return _Elem == _Val; }); } _EXPORT_STD template diff --git a/stl/inc/list b/stl/inc/list index d98a72d281e..58d87142ffe 100644 --- a/stl/inc/list +++ b/stl/inc/list @@ -1921,7 +1921,7 @@ _NODISCARD bool operator>=(const list<_Ty, _Alloc>& _Left, const list<_Ty, _Allo #if _HAS_CXX20 _EXPORT_STD template list<_Ty, _Alloc>::size_type erase(list<_Ty, _Alloc>& _Cont, const _Uty& _Val) { - return _Cont.remove_if([&](_Ty& _Elem) -> bool { return _Elem == _Val; }); + return _Cont.remove_if([&](const _Ty& _Elem) -> bool { return _Elem == _Val; }); } _EXPORT_STD template diff --git a/tests/std/test.lst b/tests/std/test.lst index 42163db9973..d8a6919fde3 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -588,6 +588,7 @@ tests\P1206R7_vector_assign_range tests\P1206R7_vector_from_range tests\P1206R7_vector_insert_range tests\P1208R6_source_location +tests\P1209R0_erase_if_erase tests\P1223R5_ranges_alg_find_last tests\P1223R5_ranges_alg_find_last_if tests\P1223R5_ranges_alg_find_last_if_not diff --git a/tests/std/tests/Dev11_0000000_user_defined_literals/test.cpp b/tests/std/tests/Dev11_0000000_user_defined_literals/test.cpp index de11548135a..21fc66fa561 100644 --- a/tests/std/tests/Dev11_0000000_user_defined_literals/test.cpp +++ b/tests/std/tests/Dev11_0000000_user_defined_literals/test.cpp @@ -435,123 +435,6 @@ int main() { assert(!const_us.contains(1)); assert(!const_ums.contains(3)); } - - // P1209R0 erase_if(), erase() - { - // Note that the standard actually requires these to be copyable. As an extension, we want - // to ensure we don't copy them, because copying some functors (e.g. std::function) is comparatively - // expensive, and even for relatively cheap to copy function objects we care (somewhat) about debug - // mode perf. - struct no_copy { - no_copy() = default; - no_copy(const no_copy&) = delete; - no_copy(no_copy&&) = default; - no_copy& operator=(const no_copy&) = delete; - no_copy& operator=(no_copy&&) = delete; - }; - - struct is_vowel : no_copy { - bool operator()(const char c) const { - return c == 'a' || c == 'e' || c == 'i' || c == 'o' || c == 'u'; - } - }; - - std::string str1{"cute fluffy kittens"}; - const auto str1_removed = std::erase_if(str1, is_vowel{}); - assert(str1 == "ct flffy kttns"); - assert(str1_removed == 5); - - std::string str2{"asynchronous beat"}; - const auto str2_removed = std::erase(str2, 'a'); - assert(str2 == "synchronous bet"); - assert(str2_removed == 2); - - struct is_odd : no_copy { - bool operator()(const int i) const { - return i % 2 != 0; - } - }; - - std::deque d{1, 2, 3, 4, 5, 6, 7, 6, 5, 4, 3, 2, 1}; - const auto d_removed = std::erase_if(d, is_odd{}); - assert((d == std::deque{2, 4, 6, 6, 4, 2})); - assert(d_removed == 7); - const auto d_removed2 = std::erase(d, 4); - assert((d == std::deque{2, 6, 6, 2})); - assert(d_removed2 == 2); - - std::vector v{1, 2, 3, 4, 5, 6, 7, 6, 5, 4, 3, 2, 1}; - const auto v_removed = std::erase_if(v, is_odd{}); - assert((v == std::vector{2, 4, 6, 6, 4, 2})); - assert(v_removed == 7); - const auto v_removed2 = std::erase(v, 4); - assert((v == std::vector{2, 6, 6, 2})); - assert(v_removed2 == 2); - - std::forward_list fl{1, 2, 3, 4, 5, 6, 7, 6, 5, 4, 3, 2, 1}; - const auto fl_removed = std::erase_if(fl, is_odd{}); - assert((fl == std::forward_list{2, 4, 6, 6, 4, 2})); - assert(fl_removed == 7); - const auto fl_removed2 = std::erase(fl, 4); - assert((fl == std::forward_list{2, 6, 6, 2})); - assert(fl_removed2 == 2); - - std::list l{1, 2, 3, 4, 5, 6, 7, 6, 5, 4, 3, 2, 1}; - const auto l_removed = std::erase_if(l, is_odd{}); - assert((l == std::list{2, 4, 6, 6, 4, 2})); - assert(l_removed == 7); - const auto l_removed2 = std::erase(l, 4); - assert((l == std::list{2, 6, 6, 2})); - assert(l_removed2 == 2); - - struct is_first_odd : no_copy { - bool operator()(const std::pair& p) const { - return p.first % 2 != 0; - } - }; - - std::map m{{1, 10}, {2, 20}, {3, 30}, {4, 40}, {5, 50}, {6, 60}, {7, 70}}; - const auto m_removed = std::erase_if(m, is_first_odd{}); - assert((m == std::map{{2, 20}, {4, 40}, {6, 60}})); - assert(m_removed == 4); - - std::multimap mm{{1, 10}, {2, 20}, {3, 30}, {4, 40}, {5, 50}, {6, 60}, {7, 70}}; - const auto mm_removed = std::erase_if(mm, is_first_odd{}); - assert((mm == std::multimap{{2, 20}, {4, 40}, {6, 60}})); - assert(mm_removed == 4); - - std::set s{1, 2, 3, 4, 5, 6, 7}; - const auto s_removed = std::erase_if(s, is_odd{}); - assert((s == std::set{2, 4, 6})); - assert(s_removed == 4); - - std::multiset ms{1, 2, 3, 4, 5, 6, 7}; - const auto ms_removed = std::erase_if(ms, is_odd{}); - assert((ms == std::multiset{2, 4, 6})); - assert(ms_removed == 4); - - // Note that unordered equality considers permutations. - - std::unordered_map um{{1, 10}, {2, 20}, {3, 30}, {4, 40}, {5, 50}, {6, 60}, {7, 70}}; - const auto um_removed = std::erase_if(um, is_first_odd{}); - assert((um == std::unordered_map{{2, 20}, {4, 40}, {6, 60}})); - assert(um_removed == 4); - - std::unordered_multimap umm{{1, 10}, {2, 20}, {3, 30}, {4, 40}, {5, 50}, {6, 60}, {7, 70}}; - const auto umm_removed = std::erase_if(umm, is_first_odd{}); - assert((umm == std::unordered_multimap{{2, 20}, {4, 40}, {6, 60}})); - assert(umm_removed == 4); - - std::unordered_set us{1, 2, 3, 4, 5, 6, 7}; - const auto us_removed = std::erase_if(us, is_odd{}); - assert((us == std::unordered_set{2, 4, 6})); - assert(us_removed == 4); - - std::unordered_multiset ums{1, 2, 3, 4, 5, 6, 7}; - const auto ums_removed = std::erase_if(ums, is_odd{}); - assert((ums == std::unordered_multiset{2, 4, 6})); - assert(ums_removed == 4); - } #endif // _HAS_CXX20 // P0007R1 as_const() diff --git a/tests/std/tests/P1209R0_erase_if_erase/env.lst b/tests/std/tests/P1209R0_erase_if_erase/env.lst new file mode 100644 index 00000000000..351a8293d9d --- /dev/null +++ b/tests/std/tests/P1209R0_erase_if_erase/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\usual_20_matrix.lst diff --git a/tests/std/tests/P1209R0_erase_if_erase/test.cpp b/tests/std/tests/P1209R0_erase_if_erase/test.cpp new file mode 100644 index 00000000000..f0449a9a359 --- /dev/null +++ b/tests/std/tests/P1209R0_erase_if_erase/test.cpp @@ -0,0 +1,177 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +template +constexpr bool is_unsized_container = false; +template +constexpr bool is_unsized_container> = true; + +// Note that the standard actually requires these to be copyable. As an extension, we want to ensure we don't copy them, +// because copying some functors (e.g. std::function) is comparatively expensive, and even for relatively cheap to copy +// function objects we care (somewhat) about debug mode perf. +struct no_copy { + no_copy() = default; + no_copy(const no_copy&) = delete; + no_copy(no_copy&&) = default; + no_copy& operator=(const no_copy&) = delete; + no_copy& operator=(no_copy&&) = delete; +}; + +struct is_vowel : no_copy { + constexpr bool operator()(const char c) const { + return c == 'a' || c == 'e' || c == 'i' || c == 'o' || c == 'u'; + } +}; + +struct is_odd : no_copy { + constexpr bool operator()(const int i) const { + return i % 2 != 0; + } +}; + +struct is_first_odd : no_copy { + bool operator()(const std::pair& p) const { + return p.first % 2 != 0; + } +}; + +constexpr bool test_string() { + std::string str1{"cute fluffy kittens"}; + const auto str1_removed = std::erase_if(str1, is_vowel{}); + assert(str1 == "ct flffy kttns"); + assert(str1_removed == 5); + + std::string str2{"asynchronous beat"}; + const auto str2_removed = std::erase(str2, 'a'); + assert(str2 == "synchronous bet"); + assert(str2_removed == 2); + + return true; +} + +template +constexpr bool test_sequence_container() { + SequenceContainer cont1{42, 1729}; + const auto cont1_removed = std::erase(cont1, 42); + if constexpr (!is_unsized_container) { + assert(cont1.size() == 1); + } + assert(cont1.front() == 1729); + assert(cont1_removed == 1); + + SequenceContainer cont2{17, 42, 29}; + const auto cont2_removed = std::erase_if(cont2, is_odd{}); + if constexpr (!is_unsized_container) { + assert(cont2.size() == 1); + } + assert(cont2.front() == 42); + assert(cont2_removed == 2); + + return true; +} + +// Also test LWG-4135 "The helper lambda of std::erase for list should specify return type as bool" + +template +struct pinned_condition { + pinned_condition() = default; + pinned_condition(const pinned_condition&) = delete; + pinned_condition& operator=(const pinned_condition&) = delete; + + operator bool() const { + return B; + } + + pinned_condition operator!() const { + return {}; + } +}; + +struct lwg_4135_src { + static constexpr pinned_condition result{}; + + friend void operator==(int&, const lwg_4135_src&) = delete; + friend void operator==(const lwg_4135_src&, int&) = delete; + + friend pinned_condition& operator==(const lwg_4135_src&, const int&) { + return const_cast&>(result); + } + friend pinned_condition& operator==(const int&, const lwg_4135_src&) { + return const_cast&>(result); + } +}; + +template +void test_list_erase() { + ListContainer ls2{42, 1729}; + const auto ls2_removed = std::erase(ls2, lwg_4135_src{}); + assert(ls2.empty()); + assert(ls2_removed == 2); +} + +static_assert(test_string()); +static_assert(test_sequence_container>()); + +int main() { + test_string(); + test_sequence_container>(); + test_sequence_container>(); + test_sequence_container>(); + test_sequence_container>(); + + test_list_erase>(); + test_list_erase>(); + + std::map m{{1, 10}, {2, 20}, {3, 30}, {4, 40}, {5, 50}, {6, 60}, {7, 70}}; + const auto m_removed = std::erase_if(m, is_first_odd{}); + assert((m == std::map{{2, 20}, {4, 40}, {6, 60}})); + assert(m_removed == 4); + + std::multimap mm{{1, 10}, {2, 20}, {3, 30}, {4, 40}, {5, 50}, {6, 60}, {7, 70}}; + const auto mm_removed = std::erase_if(mm, is_first_odd{}); + assert((mm == std::multimap{{2, 20}, {4, 40}, {6, 60}})); + assert(mm_removed == 4); + + std::set s{1, 2, 3, 4, 5, 6, 7}; + const auto s_removed = std::erase_if(s, is_odd{}); + assert((s == std::set{2, 4, 6})); + assert(s_removed == 4); + + std::multiset ms{1, 2, 3, 4, 5, 6, 7}; + const auto ms_removed = std::erase_if(ms, is_odd{}); + assert((ms == std::multiset{2, 4, 6})); + assert(ms_removed == 4); + + // Note that unordered equality considers permutations. + + std::unordered_map um{{1, 10}, {2, 20}, {3, 30}, {4, 40}, {5, 50}, {6, 60}, {7, 70}}; + const auto um_removed = std::erase_if(um, is_first_odd{}); + assert((um == std::unordered_map{{2, 20}, {4, 40}, {6, 60}})); + assert(um_removed == 4); + + std::unordered_multimap umm{{1, 10}, {2, 20}, {3, 30}, {4, 40}, {5, 50}, {6, 60}, {7, 70}}; + const auto umm_removed = std::erase_if(umm, is_first_odd{}); + assert((umm == std::unordered_multimap{{2, 20}, {4, 40}, {6, 60}})); + assert(umm_removed == 4); + + std::unordered_set us{1, 2, 3, 4, 5, 6, 7}; + const auto us_removed = std::erase_if(us, is_odd{}); + assert((us == std::unordered_set{2, 4, 6})); + assert(us_removed == 4); + + std::unordered_multiset ums{1, 2, 3, 4, 5, 6, 7}; + const auto ums_removed = std::erase_if(ums, is_odd{}); + assert((ums == std::unordered_multiset{2, 4, 6})); + assert(ums_removed == 4); +} From 5a353ca616e7671d741257916b560205a9f06b68 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 2 Dec 2024 07:02:58 -0800 Subject: [PATCH 2/5] Include `` for `std::pair`. --- tests/std/tests/P1209R0_erase_if_erase/test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/std/tests/P1209R0_erase_if_erase/test.cpp b/tests/std/tests/P1209R0_erase_if_erase/test.cpp index f0449a9a359..641a6679a43 100644 --- a/tests/std/tests/P1209R0_erase_if_erase/test.cpp +++ b/tests/std/tests/P1209R0_erase_if_erase/test.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include template From de43b858627375275f249adec1e073001c36e6d4 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 2 Dec 2024 07:43:17 -0800 Subject: [PATCH 3/5] Improve sequence container test coverage. This exercises both adjacent and non-adjacent removals, inspects the container's entire contents again, and is somewhat clearer about distinguishing element values from the number of elements removed. (In the `std::erase` case, the container no longer contains odd elements, so removing 5 elements is clear.) This is, of course, the first digits of pi. --- .../std/tests/P1209R0_erase_if_erase/test.cpp | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/tests/std/tests/P1209R0_erase_if_erase/test.cpp b/tests/std/tests/P1209R0_erase_if_erase/test.cpp index 641a6679a43..b7760deafde 100644 --- a/tests/std/tests/P1209R0_erase_if_erase/test.cpp +++ b/tests/std/tests/P1209R0_erase_if_erase/test.cpp @@ -13,11 +13,6 @@ #include #include -template -constexpr bool is_unsized_container = false; -template -constexpr bool is_unsized_container> = true; - // Note that the standard actually requires these to be copyable. As an extension, we want to ensure we don't copy them, // because copying some functors (e.g. std::function) is comparatively expensive, and even for relatively cheap to copy // function objects we care (somewhat) about debug mode perf. @@ -63,21 +58,22 @@ constexpr bool test_string() { template constexpr bool test_sequence_container() { - SequenceContainer cont1{42, 1729}; - const auto cont1_removed = std::erase(cont1, 42); - if constexpr (!is_unsized_container) { - assert(cont1.size() == 1); + SequenceContainer c{3, 1, 4, 1, 5, 9, 2, 6, 5, 3, 5, 8, 9, 7, 9, 3, 2, 3, 8, 4, 6, 2, 6, 4, 3, 3, 8, 3, 2, 7, 9, 5, + 0, 2, 8, 8, 4, 1, 9, 7, 1, 6, 9, 3, 9, 9, 3, 7, 5, 1, 0}; + + { + const auto removed1 = std::erase_if(c, is_odd{}); + assert(removed1 == 31); + const SequenceContainer expected1{4, 2, 6, 8, 2, 8, 4, 6, 2, 6, 4, 8, 2, 0, 2, 8, 8, 4, 6, 0}; + assert(c == expected1); } - assert(cont1.front() == 1729); - assert(cont1_removed == 1); - SequenceContainer cont2{17, 42, 29}; - const auto cont2_removed = std::erase_if(cont2, is_odd{}); - if constexpr (!is_unsized_container) { - assert(cont2.size() == 1); + { + const auto removed2 = std::erase(c, 8); + assert(removed2 == 5); + const SequenceContainer expected2{4, 2, 6, 2, 4, 6, 2, 6, 4, 2, 0, 2, 4, 6, 0}; + assert(c == expected2); } - assert(cont2.front() == 42); - assert(cont2_removed == 2); return true; } From 68451f3fedcd219baa46261b8808f799f141a870 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 2 Dec 2024 08:02:33 -0800 Subject: [PATCH 4/5] Return `const pinned_condition&`. --- tests/std/tests/P1209R0_erase_if_erase/test.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/std/tests/P1209R0_erase_if_erase/test.cpp b/tests/std/tests/P1209R0_erase_if_erase/test.cpp index b7760deafde..b7913e61afd 100644 --- a/tests/std/tests/P1209R0_erase_if_erase/test.cpp +++ b/tests/std/tests/P1209R0_erase_if_erase/test.cpp @@ -101,11 +101,11 @@ struct lwg_4135_src { friend void operator==(int&, const lwg_4135_src&) = delete; friend void operator==(const lwg_4135_src&, int&) = delete; - friend pinned_condition& operator==(const lwg_4135_src&, const int&) { - return const_cast&>(result); + friend const pinned_condition& operator==(const lwg_4135_src&, const int&) { + return result; } - friend pinned_condition& operator==(const int&, const lwg_4135_src&) { - return const_cast&>(result); + friend const pinned_condition& operator==(const int&, const lwg_4135_src&) { + return result; } }; From 033dc4771f8d74132c92aecd275e2aa5bbbb5c64 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 2 Dec 2024 08:14:49 -0800 Subject: [PATCH 5/5] Add `explicit` to `pinned_condition()`. --- tests/std/tests/P1209R0_erase_if_erase/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/P1209R0_erase_if_erase/test.cpp b/tests/std/tests/P1209R0_erase_if_erase/test.cpp index b7913e61afd..5f28d45b470 100644 --- a/tests/std/tests/P1209R0_erase_if_erase/test.cpp +++ b/tests/std/tests/P1209R0_erase_if_erase/test.cpp @@ -82,7 +82,7 @@ constexpr bool test_sequence_container() { template struct pinned_condition { - pinned_condition() = default; + explicit pinned_condition() = default; pinned_condition(const pinned_condition&) = delete; pinned_condition& operator=(const pinned_condition&) = delete;