On 07/12/2023 15:04, Jonathan Wakely wrote:
> On Thu, 7 Dec 2023 at 13:41, Jonathan Wakely <jwakely@redhat.com> wrote:
>> On Wed, 6 Dec 2023 at 20:55, François Dumont <frs.dumont@gmail.com> wrote:
>>> I think I still got no feedback about this cleanup proposal.
>> Can you remind me why we have all those different functions in
>> predefined_ops.h in the first place? I think it was to avoid having
>> two versions of every algorithm, one that does *l < *r and one that
>> does pred(*l, *r), right?

Right !

>>
>> One property of the current code is that _Iter_less_iter will compare
>> exactly *lhs < *rhs and so works even with this type, where its
>> operator< only accepts non-const arguments:
>>
>> struct X { bool operator<(X&); };
>>
>> Doesn't your simplification break that, because the _Less function
>> only accepts const references now?

Yes, I thought it was ok to force users to do the right thing, make 
operators const-qualified and use mutable when they want to do fancy things.

But you think otherwise so here is another version more user-friendly.

In addition to simplifying, I think, predefined-ops it also implements 
perfect-forwarding of user functors. Some algos in stl_heap.h were 
already taking functor by ref.

I wanted to show it to you now because thanks to this patch I have an 
interesting failure that I cannot really explain. I just guess that the 
new version allow more inline-ment and triggers this:

In member function 'void std::__new_allocator<_Tp>::deallocate(_Tp*, 
size_type) [with _Tp = int]',
     inlined from 'constexpr void std::allocator< 
<template-parameter-1-1> >::deallocate(_Tp*, std::size_t) [with _Tp = 
int]' at 
/home/fdumont/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/allocator
.h:208,
     inlined from 'static constexpr void 
std::allocator_traits<std::allocator<_CharT> 
 >::deallocate(allocator_type&, pointer, size_type) [with _Tp = int]' at 
/home/fdumont/dev/gcc/build/x86_64-pc-linux-gnu/libstd
c++-v3/include/bits/alloc_traits.h:513,
     inlined from 'constexpr void std::_Vector_base<_Tp, 
_Alloc>::_M_deallocate(pointer, std::size_t) [with _Tp = int; _Alloc = 
std::allocator<int>]' at 
/home/fdumont/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v
3/include/bits/stl_vector.h:389,
     inlined from 'constexpr void std::_Vector_base<_Tp, 
_Alloc>::_M_deallocate(pointer, std::size_t) [with _Tp = int; _Alloc = 
std::allocator<int>]' at 
/home/fdumont/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v
3/include/bits/stl_vector.h:385,
     inlined from 'constexpr std::_Vector_base<_Tp, 
_Alloc>::~_Vector_base() [with _Tp = int; _Alloc = std::allocator<int>]' 
at 
/home/fdumont/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_vector
.h:368,
     inlined from 'constexpr std::vector<_Tp, _Alloc>::~vector() [with 
_Tp = int; _Alloc = std::allocator<int>]' at 
/home/fdumont/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_vector.h:738,
     inlined from 'void test01()' at 
/home/fdumont/dev/gcc/git/libstdc++-v3/testsuite/25_algorithms/merge/constrained.cc:52:
/home/fdumont/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/new_allocator.h:172: 
warning: 'void operator delete(void*, std::size_t)' called on pointer 
'<unknown>' with nonzero offset [4, 20] [-Wfre
e-nonheap-object]
In member function '_Tp* std::__new_allocator<_Tp>::allocate(size_type, 
const void*) [with _Tp = int]',
     inlined from 'constexpr _Tp* std::allocator< 
<template-parameter-1-1> >::allocate(std::size_t) [with _Tp = int]' at 
/home/fdumont/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/allocator.h:196,
     inlined from 'static constexpr _Tp* 
std::allocator_traits<std::allocator<_CharT> 
 >::allocate(allocator_type&, size_type) [with _Tp = int]' at 
/home/fdumont/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/incl
ude/bits/alloc_traits.h:478,
     inlined from 'constexpr std::_Vector_base<_Tp, _Alloc>::pointer 
std::_Vector_base<_Tp, _Alloc>::_M_allocate(std::size_t) [with _Tp = 
int; _Alloc = std::allocator<int>]' at /home/fdumont/dev/gcc/build/x86_64-
pc-linux-gnu/libstdc++-v3/include/bits/stl_vector.h:380,
     inlined from 'constexpr void std::vector<_Tp, 
_Alloc>::_M_range_initialize(_ForwardIterator, _ForwardIterator, 
std::forward_iterator_tag) [with _ForwardIterator = int*; _Tp = int; 
_Alloc = std::allocator<int
 >]' at 
/home/fdumont/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_vector.h:1694,
     inlined from 'constexpr std::vector<_Tp, 
_Alloc>::vector(_InputIterator, _InputIterator, const allocator_type&) 
[with _InputIterator = int*; <template-parameter-2-2> = void; _Tp = int; 
_Alloc = std::allocato
r<int>]' at 
/home/fdumont/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_vector.h:711,
     inlined from 'void test01()' at 
/home/fdumont/dev/gcc/git/libstdc++-v3/testsuite/25_algorithms/merge/constrained.cc:47:
/home/fdumont/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/new_allocator.h:151: 
note: returned from 'void* operator new(std::size_t)'
FAIL: 25_algorithms/merge/constrained.cc  -std=gnu++20 (test for excess 
errors)

Just in case there is something to do before gcc 14 release...

François