This is the mail archive of the mailing list for the libstdc++ project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Remove algo code duplication

On 04/28/2012 01:37 PM, Daniel Krügler wrote:
2012/4/28 François Dumont<>:
    Here is an other attempt to remove duplication of code in implementation
of Standard algos. There are several enhancements on the last proposal:
Question: Isn't it necessary to put "less_aux" also into the implementations
namespace (like __less_aux)?

Shouldn't the now simply delegating function templates be declared inline (I do
not know the libstdc++ policy in this regard, so consider this more as
a question).
I prefer to let libstdc++ maintainers answer to those 2 questions.
In one place you are using an explicit cast to bool as in

"__first1 != __last1&& bool(__binary_pred(*__first1, *__first2))"

but not in another place:

"__holeIndex>  __topIndex
	&&  __comp(*(__first + __parent), __value)"

IMO this should be done consistently.

Is similar consistency needed for something like

"if (__first == __middle || __middle == __last)"

as well? (This last example is more like a joke, because there are a big number
of these in the wild. I still ask, because some general guideline seems
In fact I just use the existing code, simply using the functor version of the algos as the algo implementation. If you want to add consistency do not hesitate to propose an other patch. Maybe after mine so that merge is not too complicated :-)

In regard to "name mangling" of __ops components, I don't understand why "__negate" has been captured (there seem to be no need, because std::negate exists, and other names of library components like "equal_to" and "less" are used). I suggest to unmangle __negate or use mangling consistently.
I will have a look to this negate functor.

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
I found a lambda expression used here:

+	std::__is_heap_until(__first,
+			     std::distance(__first, __last),
+			     [](typename _ItTraits::reference __lhs,
+				typename _ItTraits::reference __rhs)
+			     { return __lhs<  __rhs; });

Good catch, indeed I kept one lambda because:
- it is in code that is already C++ 11 specific
- __is_heap_until only need a simple functor with only one kind of < expression, something that a lambda can handle easily

But if you prefer I can use a __ops operator instead.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]