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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik 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;
     };
----------------
Quuxplusone wrote:
> 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).
I thought about a name myself but couldn't find a good one. `__class` seemed to be the obvious choice, but that sounds more like `is_class_v` and not `is_class_v || is_union_v`. On the other hand, if it says `__class_or_enum` I would think of `is_class_v || is_enum_v` and not `is_class_v || is_enum_v || is_union_v`. Maybe something like `__class_type` to make clear that it's not just `is_class`? IDK, maybe we should discuss this on discord?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116239