[libc++] Refactor __less

[philnik]

Summary

This simplifies the usage of __less by making the class not depend on the types compared, but instead the operator(). We can't remove the template completely because we explicitly instantiate std::__sort with __less<T>.

[EricWF]

This cleanup seems incomplete. Why keep __less as a template at all given it's never actually used as one?

Also does the iterator's ::value_type always have to exactly match the type returned from dereferencing it?

libcxx/include/__algorithm/comp.h

37

This doesn't at all seem correct to me.

This takes away an explicit conversion

libcxx/include/__algorithm/includes.h

64

Is this correct? What if the iterator returns a type convertible to value_type?

[EricWF]

libcxx/include/__algorithm/comp.h

37

*The implicit conversion.

[philnik]

In D145285#4176202, @EricWF wrote:

This cleanup seems incomplete. Why keep __less as a template at all given it's never actually used as one?

If you block other reviews on there not being a description, at least read it.

libcxx/include/__algorithm/includes.h

64

AFAICT the standard it either very silent on this, or says one should use less{} as the comparator, which does what __less<>{} does after this change. So this change is either a conforming change, or arguably a bugfix. Although funnily enough std::less{} isn't actually well-formed, and it doesn't specifically use ranges::less{} for the classic algorithms.

[philnik]

Try to fix CI

[ldionne]

libcxx/include/__algorithm/comp.h

32

So there's actually a behavior change in this patch (the removal of implicit conversions noticed by @EricWF). However, I think it is desirable -- right now I don't think we are conforming in C++20. For example, take lower_bound: http://eel.is/c++draft/lower.bound#1. It specifically says Let comp be less{} and proj be identity{} for overloads with no parameters by those names., which implies that we should be using the transparent comparator, but we are not (before this patch).

I think this mandates a change everywhere along with tests. We should also confirm what's the expected behavior in prior Standards and make sure we conform in those as well.

33

Please add a comment explaining why it's empty.