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

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__algorithm/find.h:73
+
+  inline constexpr auto find = __find_fn(__function_like::__tag());
+} // namespace ranges
----------------
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.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105456