From 39689b4096f13257d05af367988ec821b26e93e7 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Fri, 27 Jan 2023 23:43:33 +0700 Subject: [PATCH 01/16] fix ranges::minmax --- stl/inc/algorithm | 7 ++++++- tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp | 11 +++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index d19b1e9aac..a75b3fce09 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -10102,9 +10102,14 @@ namespace ranges { if constexpr (forward_range<_Rng> && _Prefer_iterator_copies>) { const auto _Found = _RANGES _Minmax_element_unchecked( _STD move(_UFirst), _STD move(_ULast), _Pass_fn(_Pred), _Pass_fn(_Proj)); + if (_Found.min == _Found.max) { + _Vty _Temp{*_Found.min}; + return {_Temp, _Temp}; + } return {static_cast<_Vty>(*_Found.min), static_cast<_Vty>(*_Found.max)}; } else { - minmax_result<_Vty> _Found = {static_cast<_Vty>(*_UFirst), static_cast<_Vty>(*_UFirst)}; + _Vty _Temp{*_UFirst}; + minmax_result<_Vty> _Found = {_Temp, _Temp}; if (_UFirst == _ULast) { return _Found; } diff --git a/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp b/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp index 394d3ec2c5..b8215bacf3 100644 --- a/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp @@ -361,6 +361,16 @@ void test_gh_1893() { ASSERT(val == "meow"); } +void test_gh_2900() { + // GH-2900: : ranges::minmax initializes minmax_result with the moved value + { + vector v{"1"}; + auto [min, max] = ranges::minmax(ranges::subrange{make_move_iterator(v.begin()), make_move_iterator(v.end())}); + assert(min == "1"); + assert(max == "1"); + } +} + int main() { STATIC_ASSERT((nonrange_tests(), true)); nonrange_tests(); @@ -378,4 +388,5 @@ int main() { test_in(); test_gh_1893(); + test_gh_2900(); } From f0b98f17cbc13b50515896ed3d1ce04bd29b7ee6 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Sat, 28 Jan 2023 00:19:45 +0700 Subject: [PATCH 02/16] move last usage of `_Temp` Co-authored-by: Nicole Mazzuca --- stl/inc/algorithm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index a75b3fce09..530c271eb5 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -10104,12 +10104,12 @@ namespace ranges { _STD move(_UFirst), _STD move(_ULast), _Pass_fn(_Pred), _Pass_fn(_Proj)); if (_Found.min == _Found.max) { _Vty _Temp{*_Found.min}; - return {_Temp, _Temp}; + return {_Temp, _STD move(_Temp)}; } return {static_cast<_Vty>(*_Found.min), static_cast<_Vty>(*_Found.max)}; } else { _Vty _Temp{*_UFirst}; - minmax_result<_Vty> _Found = {_Temp, _Temp}; + minmax_result<_Vty> _Found = {_Temp, _STD move(_Temp)}; if (_UFirst == _ULast) { return _Found; } From d5b9cdb650d877e5c6b890be8f4108a4ed3dd781 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Sat, 28 Jan 2023 00:21:40 +0700 Subject: [PATCH 03/16] Unexpectedly Passed Tests --- tests/libcxx/expected_results.txt | 1 - tests/libcxx/skipped_tests.txt | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/libcxx/expected_results.txt b/tests/libcxx/expected_results.txt index 764505bbc2..3898a06a55 100644 --- a/tests/libcxx/expected_results.txt +++ b/tests/libcxx/expected_results.txt @@ -908,7 +908,6 @@ std/algorithms/alg.sorting/alg.heap.operations/push.heap/ranges_push_heap.pass.c std/algorithms/alg.sorting/alg.heap.operations/sort.heap/ranges_sort_heap.pass.cpp FAIL std/algorithms/alg.sorting/alg.merge/ranges_inplace_merge.pass.cpp FAIL std/algorithms/alg.sorting/alg.merge/ranges_merge.pass.cpp FAIL -std/algorithms/alg.sorting/alg.min.max/ranges.minmax.pass.cpp FAIL std/algorithms/alg.sorting/alg.min.max/requires_forward_iterator.fail.cpp FAIL std/algorithms/alg.sorting/alg.nth.element/ranges_nth_element.pass.cpp FAIL std/algorithms/alg.sorting/alg.partitions/ranges.is_partitioned.pass.cpp:0 FAIL diff --git a/tests/libcxx/skipped_tests.txt b/tests/libcxx/skipped_tests.txt index 428e61c57c..c3ae76367e 100644 --- a/tests/libcxx/skipped_tests.txt +++ b/tests/libcxx/skipped_tests.txt @@ -908,7 +908,6 @@ algorithms\alg.sorting\alg.heap.operations\push.heap\ranges_push_heap.pass.cpp algorithms\alg.sorting\alg.heap.operations\sort.heap\ranges_sort_heap.pass.cpp algorithms\alg.sorting\alg.merge\ranges_inplace_merge.pass.cpp algorithms\alg.sorting\alg.merge\ranges_merge.pass.cpp -algorithms\alg.sorting\alg.min.max\ranges.minmax.pass.cpp algorithms\alg.sorting\alg.min.max\requires_forward_iterator.fail.cpp algorithms\alg.sorting\alg.nth.element\ranges_nth_element.pass.cpp algorithms\alg.sorting\alg.partitions\ranges.is_partitioned.pass.cpp From e993f4d1006291af368bbcacf4e692f0031434a7 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Sat, 28 Jan 2023 14:23:17 +0700 Subject: [PATCH 04/16] code review suggestions Co-authored-by: Alex Guteniev Co-authored-by: timsong-cpp --- stl/inc/algorithm | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 530c271eb5..954717af22 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -10102,13 +10102,15 @@ namespace ranges { if constexpr (forward_range<_Rng> && _Prefer_iterator_copies>) { const auto _Found = _RANGES _Minmax_element_unchecked( _STD move(_UFirst), _STD move(_ULast), _Pass_fn(_Pred), _Pass_fn(_Proj)); - if (_Found.min == _Found.max) { - _Vty _Temp{*_Found.min}; - return {_Temp, _STD move(_Temp)}; + if constexpr (!is_pointer_v>) { + if (_Found.min == _Found.max) { + auto _Temp = static_cast<_Vty>(*_Found.min); + return {_Temp, _STD move(_Temp)}; + } } return {static_cast<_Vty>(*_Found.min), static_cast<_Vty>(*_Found.max)}; } else { - _Vty _Temp{*_UFirst}; + auto _Temp = static_cast<_Vty>(*_UFirst); minmax_result<_Vty> _Found = {_Temp, _STD move(_Temp)}; if (_UFirst == _ULast) { return _Found; From a2cbb8b0b33ff7235bd24aa2747d5a225a4db398 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Sat, 28 Jan 2023 19:44:50 +0700 Subject: [PATCH 05/16] add a comment about `if constexpr` --- stl/inc/algorithm | 3 +++ 1 file changed, 3 insertions(+) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 954717af22..ccec718c41 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -10102,6 +10102,9 @@ namespace ranges { if constexpr (forward_range<_Rng> && _Prefer_iterator_copies>) { const auto _Found = _RANGES _Minmax_element_unchecked( _STD move(_UFirst), _STD move(_ULast), _Pass_fn(_Pred), _Pass_fn(_Proj)); + // We can dereference normal pointers many times but some iterators could move their value after + // dereferencing so we should dereference them only once. The if constexpr is a pure optimization, we + // don't want an additional branch for normal pointers. if constexpr (!is_pointer_v>) { if (_Found.min == _Found.max) { auto _Temp = static_cast<_Vty>(*_Found.min); From c0abfa39bf82ebf7fc1aa00015b2c3700ddbc58d Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 30 Jan 2023 14:03:01 -0800 Subject: [PATCH 06/16] Code review feedback. --- stl/inc/algorithm | 4 ++-- tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index ccec718c41..e62fa6226d 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -10103,8 +10103,8 @@ namespace ranges { const auto _Found = _RANGES _Minmax_element_unchecked( _STD move(_UFirst), _STD move(_ULast), _Pass_fn(_Pred), _Pass_fn(_Proj)); // We can dereference normal pointers many times but some iterators could move their value after - // dereferencing so we should dereference them only once. The if constexpr is a pure optimization, we - // don't want an additional branch for normal pointers. + // dereferencing so we should dereference them only once. The `if constexpr` is a pure optimization; + // we don't want an additional branch for normal pointers. if constexpr (!is_pointer_v>) { if (_Found.min == _Found.max) { auto _Temp = static_cast<_Vty>(*_Found.min); diff --git a/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp b/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp index b8215bacf3..f5e110eca0 100644 --- a/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include @@ -364,10 +365,11 @@ void test_gh_1893() { void test_gh_2900() { // GH-2900: : ranges::minmax initializes minmax_result with the moved value { - vector v{"1"}; - auto [min, max] = ranges::minmax(ranges::subrange{make_move_iterator(v.begin()), make_move_iterator(v.end())}); - assert(min == "1"); - assert(max == "1"); + const string str{"this long string will be dynamically allocated"}; + vector v{str}; + auto result = ranges::minmax(ranges::subrange{move_iterator{v.begin()}, move_iterator{v.end()}}); + assert(result.min == str); + assert(result.max == str); } } From a06141ab7bb809232d0f467e3b13ac3cd3960341 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Wed, 1 Feb 2023 07:56:51 +0700 Subject: [PATCH 07/16] add a test coverage for input iterators --- .../tests/P0896R4_ranges_alg_minmax/test.cpp | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp b/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp index f5e110eca0..c84ce6110b 100644 --- a/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -362,15 +363,66 @@ void test_gh_1893() { ASSERT(val == "meow"); } +class input_move_iterator { +public: + using iterator_category = input_iterator_tag; + using iterator_concept = input_iterator_tag; + using difference_type = ptrdiff_t; + using value_type = shared_ptr; + using pointer = shared_ptr*; + using reference = shared_ptr&&; + + input_move_iterator() = default; + input_move_iterator(shared_ptr* ptr) : m_ptr(ptr) {} + + reference operator*() const { + return ranges::iter_move(m_ptr); + } + pointer operator->() { + return m_ptr; + } + + input_move_iterator& operator++() { + m_ptr++; + return *this; + } + input_move_iterator operator++(int) { + input_move_iterator tmp = *this; + ++(*this); + return tmp; + } + + friend bool operator==(const input_move_iterator& a, const input_move_iterator& b) { + return a.m_ptr == b.m_ptr; + }; + friend bool operator!=(const input_move_iterator& a, const input_move_iterator& b) { + return a.m_ptr != b.m_ptr; + }; + +private: + shared_ptr* m_ptr; +}; + void test_gh_2900() { // GH-2900: : ranges::minmax initializes minmax_result with the moved value { + // check that the range access iterator isn't moved from multiple times const string str{"this long string will be dynamically allocated"}; vector v{str}; auto result = ranges::minmax(ranges::subrange{move_iterator{v.begin()}, move_iterator{v.end()}}); assert(result.min == str); assert(result.max == str); } + { + // check that the input iterator isn't moved from multiple times + shared_ptr a[] = {make_shared(42)}; + auto range = ranges::subrange(input_move_iterator(a), input_move_iterator(a + 1)); + auto result = ranges::minmax(range); + assert(a[0] == nullptr); + assert(result.min != nullptr); + assert(result.max == result.min); + assert(*result.max == 42); + } } int main() { From 2356ebf22d26411e1542ce5ea22bc086c98145ad Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 31 Jan 2023 19:16:33 -0800 Subject: [PATCH 08/16] Code review feedback, part 2. --- .../tests/P0896R4_ranges_alg_minmax/test.cpp | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp b/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp index c84ce6110b..157c00e180 100644 --- a/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp @@ -373,51 +373,49 @@ class input_move_iterator { using reference = shared_ptr&&; input_move_iterator() = default; - input_move_iterator(shared_ptr* ptr) : m_ptr(ptr) {} + explicit input_move_iterator(shared_ptr* ptr) : m_ptr(ptr) {} reference operator*() const { return ranges::iter_move(m_ptr); } - pointer operator->() { + pointer operator->() const { return m_ptr; } input_move_iterator& operator++() { - m_ptr++; + ++m_ptr; return *this; } input_move_iterator operator++(int) { input_move_iterator tmp = *this; - ++(*this); + ++*this; return tmp; } friend bool operator==(const input_move_iterator& a, const input_move_iterator& b) { return a.m_ptr == b.m_ptr; - }; - friend bool operator!=(const input_move_iterator& a, const input_move_iterator& b) { - return a.m_ptr != b.m_ptr; - }; + } private: - shared_ptr* m_ptr; + shared_ptr* m_ptr{nullptr}; }; void test_gh_2900() { // GH-2900: : ranges::minmax initializes minmax_result with the moved value { - // check that the range access iterator isn't moved from multiple times + // check that the random access iterator isn't moved from multiple times const string str{"this long string will be dynamically allocated"}; vector v{str}; - auto result = ranges::minmax(ranges::subrange{move_iterator{v.begin()}, move_iterator{v.end()}}); + ranges::subrange rng{move_iterator{v.begin()}, move_iterator{v.end()}}; + auto result = ranges::minmax(rng); assert(result.min == str); assert(result.max == str); } { // check that the input iterator isn't moved from multiple times shared_ptr a[] = {make_shared(42)}; - auto range = ranges::subrange(input_move_iterator(a), input_move_iterator(a + 1)); - auto result = ranges::minmax(range); + ranges::subrange rng{input_move_iterator{a}, input_move_iterator{a + 1}}; + auto result = ranges::minmax(rng); assert(a[0] == nullptr); assert(result.min != nullptr); assert(result.max == result.min); From 15e4a13371b70c241a4a0e85a0ba50a871e156f7 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Wed, 1 Feb 2023 11:39:52 +0700 Subject: [PATCH 09/16] unneeded moves --- stl/inc/algorithm | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index e62fa6226d..0970649778 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -10107,14 +10107,13 @@ namespace ranges { // we don't want an additional branch for normal pointers. if constexpr (!is_pointer_v>) { if (_Found.min == _Found.max) { - auto _Temp = static_cast<_Vty>(*_Found.min); - return {_Temp, _STD move(_Temp)}; + minmax_result<_Vty> _Result = {static_cast<_Vty>(*_Found.min), _Result.min}; + return _Result; } } return {static_cast<_Vty>(*_Found.min), static_cast<_Vty>(*_Found.max)}; } else { - auto _Temp = static_cast<_Vty>(*_UFirst); - minmax_result<_Vty> _Found = {_Temp, _STD move(_Temp)}; + minmax_result<_Vty> _Found = {static_cast<_Vty>(*_UFirst), _Found.min}; if (_UFirst == _ULast) { return _Found; } From 4886833b8c53b3e4e35c95e310764fbf8d2e26ab Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Thu, 2 Feb 2023 06:10:08 +0700 Subject: [PATCH 10/16] change `if constexpr` condition Co-authored-by: Nicole Mazzuca --- stl/inc/algorithm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 0970649778..b2899b2369 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -10102,10 +10102,10 @@ namespace ranges { if constexpr (forward_range<_Rng> && _Prefer_iterator_copies>) { const auto _Found = _RANGES _Minmax_element_unchecked( _STD move(_UFirst), _STD move(_ULast), _Pass_fn(_Pred), _Pass_fn(_Proj)); - // We can dereference normal pointers many times but some iterators could move their value after - // dereferencing so we should dereference them only once. The `if constexpr` is a pure optimization; - // we don't want an additional branch for normal pointers. - if constexpr (!is_pointer_v>) { + // Some iterators could move their value after dereferencing so we should dereference them only once. + // The `if constexpr` is a pure optimization; + // we don't want an additional branch if iterators dereference return lvalue reference. + if constexpr (!is_lvalue_reference_v>) { if (_Found.min == _Found.max) { minmax_result<_Vty> _Result = {static_cast<_Vty>(*_Found.min), _Result.min}; return _Result; From 2aff9683be5c7ad7c18d89642e96dd80ba7e47e6 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Thu, 2 Feb 2023 06:23:59 +0700 Subject: [PATCH 11/16] range_reference_t --- stl/inc/algorithm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index b2899b2369..12577cf50d 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -10105,7 +10105,7 @@ namespace ranges { // Some iterators could move their value after dereferencing so we should dereference them only once. // The `if constexpr` is a pure optimization; // we don't want an additional branch if iterators dereference return lvalue reference. - if constexpr (!is_lvalue_reference_v>) { + if constexpr (!is_lvalue_reference_v>) { if (_Found.min == _Found.max) { minmax_result<_Vty> _Result = {static_cast<_Vty>(*_Found.min), _Result.min}; return _Result; From b71f40263e6dd7e2e8c2c7c6bb8b49a50fd33264 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 1 Feb 2023 15:30:15 -0800 Subject: [PATCH 12/16] Clarify comment. --- stl/inc/algorithm | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 12577cf50d..a4ee1443ad 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -10102,9 +10102,8 @@ namespace ranges { if constexpr (forward_range<_Rng> && _Prefer_iterator_copies>) { const auto _Found = _RANGES _Minmax_element_unchecked( _STD move(_UFirst), _STD move(_ULast), _Pass_fn(_Pred), _Pass_fn(_Proj)); - // Some iterators could move their value after dereferencing so we should dereference them only once. - // The `if constexpr` is a pure optimization; - // we don't want an additional branch if iterators dereference return lvalue reference. + // We need to detect move_iterators (and similar types) to avoid dereferencing them more than once. + // The `if constexpr` is a pure optimization; we don't want an additional branch for normal iterators. if constexpr (!is_lvalue_reference_v>) { if (_Found.min == _Found.max) { minmax_result<_Vty> _Result = {static_cast<_Vty>(*_Found.min), _Result.min}; From bc3262d87e5554576e2d3fbac7ed2512841dd9a0 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Thu, 2 Feb 2023 21:51:20 +0700 Subject: [PATCH 13/16] code review suggestions Co-authored-by: Casey Carter --- stl/inc/algorithm | 9 ++++++--- tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp | 4 +--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index a4ee1443ad..db3f2d1b02 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -10102,16 +10102,19 @@ namespace ranges { if constexpr (forward_range<_Rng> && _Prefer_iterator_copies>) { const auto _Found = _RANGES _Minmax_element_unchecked( _STD move(_UFirst), _STD move(_ULast), _Pass_fn(_Pred), _Pass_fn(_Proj)); - // We need to detect move_iterators (and similar types) to avoid dereferencing them more than once. - // The `if constexpr` is a pure optimization; we don't want an additional branch for normal iterators. - if constexpr (!is_lvalue_reference_v>) { + // Avoid repeatedly initializing objects from the result of an iterator dereference when doing so might + // not be idempotent. The `if constexpr` to avoid the extra branch in cases where it's not needed + if constexpr (same_as>, _Vty> + && !is_lvalue_reference_v>) { if (_Found.min == _Found.max) { + // This initialization is correct, similar to WG21-N4928 [dcl.init.aggr]/6 example minmax_result<_Vty> _Result = {static_cast<_Vty>(*_Found.min), _Result.min}; return _Result; } } return {static_cast<_Vty>(*_Found.min), static_cast<_Vty>(*_Found.max)}; } else { + // This initialization is correct, similar to WG21-N4928 [dcl.init.aggr]/6 example minmax_result<_Vty> _Found = {static_cast<_Vty>(*_UFirst), _Found.min}; if (_UFirst == _ULast) { return _Found; diff --git a/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp b/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp index 157c00e180..5297dbff13 100644 --- a/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp @@ -392,9 +392,7 @@ class input_move_iterator { return tmp; } - friend bool operator==(const input_move_iterator& a, const input_move_iterator& b) { - return a.m_ptr == b.m_ptr; - } + friend bool operator==(const input_move_iterator& a, const input_move_iterator& b) = default; private: shared_ptr* m_ptr{nullptr}; From fa6d225b7a51468eacbda9bb9a0afc02c920fd4a Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Thu, 2 Feb 2023 22:06:52 +0700 Subject: [PATCH 14/16] wrong condition --- stl/inc/algorithm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index db3f2d1b02..b77d231826 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -10104,8 +10104,8 @@ namespace ranges { _STD move(_UFirst), _STD move(_ULast), _Pass_fn(_Pred), _Pass_fn(_Proj)); // Avoid repeatedly initializing objects from the result of an iterator dereference when doing so might // not be idempotent. The `if constexpr` to avoid the extra branch in cases where it's not needed - if constexpr (same_as>, _Vty> - && !is_lvalue_reference_v>) { + if constexpr (!same_as>, _Vty> + || !is_lvalue_reference_v>) { if (_Found.min == _Found.max) { // This initialization is correct, similar to WG21-N4928 [dcl.init.aggr]/6 example minmax_result<_Vty> _Result = {static_cast<_Vty>(*_Found.min), _Result.min}; From 8b96897296176da4881e3b4b21637edd2df443aa Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Thu, 2 Feb 2023 09:31:37 -0800 Subject: [PATCH 15/16] iterators that yield elements by value are fine --- stl/inc/algorithm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index b77d231826..081ae38946 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -10105,7 +10105,7 @@ namespace ranges { // Avoid repeatedly initializing objects from the result of an iterator dereference when doing so might // not be idempotent. The `if constexpr` to avoid the extra branch in cases where it's not needed if constexpr (!same_as>, _Vty> - || !is_lvalue_reference_v>) { + || is_rvalue_reference_v>) { if (_Found.min == _Found.max) { // This initialization is correct, similar to WG21-N4928 [dcl.init.aggr]/6 example minmax_result<_Vty> _Result = {static_cast<_Vty>(*_Found.min), _Result.min}; From efbb4d49bde7dca5d9646e524c8d24b870454604 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 2 Feb 2023 13:23:24 -0800 Subject: [PATCH 16/16] Nitpicks! --- stl/inc/algorithm | 6 +++--- tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 081ae38946..a07ba83041 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -10103,18 +10103,18 @@ namespace ranges { const auto _Found = _RANGES _Minmax_element_unchecked( _STD move(_UFirst), _STD move(_ULast), _Pass_fn(_Pred), _Pass_fn(_Proj)); // Avoid repeatedly initializing objects from the result of an iterator dereference when doing so might - // not be idempotent. The `if constexpr` to avoid the extra branch in cases where it's not needed + // not be idempotent. The `if constexpr` avoids the extra branch in cases where it's not needed. if constexpr (!same_as>, _Vty> || is_rvalue_reference_v>) { if (_Found.min == _Found.max) { - // This initialization is correct, similar to WG21-N4928 [dcl.init.aggr]/6 example + // This initialization is correct, similar to the N4928 [dcl.init.aggr]/6 example minmax_result<_Vty> _Result = {static_cast<_Vty>(*_Found.min), _Result.min}; return _Result; } } return {static_cast<_Vty>(*_Found.min), static_cast<_Vty>(*_Found.max)}; } else { - // This initialization is correct, similar to WG21-N4928 [dcl.init.aggr]/6 example + // This initialization is correct, similar to the N4928 [dcl.init.aggr]/6 example minmax_result<_Vty> _Found = {static_cast<_Vty>(*_UFirst), _Found.min}; if (_UFirst == _ULast) { return _Found; diff --git a/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp b/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp index 5297dbff13..0778cea50d 100644 --- a/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp @@ -392,7 +392,7 @@ class input_move_iterator { return tmp; } - friend bool operator==(const input_move_iterator& a, const input_move_iterator& b) = default; + friend bool operator==(const input_move_iterator&, const input_move_iterator&) = default; private: shared_ptr* m_ptr{nullptr};