[libcxx-commits] [PATCH] D130295: [libc++] Uses operator<=> in string_view

[Kent Ross via Phabricator via libcxx-commits]

mumbleskates added inline comments.


================
Comment at: libcxx/test/std/strings/string.view/string.view.comparison/comparison.pass.cpp:126-129
+  test<std::basic_string_view<CharT, constexpr_char_traits<CharT>>,
+       std::weak_ordering>(); // No ordering in its char_traits
+  test<std::basic_string_view<CharT, char_traits<CharT, std::weak_ordering>>, std::weak_ordering>();
+  test<std::basic_string_view<CharT, char_traits<CharT, std::partial_ordering>>, std::partial_ordering>();
----------------
while looking at asserting the returned ordering type inside `testOrder()` in `test_comparisons.h`, i found that the only tests in the entire suite that fail are these right here, because `test()` above immediately discards the char_traits after the assertions it makes with type `T` on lines 74 and 75. Only those two lines are effectively different; after that, the `basic_string` and `basic_string_view` types are reconstructed from the extracted `CharT` and, for the same `CharT`, are completely identical.

is this intentional? it seems like it isn't, because every three-way comparison tested after `AssertOrderReturn<Ordering, T>();` on line 75 actually produces a `std::strong_ordering` no matter what.

The good news is that the string view types do indeed seem to return the correct ordering, as it's tested on the first 2 lines, but it seems like this might be unintentionally missed surface area and i can't be sure on my own.

You can see this yourself, by replacing all the `?:` expressions with `i` and `j` with `i <=> j` and adding `ASSERT_SAME_TYPE(decltype(t1 <=> t2), Order);` inside `testOrder()` and it will pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130295