5

I'm trying to write an overloaded method that returns non-const result only when both the object on which it is called is non-const and iterator passed in the argument is non-const. (Think of it like the standard methods begin() and begin() const that additionally take an iterator argument.)

I made a version for normal iterators with no problems. However, for some reason, when I'm trying to do the same for reverse iterators, I get a compilation error about an ambiguous function call.

Here's a minimal example:

#include <vector>

class Foo
{
public:
    void bar(std::vector<int>::iterator x) {}
    void bar(std::vector<int>::const_iterator x) const {}

    void baz(std::vector<int>::reverse_iterator x) {}
    void baz(std::vector<int>::const_reverse_iterator x) const {}
};

int main()
{
    std::vector<int> v;
    Foo foo;
    foo.bar(v.cbegin());  // OK
    foo.baz(v.crbegin()); // ambiguous
}

For some reason it compiles if I remove const from the second method baz. It also works in C++20 but currently I cannot use that version.

live demo

How can I make the function baz work in analogous way to the function bar?

Piotr Siupa
  • 3,929
  • 2
  • 29
  • 65
  • fyi: looks like a `std` library implementation issue if you add `-stdlib=libc++` it compiles - https://godbolt.org/z/s4jfzjWfE Both GCC and MSVC also have a problem - live - https://godbolt.org/z/ozdzqY41Y – Richard Critten Nov 26 '22 at 12:17
  • 2
    Note: you can also add `void baz(std::vector::const_reverse_iterator x) {}` to fix this issue... My guess would be that `crbegin()` doesn't return a `const_reverse_iterator` for some reason, but something implicitly convertible to one making the conversions of the `Foo` to const and the conversion of the operand an ambiguity the compiler won't resolve, but I may be wrong here... – fabian Nov 26 '22 at 12:18
  • 3
    It seems the ambiguity comes from [template constexpr reverse_iterator(const reverse_iterator& other)](https://en.cppreference.com/w/cpp/iterator/reverse_iterator/reverse_iterator) constructor and it indeed was addressed in c++20 only: `This overload participates in overload resolution only if U is not the same type as Iter and std::convertible_to is modeled (since C++20)` – dewaffled Nov 26 '22 at 12:44
  • 1
    Related/dupe: [How can I check that assignment of const_reverse_iterator to reverse_iterator is invalid?](https://stackoverflow.com/questions/52279473/how-can-i-check-that-assignment-of-const-reverse-iterator-to-reverse-iterator-is) – Jason Nov 26 '22 at 12:47
  • The quick fix is: `Foo const& cfoo = foo; cfoo.baz(v.crbegin());` – Eljay Nov 26 '22 at 12:49

1 Answers1

7

Oh the joys of overloading resolution rules and SFINAE.

The methods are equivalent to free functions:

void bbaz(Foo&,std::vector<int>::reverse_iterator){}
void bbaz(const Foo&,std::vector<int>::const_reverse_iterator){}

and your usage becomes:

int main()
{
    std::vector<int> v;
    Foo foo;
    bbaz(foo,v.crbegin());
}

The arguments do not exactly match either call:

  • foo is Foo&, not const Foo&
  • v.crbegin() return vector::const_reverse_iterator which is just a different instantiation of the same std::reverse_iterator template as vector::reverse_iterator.
    • reverse_iterator->std::reverse_iterator<vector::iterator>
    • const_reverse_iterator-> std::reverse_iterator<vector::const_iterator>

Cause of ambiguity

Now, the issue is that std::reverse_iterator's ctor is not SFINAE-friendly until C++20:

template< class U >
std::reverse_iterator( const std::reverse_iterator<U>& other );

I.e. there is a viable candidate converting std::reverse_iterator<T> to std::reverse_iterator<U> between any T-U pairs. In this case for T=vector::const_iterator, U=vector::iterator. But of course the template instantiation fails later because it cannot convert const int* to int*.

Since that happens in the template function's body, not the signature, it is too late for SFINAE and overloading considers it a viable candidate function, hence the ambiguity since both calls require one implicit conversion - although only the second one would compile.

This is explained in these answers, making this one essentially a duplicate of that question but it would be IMHO cruel to mark it as such without an explanation which I cannot fit into a comment.

C++20 fixes this omission and SFINAEs that ctor - cppreference:

This overload participates in overload resolution only if U is not the same type as Iter and std::convertible_to<const U&, Iter> is modeled (since C++20)

Solution

As pointed in the comments by @Eljay, forcing const Foo& at the call site is one option, one can use C++17 std::as_const:

#include <utility>
std::as_const(foo).baz(v.crbegin());

Fixing this at definition is more tricky, you could use SFINAE to actually force these overloads but that might be a hassle. @fabian 's solution with adding a third overload without const method qualifier seems easiest to me:

void Foo::baz(std::vector<int>::const_reverse_iterator x) { 
    return std::as_const(*this).baz(x); 
}

It works because now it is a better (exact) match for non-const Foos than the still considered vector::reverse_iterator which would not compile anyway.

Quimby
  • 17,735
  • 4
  • 35
  • 55