[libcxx-commits] [PATCH] D116384: [libc++] Eliminate `__function_like`.

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

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/special_function.compile.pass.cpp:16-19
+// Because this is a variable and not a function, it's guaranteed that ADL won't be used. However,
+// implementations are allowed to use a different mechanism to achieve this effect, so this check is
+// libc++-specific.
+LIBCPP_STATIC_ASSERT(std::is_class_v<decltype(std::ranges::advance)>);
----------------
@ldionne wrote:
> The testing simplifications make sense to me, but removing `__function_like` does mean that we weaken our implementation w.r.t. Hyrum's law. I don't remember coming to a consensus on removing that part (and I would most likely object to that).

(1) It's physically possible to land the libcxx/test/ changes without the libcxx/include/ changes. Shall I do that? (and then leave this PR open for discussion of the libcxx/include/ changes)

(2) Re Hyrum's Law: You have to keep in mind that "there's no such thing as an archetype." C++ has only leaky abstractions. Here we have to choose, for each niebloid in the library, should it be copyable or non-copyable? final or non-final? default-initializable or non-default-initializable? We cannot avoid choosing one or the other. Whichever one we choose, the user //might// (accidentally or on purpose) end up relying on it. They'll have some template that does one thing if the type is copyable and a different thing if it's not, and then boom, technically we can't change that without an ABI break.
The paper standard saves us here by making most of those problematic things IFNDR, so we actually //can// change this stuff without worrying about the breakage to users. Therefore I think our goal should be simplicity and predictability: we should do the simplest thing that works, because complexity without reason is bad.

FWIW, today, GCC accepts and MSVC rejects https://godbolt.org/z/hhae7zEf9
```
    std::ranges::transform(v, w.begin(), std::ranges::next);
```
Everyone (including libc++) accepts things like
```
    (void) std::identity()(std::ranges::next);
    std::optional<decltype(std::ranges::next)> o;
    std::function<int*(int*)> f = std::ref(std::ranges::next);
```
These are all IFNDR as far as I'm aware: the user is supposed to know that `std::ranges::next` (despite being a niebloid) isn't //supposed// to be used in these ways. This is basically analogous to the stuff that appears in SD-8: "sure you can try doing this, but the library vendor doesn't have to coddle you and you're on your own if it breaks later."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116384