[libcxx-commits] [PATCH] D114658: [libc++] [test] Refactor string_view comparison tests for comprehensiveness.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

In D114658#3157163 <https://reviews.llvm.org/D114658#3157163>, @jloser wrote:

> Any idea what the existing motivation is to support string_view all the way back to C++03?

It was seen as a really useful type and at the time it was thought that it would be beneficial for users that are stuck in C++03 to be able to use this. After being bit by backporting features a couple of times, we don't do this anymore and we instead encourage people to upgrade to a newer version of the standard.



================
Comment at: libcxx/test/std/strings/string.view/string.view.comparison/opge.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
Can you name the new tests something less terse, e.g. `equal.pass.cpp` and so on?


================
Comment at: libcxx/test/std/strings/string.view/string.view.comparison/opgt.pass.cpp:12
+// template<class charT, class traits>
+//   constexpr bool operator>(basic_string_view<charT,traits> lhs, basic_string_view<charT,traits> rhs);
+// (plus "sufficient additional overloads" to make implicit conversions work as intended)
----------------
Nitpick. Applies elsewhere too.


================
Comment at: libcxx/test/std/strings/string.view/string.view.comparison/opne.pass.cpp:48-52
+            assert((v[i] != v[j]) == expected);
+            assert((v[i].data() != v[j]) == expected);
+            assert((v[i] != v[j].data()) == expected);
+            assert((ConvertibleTo<SV>(v[i]) != v[j]) == expected);
+            assert((v[i] != ConvertibleTo<SV>(v[j])) == expected);
----------------
Can you link to http://eel.is/c++draft/string.view#tab:string.view.comparison.overloads so it's easy to see what you are testing for exhaustiveness? Same for other operators.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114658