This is the mail archive of the
mailing list for the libstdc++ project.
Re: Remove algo code duplication
- From: François Dumont <frs dot dumont at gmail dot com>
- To: "libstdc++ at gcc dot gnu dot org" <libstdc++ at gcc dot gnu dot org>
- Cc: Marc Glisse <marc dot glisse at inria dot fr>
- Date: Wed, 02 May 2012 21:34:12 +0200
- Subject: Re: Remove algo code duplication
- References: <4F9BA3CF.email@example.com> <alpine.DEB.firstname.lastname@example.org>
On 04/28/2012 01:28 PM, Marc Glisse wrote:
On Sat, 28 Apr 2012, François Dumont wrote:non-template, really ?
Here is an other attempt to remove duplication of code in
implementation of Standard algos. There are several enhancements on
the last proposal:
Hello, a few comments/questions because I am intested (don't take it
as me trying to review your patch...).
1. No usage of C++11 lambdas anymore, code works fine in C++98 and
code has been really deleted this time. About 700 lines removed in
stl_algo.h for instance.
Ah, yes, to remove the code, the new solution has to work in C++98,
which prevents the simpler solution of having a non-template functor
__less that perfectly forwards anything it is given to operator<.
You mean functor copies, right ? I expect the compiler to optimize them
away even if there are options to forbid him to do so. In fact I also
expect the compiler to optimize away all the functor simple plumbery, if
there is a way to make this assumption safer do not hesitate to tell me.
I wonder if not qualifying the parenthesis operators as const might be a
reason for the compiler to not inline it. If it is so important I will
try to detect if the invoked operator is const qualified to make the
parenthesis operator const also. Maybe I should add a noexcept
2. Compatible with existing code, code submitted by Christopher last
time will still compile. To do so I used
std::iterator_traits<>::reference type so that if the iterator is not
const then the operator do not have to take const reference or to be
const qualified neither. For the same reason I have also avoided some
const qualifiers on the introduced functors.
It probably doesn't matter, but the difference with the current code
can be seen. With -fno-elide-constructors you perform copies,
operator< is always given an lvalue, etc.
In addition to the reason below I also prefer to see concept checks
performed as near as possible in the call-stack to the Standard algo
called by user code to guaranty the cleanest diagnostic as possible even
at code duplication cost in this case.
In libstdcxx_so_7 branch the algo version using operators was calling
the version taking a functor. The result was that concept checks on
the operator version was performed twice. It is not a big concern
when it is really pure concept checks but in trunk there is also the
debug checks that can be quite expensive, checking that a range is
sorted for instance. So I prefer to introduce an internal
implementation that contains only the algo implementation without any
concept checks and have it call from the Standard algos.
Can't you just remove the checks from the operator version, in that
case? It also avoids code duplication.
Those pure implementations are also the one used for internal purpose.
Ah, makes sense then to have an unchecked version.
* include/bits/predefined_ops.h: New.
A name like __gnu_cxx::__ops::less<Iter> that designates a comparison
on dereferenced iterators is quite confusing as it differs from
std::less. Maybe with "iterator" (or just "iter") in the name of the
namespace (or the functor) it would help?