A comment today on D87171 made me look at it, and then I made a bunch of review comments before finally noticing that it was shipped ages ago. 😛 Well, rather than let my comments go to waste, here's the gist of them.
- Simplify the structure of the new tests.
- Test const containers as well as non-const containers, since it's easy to do so.
- Remove redundant enable-iffing of helper structs' member functions. (They're not instantiated unless they're called, and who would call them?)
- Drive-by: _LIBCPP_INLINE_VISIBILITY on some swap functions where I don't see why it was missing in the first place.
I think those enable_ifs are important. Otherwise, if a non-transparent comparator is used with arguments that would trigger a conversion if one of the overloads above were used, it would be possible to observe the fact that a conversion is not being performed. Am I misunderstanding?