Unexpected greedy ops failure

[François Dumont]

On 11/08/21 3:32 pm, Jonathan Wakely wrote:
> On Wed, 11 Aug 2021 at 14:23, François Dumont via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
>> With the patch proposed in:
>> https://gcc.gnu.org/pipermail/libstdc++/2021-August/053009.html to
>> extend some _GLIBCXX_DEBUG checks in _GLIBCXX_ASSERTIONS I have 2 tests
>> in failure:
>>
>> Running target unix/-D_GLIBCXX_DEBUG
>> Running
>> /home/fdt/dev/gcc/git/libstdc++-v3/testsuite/libstdc++-dg/conformance.exp
>> ...
>> FAIL: 23_containers/deque/types/1.cc (test for excess errors)
>> FAIL: 23_containers/vector/types/1.cc (test for excess errors)
>>
>> The error is:
>>
>> /home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/helper_functions.h:96:
>> error: ambiguous overload for 'operator-' in '__rhs - __lhs' (operand
>> types are 'std::move_iterator<greedy_ops::X*>' and
>> 'std::move_iterator<greedy_ops::X*>')
>> In file included from
>> /home/fdt/dev/gcc/git/libstdc++-v3/testsuite/23_containers/vector/types/1.cc:24:
>> /home/fdt/dev/gcc/git/libstdc++-v3/testsuite/util/testsuite_greedy_ops.h:48:
>> note: candidate: 'greedy_ops::X greedy_ops::operator-(T, T) [with T =
>> std::move_iterator<greedy_ops::X*>]'
>> In file included from
>> /home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:67,
>>                    from
>> /home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/vector:60,
>>                    from
>> /home/fdt/dev/gcc/git/libstdc++-v3/testsuite/23_containers/vector/types/1.cc:23:
>> /home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_iterator.h:1692:
>> note: candidate: 'constexpr decltype ((__x.base() - __y.base()))
>> std::operator-(const std::move_iterator<_IteratorL>&, const
>> std::move_iterator<_IteratorR>&) [with _IteratorL = gree
>> dy_ops::X*; _IteratorR = greedy_ops::X*; decltype ((__x.base() -
>> __y.base())) = long int]'
>> compiler exited with status 1
>> FAIL: 23_containers/vector/types/1.cc (test for excess errors)
>>
>> I really don"t understand the link between the failures and this patch.
> The same tests fail with -std=gnu++20 since Jason made a fix to G++. I
> haven't decided yet whether we should just disable those tests for
> C++20, or do something else.

Ok, so I rebuilt the compiler and I do not see this strange side effect 
anymore. My patch on the other thread is fully tested, in _GLIBCXX_DEBUG 
mode too.

>
> Basically, the "greedy" operators on those tests are bad, and nobody
> should write code like that (C++20 concepts make it unnecessary to do
> it anyway).
>
>
>> But at the same time I am surprised that there is no definition for
>> operator-(const move_iterator&, const move_iterator&) in the library
>> (like in attached patch). With this change the failure vanishes.
> See line 1689 in <bits/stl_iterator.h>
Yes, I saw this one but I meant the basic operator- with both operands 
of the same type like the one in the proposed patch.
>
>> Moreover is there a reason for having added several operators as normal
>> std namespace operators rather than inline friend like in my patch ? I
>> know we already talk about it but I don't remember if the reply was that
>> we cannot do it or I simply forgot to propose the patch to do so.
> The standard declares that operator- in namespace std, not as a
> friend, see the synopsis in [iterator.synopsis]. That has observable
> semantic effects, e.g. it's possible to call it with:
>
> struct X {
>    operator std::move_iterator<int*>() const;
> };
> X x;
> auto d = x - x;
>
> If the function is only defined as a friend, then it can only be found
> by ADL, and this example won't compile.
>
> I suppose we could add the hidden friend *and* keep the generic
> overload at namespace scope. That would meet the requirements of the
> standard. I think we would still have the problem with greedy
> operators for comparing related but not identical types, e.g.
> move_iterator<int*> and move_iterator<const int*>.

Maybe we should cause it looks like it boosts a little bit the 
compilation time.

Before I rebuilt the compiler the difference was noticeable from a human 
perspective (but I didn't measure it at that time). Now I measured it 
and with this testsuite_files:

23_containers/deque/types/1.cc
23_containers/vector/types/1.cc

time make check-debug:

real    0m20.406s
user    0m19.522s
sys    0m0.905s

with the additional operator:

real    0m20.391s
user    0m19.689s
sys    0m0.722s

The diff is not huge but with the additional operator the compiler can 
pick up this perfect match without considering all the others, no ?