Re: [committed] libstdc++: Reduce header dependencies for C++20 std::erase [PR92546]

[Jonathan Wakely <jwakely@redhat.com>]

On Thu, 7 Oct 2021 at 07:52, Jonathan Wakely via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> On Thu, 7 Oct 2021, 07:35 François Dumont, <frs.dumont@gmail.com> wrote:
>
> > On 02/10/21 2:47 pm, Jonathan Wakely wrote:
> >
> >
> >
> > On Sat, 2 Oct 2021, 13:02 François Dumont via Libstdc++, <
> > libstdc++@gcc.gnu.org> wrote:
> >
> >> On 01/10/21 9:38 pm, Jonathan Wakely via Libstdc++ wrote:
> >> > This reduces the preprocessed size of <deque>, <string> and <vector> by
> >> > not including <bits/stl_algo.h> for std::remove and std::remove_if.
> >> >
> >> > Also unwrap iterators using __niter_base, to avoid redundant debug mode
> >> > checks.
> >>
> >> I don't know if you noticed but the __niter_base is a no-op here.
> >>
> >
> > Oh, I didn't notice.
> >
> >
> >> __niter_base unwrap only random access iterators because it is the only
> >> category for which we know that we have been able to confirm validity or
> >> not.
> >>
> >
> > But these are all random access. I must be missing something.
> >
> > It is in a method called '__erases_node_if', I'm not aware of any
> > node-based container providing random access iterators.
> >
>
> Ah, I thought you meant the deque, string and vector part of the patch,
> sorry.
>
> Moreover I just noticed that if it was really doing anything then you would
> > be missing the std::__niter_wrap in the __cont.erase(__iter), it just
> > wouldn't compile.
> >
> >
> >> We would need to introduce another function to do this or specialize in
> >> some way erase_if for debug containers. I'll try to find a solution.
> >>
> >
> > OK, thanks. Maybe we can just leave the checking there. I wanted to avoid
> > the overhead because we know that the iterator range is valid. Any checks
> > done on each increment and equality comparison are wasteful, as they will
> > never fail.
> >
> > Yes, that would be better indeed.
> >
> > But doing it this way you still have the overhead of the _Safe_iterator
> > addition to the list of the safe container iterators, so a mutex
> > lock/unlock.
> >
> > I'll try to find out how to get a normal iterator from a safe container
> > even if in this case we will have to support operations on safe container
> > with normal iterators.
> >
> When we know the type, we could use __cont._GLIBCXX_STD_C::vector<T,
> Alloc>::begin() to access the base version directly. But for the node
> containers we have a generic function where we don't know the exact type.
> Is the _Base typedef accessible? __cont.decltype(__cont)::_Base::begin()
> would work, but it's ugly.

The solution is simple:

    erase_if(set<_Key, _Compare, _Alloc>& __cont, _Predicate __pred)
-    { return __detail::__erase_nodes_if(__cont, __pred); }
+    {
+      _GLIBCXX_STD_C::set<_Key, _Compare, _Alloc>& __c = __cont;
+      return __detail::__erase_nodes_if(__c, __pred);
+    }


i.e. just bind a reference to the non-debug container type. For
non-debug mode, that is a no-op. For debug mode it binds to the base,
and the rest of the function works directly on the base, without safe
iterators.

I'm testing the patch now.