Re: Add tree heterogeneous lookup in alternative modes

[Jonathan Wakely <jwakely at redhat dot com>]

On 05/06/15 23:00 +0200, François Dumont wrote:
> On 04/06/2015 14:41, Jonathan Wakely wrote:
> > On 03/06/15 21:56 +0200, François Dumont wrote:
> > > Hi
> > >
> > >   Here is a patch to add heterogeneous lookup to alternative 
> > > modes.
> >
> > Excellent.
> >
> > > To do so I had to expose __is_transparent as __has_is_transparent 
> > > to avoid confilct with existing __is_transparent.
> >
> > That's a better name, thanks.
> >
> >
> > > Should I put it in __detail namespace ?
> >
> > Could it just be protected instead of at namespace scope?
>
> No, debug containers are inheriting from normal containers, not from 
> _Rb_tree.

Ah yes, of course.

> > If it can't be protected it's OK here you put it, it doesn't need to
> > be in __detail.
> >
> > But maybe we should rename the _Kt parameter of __hash_is_transparent
> > to _Unused, _Dummy or something like that. It only exists to make the
> > specialization dependent on a template argument of the functions that
> > use it.
>
> I renamed it into _SfinaeType to make the purpose of this type clearer.
>
> > >   * include/bits/stl_tree.h (_Rb_tree<>::__is_transparent<>): Move to
> > >   outer scope and rename in ...
> >
> > "rename to ..."
> not "rename into..." :-)

Nope :-)

> > >   (std::__hash_is_transparent<>): ... this.
> >
> > In <bits/stl_set.h> ...
> >
> > > @@ -429,12 +477,34 @@
> > >      std::pair<const_iterator,const_iterator>
> > >      equal_range(const key_type& __x) const
> > >      {
> > > -    std::pair<_Base_iterator, _Base_iterator> __res =
> > > +    std::pair<_Base_const_iterator, _Base_const_iterator> __res =
> > >    _Base::equal_range(__x);
> > >    return std::make_pair(const_iterator(__res.first, this),
> > >                  const_iterator(__res.second, this));
> > >      }
> >
> > Huh! Did calls to this function even compile before this fix?
>
> I had a closer look and yes, it was compiling because for set iterator 
> and const_iterator are just the same:

Ah, yes, of course (again).

>      // DR 103. set::iterator is required to be modifiable,
>      // but this allows modification of keys.
>      typedef typename _Rep_type::const_iterator iterator;
>      typedef typename _Rep_type::const_iterator const_iterator;
>
> However I slightly changed some tests to make sure the const version 
> of equal_range and some others are used.
>
> If it is still fine I will commit the attached patch tomorrow.

Yes, OK, thanks.