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

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Jan 18 13:07:27 PST 2021


Quuxplusone added a comment.

@ldionne: LGTM except that the non-constexpr tests no longer need to `return true;` from `bool test()` — they could do `void test()` for now.
Also, I don't think this will rebase as-is on generate_feature_test_macro_components.py, will it? You'll have to add the entry for `__cpp_lib_string_contains`, which will be easy — but I notice that it //will// be a divergence from what's literally in https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations as of this writing. I would bet that's fine; I'm just bringing it up for awareness.

The current direction on constexpr/noexcept sounds great to me.



================
Comment at: libcxx/test/std/strings/basic.string/string.contains/contains.char.pass.cpp:35
+
+    return true;
+}
----------------
Nit: you can remove the `bool` and `return true` for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93912



More information about the libcxx-commits mailing list