[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:43-46
+    __class_or_enum<remove_cvref_t<_Tp>> &&
     requires(_Tp&& __t) {
       { _LIBCPP_AUTO_CAST(__t.begin()) } -> input_or_output_iterator;
     };
----------------
philnik wrote:
> `enum E; E{}.begin()` will never be a valid statement, right? So `__class_or_enum` should just be `is_class_v<_Tp> || is_union_v<_Tp>` or something similar.
Right, but that would be longer; and breaking it down into two atomic constraints like that //might// lead to worse diagnostics (although I haven't tested that myself). Admittedly our diagnostics here are bad to begin with.
Since this is a pattern we seem to need a lot, I'd be amenable to creating a `concept __class_or_union` right next to `concept __class_or_enum` (in `<__concepts/class_or_enum.h>`). The only problem with that is the awkwardly asymmetric naming: in `__class_or_enum` the term "class" means "class-or-union type" already. Really what we mean (here and everywhere `__class_or_enum` is used) is something like `concept __is_worth_triggering_adl_for`. If it's a simple pointer or function type, we must //not// trigger ADL; but if it's a class or enum type, either we //must// trigger ADL (because the standard says so) or else it's //harmless// to trigger ADL (on an enum type).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116239