Re: Unexpected greedy ops failure

[Jonathan Wakely <jwakely@redhat.com>]

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.

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>


>
> 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*>.