[libcxx-commits] [PATCH] D105456: [libcxx][algorithms] adds `std::ranges::find`

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added inline comments.


================
Comment at: libcxx/include/__algorithm/find.h:73
+
+  inline constexpr auto find = __find_fn(__function_like::__tag());
+} // namespace ranges
----------------
Quuxplusone wrote:
> cjdb wrote:
> > zoecarver wrote:
> > > Can you add a test for something like this:
> > > ```
> > > namespace std { namespace ranges {
> > > 
> > > struct dummy {
> > >   friend void find() { }
> > > };
> > > 
> > > }}
> > > ```
> > > Or at least fix the problem that it will uncover. 
> > Unlike with CPOs, `ranges` algorithms don't have a problem here, because `find(std::ranges::dummy())` is [[ https://eel.is/c++draft/algorithms.requirements#2 | expressly forbidden ]].
> Well, it's not that `find(std::ranges::dummy())` is //forbidden//; it's just that such a call expression is guaranteed not to find `std::ranges::find` (because `std::ranges::find` is not a function*).  (*—Okay, in standardese it //is// a function, but it's a novel kind of function that in all respects behaves exactly like a niebloid variable; for example, it follows the lookup rules for variables, not the rules for functions.)
> 
> //However//, @cjdb, I believe that @zoecarver's main point here is that you forgot the extra `inline namespace std::ranges::__cpo` that needs to go around every niebloid variable. Without that namespace, you get ill-formedness if anyone (any libc++ maintainer) later tries to add a function by that same name into `namespace std::ranges`. See https://reviews.llvm.org/D107098#inline-1019805 for previous discussion of the `inline namespace __cpo` pattern and rationale for using it consistently rather than trying to guess which names we need it on this year. (And notice that once we ship //without// `__cpo`, it's technically an ABI break to go adding it. So that's why it's useful to get right on the first go-round.)
There appears to be conflation of the terms 'niebloid' and 'customisation point object', which might explain the confusion here. A 'niebloid' is any function that's in namespace `std::ranges`. Neibloids cannot be found by ADL, and when found by unqualified lookup, inhibit ADL. That we choose to implement them as function objects is a design decision I'm not entirely happy with.

This means that enclosing `ranges::find` in an inline namespace is incorrect in the niebloid case, because it gives way for a `ranges::find` function to be found by ADL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105456