[libcxx-commits] [PATCH] D116239: [libc++] [ranges] ADL-proof the [range.access] CPOs.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__ranges/access.h:55
     __can_borrow<_Tp> &&
     __class_or_enum<remove_cvref_t<_Tp>> &&
     requires(_Tp && __t) {
----------------
Quuxplusone wrote:
> @jloser writes:
> > Regarding the class_or_enum discussion, note `__unqualified_begin` uses `__class_or_enum`, so we should change that (and others, where appropriate)
> 
> For `__unqualified_begin`, `__class_or_enum` is both sufficient //and// necessary: it's looking for `begin(__t)`, which means it //needs// to do ADL even for enums (but, again, not for pointers).
> 
> I still don't understand why the well-formedness check of `t.begin()` triggers instantiation of `Holder<Incomplete>` when `t` is a pointer ( https://godbolt.org/z/Tfb3vTrvf ); I should ask around about that.
@ldionne wrote:
> This looks like a Clang bug to me, is that possible? I don't understand why t.begin() could ever trigger ADL, and hence why this fix is necessary from the library side at all. Instead, I would expect that we fix Clang and then add the tests.

Yes, it's a Clang bug — and I just tracked it down and filed it as https://github.com/llvm/llvm-project/issues/52970 . However, even if we can get Clang14 fixed, we still need this as a workaround since we want libc++14 to work with Clang13.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116239