[libcxx-commits] [PATCH] D114912: [libc++] [P1614] Hidden-friend operator<=> for string_view.

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

Quuxplusone marked an inline comment as done.
Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/strings/string.view/string.view.comparison/spaceship.pass.cpp:95-101
+struct TraitsWithCompCat : std::char_traits<char> {
+    using comparison_category = std::partial_ordering;
+};
+
+struct TraitsWithoutCompCat : std::char_traits<char> {
+    void comparison_category();  // does not denote a type
+};
----------------
Quuxplusone wrote:
> philnik wrote:
> > > I recommend not inheriting from standard types in general.
> > 
> > Is there any reason this does not apply here?
> Sheer laziness. ;) See `libcxx/test/support/constexpr_char_traits.h` for the amount of stuff that would have to be repeated here: it's about 100 lines. I'm vocally willing to spend 10 or even 20 lines to avoid inheritance (and on the average iterator it's like 5 lines), but somewhere between 20 and 100 lines I lose some of my enthusiasm. :)  Still, if someone shows how to eliminate this inheritance without adding  200 lines to this test, I'll be happy!
> 
> One idea to which I would be amenable is: Refactor `libcxx/test/support/constexpr_char_traits.h` into `libcxx/test/support/test_char_traits.h`, containing `cpp17_char_traits<T>` (which replaces `constexpr_char_traits` throughout the tests) and `cpp20_char_traits<T, Ordering>` (which replaces `TraitsWithCompCat` here, and in the tests for `string::operator<=>` once that's written). ... But even as I'm writing it out, it seems like a bad idea. `cpp20_char_traits<T, Ordering>` would be used in only 2 tests, so it doesn't pay for itself; and `TraitsWithoutCompCat` still needs to be written out longhand.
> 
> And I just realized that it might be nice to have another test for
> ```
> struct TraitsWithUserDefinedCompCat : std::char_traits<char> {
>     struct comparison_category {
>         explicit comparison_category(std::strong_ordering);
>     };
> };
> ```
> since for some reason C++20 doesn't require that `comparison_category` be a comparison category type.
Update: What I wrote here doesn't work on the GCC buildkite anyway, because GCC and EDG have what-looks-like-a-bug: the compiler will "dig past" non-typenames to discover typenames in base classes. https://godbolt.org/z/qWxxoeE8r So I'll have to do something not-involving-inheritance here anyway! I don't have any good ideas what, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114912