[libcxx-commits] [PATCH] D93912: [libc++][P1679] add string contains

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Dec 29 13:08:30 PST 2020

Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.

Comment at: libcxx/include/string:330
+    bool contains(const charT* s) const;                               // C++23
     bool __invariants() const;
Before libc++ starts adding C++23 stuff, I think it needs someone (notme) to do a "C++20 exists now" audit. References to "2a" should be changed to "20" — especially in the test suite, where I keep tripping up by typing `--param std=c++20` instead of `--param std=c++2a` — and then a "2b" mode needs to be added.

I weakly recommend using "2b" instead of "23" here, for pedantic accuracy. I admit that it is //very likely// that C++2b will be C++23, and that by recommending "2b" I'm just making work for somebody three years from now. :P

Comment at: libcxx/include/string:1437
     bool ends_with(const value_type* __s) const _NOEXCEPT
     { return ends_with(__self_view(__s)); }
Incidentally, I wonder why this existing code isn't `return __self_view(data(), size()).ends_with(__s)`. I like the symmetrical way you did `contains` below, and wish `starts_with` and `ends_with` would match it... unless there's a good reason they were done this way originally.

Comment at: libcxx/test/std/strings/basic.string/string.contains/contains.ptr.pass.cpp:22
+    {
+    typedef std::string S;
+    const char *s = "abcde";
Could use `using S = std::string;` here, since this is a non-03 test.

Comment at: libcxx/test/std/strings/basic.string/string.contains/contains.ptr.pass.cpp:30
+//  S   s4  { s + 1, 4 };
+//  S   s5  { s,     5 };
+    S  sNot { "xyz", 3 };
Why commented out?

Comment at: libcxx/test/std/strings/string.view/string.view.template/contains.char.pass.cpp:37
+#if TEST_STD_VER > 11
+    {
This is always true; you don't need the `#if`.
You don't need `constexpr_char_traits` in '20 and later (which includes this test).

Also, please do
constexpr bool test() {
    std::string_view sv1;
    std::string_view sv2 = "abcde";
    return true;

int main(int, char **)
to get constexpr coverage without repeating the code.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list