On Mon, 28 Apr 2025 at 14:59, Patrick Palka <ppa...@redhat.com> wrote:
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk/15?
>
> -- >8 --
>
> These std::erase_if overloads were wrongly implemented as hidden
> friends, visible only via ADL, so erase_if(x) would work but not
> std::erase_if(x).
Would it be easier to just add a namespace-scope declaration of the
friend, so they're not hidden? That would avoid needing both a private
member *and* a friend.
>
> PR libstdc++/119427
>
> libstdc++-v3/ChangeLog:
>
> * include/std/flat_map (_Flat_map_impl::erase_if): Replace
> this hidden friend with ...
> (_Flat_map_impl::_M_erase_if): ... this member function.
> (flat_map): Export _Flat_map_impl::_M_erase_if.
> (flat_multimap): Likewise.
> * include/std/flat_set (_Flat_set_impl::erase_if): Replace
> with ...
> (_Flat_set_impl::_M_erase_if): ... this member function.
> (flat_set): Export _Flat_set_impl::_M_erase_if.
> (flat_multiset): Likewise.
> * testsuite/23_containers/flat_map/1.c (test07): New test.
> * testsuite/23_containers/flat_multimap/1.cc (test07): New test.
> * testsuite/23_containers/flat_multiset/1.cc (test09): New test.
> * testsuite/23_containers/flat_set/1.cc (test09): New test.
> ---
> libstdc++-v3/include/std/flat_map | 28 +++++++++++++++----
> libstdc++-v3/include/std/flat_set | 28 +++++++++++++++----
> .../testsuite/23_containers/flat_map/1.cc | 11 ++++++++
> .../23_containers/flat_multimap/1.cc | 11 ++++++++
> .../23_containers/flat_multiset/1.cc | 11 ++++++++
> .../testsuite/23_containers/flat_set/1.cc | 11 ++++++++
> 6 files changed, 89 insertions(+), 11 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/flat_map
> b/libstdc++-v3/include/std/flat_map
> index 405caa8a81bf..6593988d213c 100644
> --- a/libstdc++-v3/include/std/flat_map
> +++ b/libstdc++-v3/include/std/flat_map
> @@ -890,14 +890,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> { return __x.swap(__y); }
>
> template<typename _Predicate>
> - friend size_type
> - erase_if(_Derived& __c, _Predicate __pred)
> + size_type
> + _M_erase_if(_Predicate __pred)
> {
> - auto __guard = __c._M_make_clear_guard();
> - auto __zv = views::zip(__c._M_cont.keys, __c._M_cont.values);
> + auto __guard = _M_make_clear_guard();
> + auto __zv = views::zip(_M_cont.keys, _M_cont.values);
> auto __sr = ranges::remove_if(__zv, __pred);
> auto __erased = __sr.size();
> - __c.erase(__c.end() - __erased, __c.end());
> + erase(end() - __erased, end());
> __guard._M_disable();
> return __erased;
> }
> @@ -1329,6 +1329,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> using _Impl::lower_bound;
> using _Impl::upper_bound;
> using _Impl::equal_range;
> +
> + using _Impl::_M_erase_if;
> };
>
> template<typename _KeyContainer, typename _MappedContainer,
> @@ -1412,6 +1414,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> && uses_allocator_v<_MappedContainer, _Alloc>>
> { };
>
> + template<typename _Key, typename _Tp, typename _Compare,
> + typename _KeyContainer, typename _MappedContainer, typename
> _Predicate>
> + typename flat_map<_Key, _Tp, _Compare, _KeyContainer,
> _MappedContainer>::size_type
> + erase_if(flat_map<_Key, _Tp, _Compare, _KeyContainer, _MappedContainer>&
> __c,
> + _Predicate __pred)
> + { return __c._M_erase_if(std::move(__pred)); }
> +
> /* Class template flat_multimap - container adaptor
> *
> * @ingroup
> @@ -1487,6 +1496,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> using _Impl::lower_bound;
> using _Impl::upper_bound;
> using _Impl::equal_range;
> +
> + using _Impl::_M_erase_if;
> };
>
> template<typename _KeyContainer, typename _MappedContainer,
> @@ -1571,6 +1582,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> && uses_allocator_v<_MappedContainer, _Alloc>>
> { };
>
> + template<typename _Key, typename _Tp, typename _Compare,
> + typename _KeyContainer, typename _MappedContainer, typename
> _Predicate>
> + typename flat_multimap<_Key, _Tp, _Compare, _KeyContainer,
> _MappedContainer>::size_type
> + erase_if(flat_multimap<_Key, _Tp, _Compare, _KeyContainer,
> _MappedContainer>& __c,
> + _Predicate __pred)
> + { return __c._M_erase_if(std::move(__pred)); }
> +
> _GLIBCXX_END_NAMESPACE_VERSION
> } // namespace std
> #endif // __cpp_lib_flat_map
> diff --git a/libstdc++-v3/include/std/flat_set
> b/libstdc++-v3/include/std/flat_set
> index 3e15d1af4162..c48340d79809 100644
> --- a/libstdc++-v3/include/std/flat_set
> +++ b/libstdc++-v3/include/std/flat_set
> @@ -745,15 +745,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> { return __x.swap(__y); }
>
> template<typename _Predicate>
> - friend size_type
> - erase_if(_Derived& __c, _Predicate __pred)
> + size_type
> + _M_erase_if(_Predicate __pred)
> {
> - auto __guard = __c._M_make_clear_guard();
> - auto __first = __c._M_cont.begin();
> - auto __last = __c._M_cont.end();
> + auto __guard = _M_make_clear_guard();
> + auto __first = _M_cont.begin();
> + auto __last = _M_cont.end();
> __first = std::remove_if(__first, __last, __pred);
> auto __n = __last - __first;
> - __c.erase(__first, __last);
> + erase(__first, __last);
> __guard._M_disable();
> return __n;
> }
> @@ -860,6 +860,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> using _Impl::lower_bound;
> using _Impl::upper_bound;
> using _Impl::equal_range;
> +
> + using _Impl::_M_erase_if;
> };
>
> template<typename _KeyContainer,
> @@ -930,6 +932,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> : bool_constant<uses_allocator_v<_KeyContainer, _Alloc>>
> { };
>
> + template<typename _Key, typename _Compare, typename _KeyContainer,
> + typename _Predicate>
> + typename flat_set<_Key, _Compare, _KeyContainer>::size_type
> + erase_if(flat_set<_Key, _Compare, _KeyContainer>& __c, _Predicate __pred)
> + { return __c._M_erase_if(std::move(__pred)); }
> +
> /* Class template flat_multiset - container adaptor
> *
> * @ingroup
> @@ -999,6 +1007,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> using _Impl::lower_bound;
> using _Impl::upper_bound;
> using _Impl::equal_range;
> +
> + using _Impl::_M_erase_if;
> };
>
> template<typename _KeyContainer,
> @@ -1069,6 +1079,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> : bool_constant<uses_allocator_v<_KeyContainer, _Alloc>>
> { };
>
> + template<typename _Key, typename _Compare, typename _KeyContainer,
> + typename _Predicate>
> + typename flat_multiset<_Key, _Compare, _KeyContainer>::size_type
> + erase_if(flat_multiset<_Key, _Compare, _KeyContainer>& __c, _Predicate
> __pred)
> + { return __c._M_erase_if(std::move(__pred)); }
> +
> _GLIBCXX_END_NAMESPACE_VERSION
> } // namespace std
> #endif // __cpp_lib_flat_set
> diff --git a/libstdc++-v3/testsuite/23_containers/flat_map/1.cc
> b/libstdc++-v3/testsuite/23_containers/flat_map/1.cc
> index 4fd33f616f78..2e96dc0c8ea4 100644
> --- a/libstdc++-v3/testsuite/23_containers/flat_map/1.cc
> +++ b/libstdc++-v3/testsuite/23_containers/flat_map/1.cc
> @@ -242,6 +242,16 @@ test06()
> VERIFY( std::ranges::equal(m | std::views::values, (int[]){2, 3, 4, 5, 6})
> );
> }
>
> +void
> +test07()
> +{
> + // PR libstdc++/119427 - std::erase_if(std::flat_foo) does not work
> + std::flat_map<int, int> m = {std::pair{1, 2}, {3, 4}, {5, 6}};
> + auto n = std::erase_if(m, [](auto x) { auto [k,v] = x; return k == 1 || v
> == 6; });
> + VERIFY( n == 2 );
> + VERIFY( std::ranges::equal(m, (std::pair<int,int>[]){{3,4}}) );
> +}
> +
> int
> main()
> {
> @@ -254,4 +264,5 @@ main()
> test04();
> test05();
> test06();
> + test07();
> }
> diff --git a/libstdc++-v3/testsuite/23_containers/flat_multimap/1.cc
> b/libstdc++-v3/testsuite/23_containers/flat_multimap/1.cc
> index ea0d4b41e9fb..13919b44b6e4 100644
> --- a/libstdc++-v3/testsuite/23_containers/flat_multimap/1.cc
> +++ b/libstdc++-v3/testsuite/23_containers/flat_multimap/1.cc
> @@ -220,6 +220,16 @@ test06()
> VERIFY( std::ranges::equal(m | std::views::values, (int[]){2, 3, 4, 5, 6})
> );
> }
>
> +void
> +test07()
> +{
> + // PR libstdc++/119427 - std::erase_if(std::flat_foo) does not work
> + std::flat_multimap<int, int> m = {std::pair{1, 2}, {3, 4}, {3, 3}, {5, 6},
> {6, 6}};
> + auto n = std::erase_if(m, [](auto x) { auto [k,v] = x; return k == 1 || v
> == 6; });
> + VERIFY( n == 3 );
> + VERIFY( std::ranges::equal(m, (std::pair<int,int>[]){{3,4},{3,3}}) );
> +}
> +
> int
> main()
> {
> @@ -232,4 +242,5 @@ main()
> test04();
> test05();
> test06();
> + test07();
> }
> diff --git a/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
> b/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
> index 7d9a33c6b000..63855e070650 100644
> --- a/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
> +++ b/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
> @@ -235,6 +235,16 @@ test08()
> VERIFY( copy_counter == 2 );
> }
>
> +void
> +test09()
> +{
> + // PR libstdc++/119427 - std::erase_if(std::flat_foo) does not work
> + std::flat_multiset<int> s = {1,1,2,2,3,4,5};
> + auto n = std::erase_if(s, [](int x) { return x % 2 != 0; });
> + VERIFY( n == 4 );
> + VERIFY( std::ranges::equal(s, (int[]){2,2,4}) );
> +}
> +
> int
> main()
> {
> @@ -247,4 +257,5 @@ main()
> test06();
> test07();
> test08();
> + test09();
> }
> diff --git a/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
> b/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
> index ed24fab785b5..b1d9002caba3 100644
> --- a/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
> +++ b/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
> @@ -248,6 +248,16 @@ test08()
> VERIFY( copy_counter == 1 );
> }
>
> +void
> +test09()
> +{
> + // PR libstdc++/119427 - std::erase_if(std::flat_foo) does not work
> + std::flat_set<int> s = {1,2,3,4,5};
> + auto n = std::erase_if(s, [](int x) { return x % 2 != 0; });
> + VERIFY( n == 3 );
> + VERIFY( std::ranges::equal(s, (int[]){2,4}) );
> +}
> +
> int
> main()
> {
> @@ -260,4 +270,5 @@ main()
> test06();
> test07();
> test08();
> + test09();
> }
> --
> 2.49.0.459.gf65182a99e
>
