[libcxx-commits] [PATCH] D110515: [libc++] [compare] Named comparison functions, is_eq etc.

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__compare/is_eq.h:1
+//===----------------------------------------------------------------------===//
+//
----------------
rarutyun wrote:
> I would prefer to change this file to `named_comparison_functions.h` with the corresponding changes in guards and module.modulemap. From my perspective it gives better match to what section of the standard this private module implements.
I'm not //strongly// opposed to that idea. In fact I certainly would have done that, if these functions had actually been described in a named section like [named.comparison.functions]. Unfortunately, they're submarined into [compare.syn] with no English description anywhere in the standard (and `compare_syn.h` felt like the wrong answer).
Therefore, the recent-historical default behavior of "name the header after the entity it declares" came into play. I don't think anyone will be confused by how `is_eq.h` defines //both// `is_eq` and `is_gt`. And vice versa, if I'm looking for the header that defines `is_gt`, I'll sooner look in `is_*.h` than in `named_*.h`.


================
Comment at: libcxx/test/libcxx/diagnostics/detail.headers/compare/is_eq.module.verify.cpp:16
+// expected-error@*:* {{use of private header from outside its module: '__compare/is_eq.h'}}
+#include <__compare/is_eq.h>
----------------
@mordante wrote:
> Would it be possible to handle the comment in test/std/ranges/range.adaptors/range.transform/iterator/compare.pass.cpp:39 in this commit/in a separate commit?

In a separate PR, yes. That was actually how I stumbled onto this. It turns out that that test can't be merely uncommented; it needs some actual "design decision" stuff, so I want to do it in a separate PR. (Namely, `ContiguousView` has iterator type `int*`, so its iterators //are// three-way-comparable; which makes `ThreeWayComparableView` basically redundant. Whereas `contiguous_iterator<int*>` right now is //not// three-way-comparable, but I don't know that we should hard-code too many tests that rely on that being forever true. Anyway, separate PR, etc. :))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110515