[libcxx-commits] [PATCH] D128864: [libc++] Fix algorithms which use reverse_iterator

[Hui via Phabricator via libcxx-commits]

huixie90 added inline comments.


================
Comment at: libcxx/include/__iterator/reverse_iterator.h:429
+  _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator==(const _AlgRevIter& __lhs, const _AlgRevIter& __rhs) {
+    return __lhs.base() == __rhs.base();
+  }
----------------
var-const wrote:
> philnik wrote:
> > var-const wrote:
> > > philnik wrote:
> > > > var-const wrote:
> > > > > Hmm, so this expression is not ambiguous, but when it's used as a constraint in a `requires` clause, it is considered ambiguous? Am I missing something?
> > > > No, that's exactly the case.
> > > Interesting. Why is that?
> > I don't know exactly why, but I think it is because that would change the overload resolution. With the change in SFINAE contexts you could end up calling different functions, not just allow code that would otherwise be an error.
> It looks like this paper aims to fix the language issue: https://github.com/cplusplus/papers/issues/1127
> Unfortunately, even though the paper was approved this February, it doesn't fix all the potential cases that create ambiguity, so it won't change anything for us even after it's implemented in the compiler (or at least that's how I read it).
First of all, I think the current clang behaviour is really confusing.
See this example https://godbolt.org/z/v767Kdd7K
- If you write `foo1 == foo2`, it actually compiles fine with just a warning
- If you write a concept that checks `foo1 == foo2` is valid expression, it returns `false`
I think clang contradicts itself.


@var-const Thanks for posting this paper P2468R2, I actually think the paper fixes our issue. The example in the paper section 1.2.1
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2468r2.html#open-source-sampling
is exactly what we are facing in robust_against_cpp20_hostile_iterators.compile.pass.cpp
https://github.com/llvm/llvm-project/blob/main/libcxx/test/libcxx/algorithms/robust_against_cpp20_hostile_iterators.compile.pass.cpp#L50

They both have CRTP base with a member `==` that takes a derived class. And the usage of `derived == derived` is ambiguous due to the fact that both sides are equally convertible from derived to base.

The straw poll of this paper will happen next Monday. I am not sure what is the best action for us for llvm-15


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128864/new/

https://reviews.llvm.org/D128864