arximboldi / immer

Postmodern immutable and persistent data structures for C++ — value semantics at scale
https://sinusoid.es/immer
Boost Software License 1.0
2.5k stars 183 forks source link

Ambigous Overloaded Operator in C++20 #192

Closed guhwanbae closed 2 years ago

guhwanbae commented 3 years ago

I'm trying to build my project with c++2a version in Clang(clang++-10). But the compiler reports errors which is related to an ambiguous operator.

Example:

#include <algorithm>
#include <iostream>

#include <immer/flex_vector.hpp>

int main() {
  immer::flex_vector<int> v0{1, 2, 3};
  if (auto it = std::find(v0.begin(), v0.end(), 2); it != v0.end()) {
    std::cout << *it << "\n";
  }
  return 0;
}

Compiler Report:

main.cc:8:56: error: use of overloaded operator '!=' is ambiguous (with operand types 'immer::detail::rbts::rrbtree_iterator<int, immer::memory_policy<immer::free_list_heap_policy<immer::cpp_heap, 1024>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>, 5, 6>' and 'immer::flex_vector<int, immer::memory_policy<immer::free_list_heap_policy<immer::cpp_heap, 1024>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>, 5, 6>::iterator' (aka 'rrbtree_iterator<int, immer::memory_policy<immer::free_list_heap_policy<immer::cpp_heap, 1024>, immer::refcount_policy, immer::spinlock_policy, immer::no_transience_policy, false, true>, 5U, 6U>'))
  if (auto it = std::find(v0.begin(), v0.end(), 2); it != v0.end()) {
                                                    ~~ ^  ~~~~~~~~
/usr/local/include/immer/detail/iterator_facade.hpp:124:10: note: candidate function
    bool operator!=(const DerivedT& rhs) const
         ^
/usr/local/include/immer/detail/iterator_facade.hpp:120:10: note: candidate function
    bool operator==(const DerivedT& rhs) const
         ^
/usr/local/include/immer/detail/iterator_facade.hpp:120:10: note: candidate function (with reversed parameter order)

Since C++20, The compiler considers reversed order. So it has two candidates.

// Compiler's candidates
iterator.operaetor==(iterator_facade)
iterator_facade.operator==(iterator)

First candidate is perfectly match in example case, but parameter type is base class. Second candidate has perfect parameter type, but it is calling base method.

I fix this problem by just changing parameter type of the operator==.

template <...>
class iterator_facade {
  // ...
  bool operator==(const iterator_facade& rhs) const {
    return access_t::equal(derived(), rhs.derived());
  }
  // ...
}

But I'm not sure of original intention. Why iterator_facade's comparison operators has DerivedT type parameter, not iterator_facade?

In my opinion, operator==(const iterator_facade&) is enough since C++20. (Or global type operator==(iterator_facade, iterator_facade) Compiler will make operator!= from operator==. Like !(lhs == rhs). And it considers reversed order.

arximboldi commented 3 years ago

Hi!

The DerivedT is required to ensure the right types are used. Otherwise, you could compare iterators that use the iterator_facade of the wrong type (immer::vector with immer::map) creating undefined behavior. Reversing the parameter order is also feasible I think. I will try reproduce this case in a test and fix this.

Thanks a lot for reporting the issue!